Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GoTestCompile not always deleting created test-binary with Neovim #907

Closed
Limdi opened this issue Jun 18, 2016 · 10 comments · Fixed by #1317
Closed

GoTestCompile not always deleting created test-binary with Neovim #907

Limdi opened this issue Jun 18, 2016 · 10 comments · Fixed by #1317

Comments

@Limdi
Copy link

Limdi commented Jun 18, 2016

Hi,
#879 said GoTestCompile removes its created test-binaries which works fine working on go-files in the same directory, I'd love to see this removing of the test-binary incase it got created in another directory too.

Actual behavior

➜  pkg git:(master) ✗ ls storage 
github  github.go  gitlab.go  storage.go  storage_test.go
➜  pkg git:(master) ✗ nvim storage/storage.go
# Running in neovim GoTestCompile
➜  pkg git:(master) ✗ ls storage 
github  github.go  gitlab.go  storage.go  storage_test.go  vim-go-test-compile

For me it reproduces nicely for other directories too

➜  pkg git:(master) ✗ find . | grep vim-go-test     
./storage/vim-go-test-compile
./mirror/project/vim-go-test-compile
./mirror/vim-go-test-compile

Expected behavior

➜  pkg git:(master) ✗ ls storage 
github  github.go  gitlab.go  storage.go  storage_test.go
➜  pkg git:(master) ✗ vim storage/storage.go
# Running in vim :GoTestCompile
➜  pkg git:(master) ✗ ls storage
github  github.go  gitlab.go  storage.go  storage_test.go

Configuration

Add here your current configuration and additional information that might be
useful, such as:

  • vimrc you used to reproduce
  • vim version:
NVIM 0.1.4
Build type: RelWithDebInfo
Compilation: /usr/bin/cc -march=x86-64 -mtune=generic -O2 -pipe -fstack-protector-strong -Wconversion -O2 -g -DDISABLE_LOG -Wall -Wextra -pedantic -Wno-unused-parameter -Wstrict-prototypes -std=gnu99 -Wvla -fstack-protector-strong -fdiagnostics-color=auto -DINCLUDE_GENERATED_DECLARATIONS -DHAVE_CONFIG_H -D_GNU_SOURCE -I/build/neovim/src/build/config -I/build/neovim/src/neovim-0.1.4/src -I/usr/include -I/usr/include -I/usr/include/luajit-2.0 -I/usr/include -I/usr/include -I/usr/include -I/usr/include -I/usr/include -I/build/neovim/src/build/src/nvim/auto -I/build/neovim/src/build/include
  • vim-go version: latest updated with :PlugUpdate
523d5a435d490c8c5ea3d8ed0ecac453e2f33f93
  • go version
go version go1.6.2 linux/amd64
@nightlyone
Copy link

I can reproduce this too:

  • Linux (Ubuntu 16.04, codename Xenial) using go 1.6.2 too.
  • Vim: VIM - Vi IMproved 7.4 (2013 Aug 10, compiled Apr 08 2016 11:38:28)

Note:
A simple workaround for me was to use /dev/null (or NUL on windows) instead of "vim-compile-test" as the compiler target at https://github.com/fatih/vim-go/blob/master/autoload/go/cmd.vim#L194 in order to discard compiler result.

@fatih
Copy link
Owner

fatih commented Jun 22, 2016

Thanks @Limdi for the report. I'm looking into it.

@nightlyone unfortunately /dev/null/ is not supported by the go tool. We have the same problem for go build as well. I know the problem for this issue. I have a fixed version for Vim already, testing it now for NeoVim.

@fatih
Copy link
Owner

fatih commented Jun 22, 2016

Thanks for the feedback. This issue is now fixed both for Vim and NeoVim. Please pull the latest HEAD of the master branch. Thanks.

@Limdi
Copy link
Author

Limdi commented Jun 23, 2016

Wonderfull, thanks you fatih!

@7AC
Copy link

7AC commented Jan 12, 2017

I'm still having this issue with vim 8.0, vim-go v1.11 installed via Vundle and this config which I basically copied from the tutorial:

function! s:build_go_files()
  let l:file = expand('%')
  if l:file =~# '^\f\+_test\.go$'
    call go#cmd#Test(0, 1)
  elseif l:file =~# '^\f\+\.go$'
    call go#cmd#Build(0)
  endif
endfunction
au BufWritePost *.go :call <SID>build_go_files()

I verified that my version has 1457e6e.

@oryband
Copy link

oryband commented Jan 22, 2017

Still happenning to me as well.

@rafael84
Copy link

I'm using vim 8.0.95 and vim-go 1.12. The issue is still happening.

@rafael84
Copy link

So I think the problem only happens when the current working directory is not the base directory of the test file.

I can easily reproduce the problem by running the following commands:

% tree
.
└── mypkg
    └── my_test.go

1 directory, 1 file


% cat mypkg/my_test.go
package mypkg

import "testing"

func TestMy(t *testing.T) {

}


% vim mypkg/my_test.go +GoTestCompile +'sleep 1' +qall


% tree
.
└── mypkg
    ├── my_test.go
    └── vim-go-test-compile

1 directory, 2 files

However, check what happens when the current directory is mypkg:

% tree
.
└── mypkg
    └── my_test.go

1 directory, 1 file


% cd mypkg; vim my_test.go +GoTestCompile +'sleep 1' +qall; cd ..


% tree
.
└── mypkg
    └── my_test.go

1 directory, 1 file

fatih added a commit that referenced this issue Jun 2, 2017
This change improves the way we compile a Go test. Previously we would
build a test binary to catch any build errors. A callback was then
responsible to delete the test binary after showing build errors (if
there were any).

This could be racy though, because if the tests lasts long and we quit
early, the callback to clean the binary file would never run. So we
would end up with artifacts in our working directory.

To fix the issue we're going to tell to the `go` tool to run a specific,
unique function. We're passing a unique identifier as a function name
(which is a randomly generated). This will cause the go tool to build
the test file and then try to run the test function. Because there is no
such test function, it'll silently quit with zero exit status. As a side
effect it'll compile the test file, so we're able to catch any build
errors (if any)

fixes #907
fatih added a commit that referenced this issue Jun 2, 2017
This change improves the way we compile a Go test. Previously we would
build a test binary to catch any build errors. A callback was then
responsible to delete the test binary after showing build errors (if
there were any).

This could be racy though, because if the tests lasts long and we quit
early, the callback to clean the binary file would never run. So we
would end up with artifacts in our working directory.

To fix the issue we're going to tell to the `go` tool to run a specific,
unique function. We're passing a unique identifier as a function name
(which is a randomly generated). This will cause the go tool to build
the test file and then try to run the test function. Because there is no
such test function, it'll silently quit with zero exit status. As a side
effect it'll compile the test file, so we're able to catch any build
errors (if any)

fixes #907
fatih added a commit that referenced this issue Jun 2, 2017
This change improves the way we compile a Go test. Previously we would
build a test binary to catch any build errors. A callback was then
responsible to delete the test binary after showing build errors (if
there were any).

This could be racy though, because if the tests lasts long and we quit
early, the callback to clean the binary file would never run. So we
would end up with artifacts in our working directory.

To fix the issue we're going to tell to the `go` tool to run a specific,
unique function. We're passing a unique identifier as a function name
(which is a randomly generated), i.e:

```
go test -run "F97CC94D-4414-41CF-A185-A077F645DF24"
testing: warning: no tests to run
PASS
ok  	demo	0.006s
```

This will cause the go tool to build
the test file and then try to run the test function. Because there is no
such test function, it'll silently quit with zero exit status. As a side
effect it'll compile the test file, so we're able to catch any build
errors (if any)

fixes #907
@fatih
Copy link
Owner

fatih commented Jun 2, 2017

@rafael84 please pull the latest master. We don't produce a test file anymore.

@rafael84
Copy link

rafael84 commented Jun 3, 2017

Awesome, thanks fatih.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants