GoTest: Output original message on failure return code

[shazow]

When using jobcontrol, assume FAILED if return code is >0. Only prints the first line.

This one is a little messier than the GoRename fix. The non-jobcontrol semantics aren't identical, as it prints the entire output rather than just the first line (but it's more consistent with the GoRename fix).

Let me know if you'd like for me to change it to be consistent. The output on unexpected test failures is typically a panic/timeout, so it's quite big.

[shazow]

jobwait(...) is needed to get the return code of the job. Since it's called on_exit of the respective job, it should never block, so I didn't provide a timeout.

[fatih]

This is a highly critical path as it's used by many applications. As I know relying on exit code might produce wrong results. I don't remember which subcommand or command, but there was a case where I wanted to get the output regardless of job id. Do we need to change it here ?

[shazow]

This should make it equivalent to checking v:shell_error. But yes, if there are commands that specifically avoid checking v:shell_error, then using jobcontrol would yield unexpected FAILURE. Are there any? Only one I can think of off the top of my head is the linter, which refuses to use proper return codes in a silly philosophical protest.

[fatih]

Ok, valid arguments. Right now we only use it for build and test. If there is anything wrong with any upstream app, we should fix it instead of.

[fatih]

Jobcontrol is only valid for neovim though, did you also test it with Vim? Or is this only valid for Neovim?

[shazow]

https://github.com/fatih/vim-go/pull/676/files#diff-a3998a862a20bc1665a15c84f401349fR198 is the change for plain-Vim. It's exactly the same change as gorename had. As I mentioned, it dumps the full output unlike the neovim path which dumps just the first line.

I have mixed feelings about that inconsistency, let me know how you feel about it. :)

[fatih]

Btw, I've just pulled this and tried it on a very simple app, such as :

package main

import (
    "fmt"
)

func main() {
    fmt.Println("fatih arslan")
}

The output is:

I've echoed it and it returns -1. There are two weird things going on here:

1. It seems it doesn't return a list, which contradicts with the help documentation (it says a list of integers)
2. -1 means : if the wait timed out for the job. The job already exited, so I think jobwait can't handle this case?

[shazow]

Hm what version of neovim are you running? I'm on NVIM 0.1.1.

[fatih]

I've echoed it and it returns -1

Forgot to say which part I've echoed:

let a = jobwait([a:job_id])
echo join(a, '')

[fatih]

Hm what version of neovim are you running? I'm on NVIM 0.1.1.

I have this too: NVIM 0.1.1 (compiled Dec 28 2015 16:18:38)

[shazow]

HMMMMM strange, can't reproduce it on my end. Will dig around, let me know if you stumble on a better way. :)

[shazow]

Ah :GoBuild does indeed reproduce, I was trying :GoTest. Hmm.

[fatih]

Yeah, I was going to try :GoBuild and :GoTest, couldn't try :GoTest as :GoBuild just failed.

[shazow]

Aha, a:data has the return code, so we don't need jobwait. Fixing.

[fatih]

Can you rename the variable to exit_status? It's ok as data for stderr and stdout handlers, but for the exit it would be better. In the future it also would helpful for others :)

[shazow]

Fixed! Btw nice way to test a command error,

:let g:go_test_timeout="badstring"
:GoTest

[fatih]

Fixed! Btw nice way to test a command error,

Great, indeed it works perfect :) Thanks!. Btw can you change the variable name? I think you missed it? If not I can do it later after merging :) Just let me know please

[fatih]

Thanks @shazow 👍

[shazow]

It's changed, I've been force-pushing into the same commit like a gentleman/savage (depending on your practices). :p

[shazow]

Happy to help! 🍮

[fatih]

Yeah, I had the cache, didn't saw it 🤦 :)

[dangerhuss]

This is causing errors when std_combined is empty.