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

GoCoverage broken in recent commit #824

Closed
bradleyfalzon opened this issue Apr 28, 2016 · 13 comments
Closed

GoCoverage broken in recent commit #824

bradleyfalzon opened this issue Apr 28, 2016 · 13 comments

Comments

@bradleyfalzon
Copy link
Contributor

Actual behavior

Checkout f830d0d and try to use GoCoverage
Tests run, but coverage report doesn't appear.

See also PR #801

Expected behavior

Write here what you're expecting ...

Steps to reproduce:

Please create a reproducible case of your problem. Re produce it
with a minimal vimrc with all plugins disabled and only vim-go
enabled:

  1. Open a go file that has tests
  2. Checkout vim-go 1.6
  3. Run GoCoverage
  4. See coverage report
  5. Checkout vim-go f830d0d
  6. Run GoCoverage
  7. No longer see coverage report

Configuration

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

  • vimrc you used to reproduce: NA
  • vim version: 7.4
  • vim-go version: Broken in f830d0d
  • go version: go 1.6.2 linux/amd64
@bradleyfalzon bradleyfalzon changed the title v:shell_error != return go#util#ShellError {return v:shell_error} GoCoverage broken in recent commit Apr 28, 2016
@bradleyfalzon
Copy link
Contributor Author

So the issue, as I believe is due to https://github.com/fatih/vim-go/pull/801/files#diff-4

This line has recently changed from:

if !v:shell_error
    call go#coverage#overlay(l:tmpname)
endif

To:

if go#util#ShellError() != 0
    call go#coverage#overlay(l:tmpname)
endif

Which appears to be a typo, as the call to call is only executed when ShellError is non-zero - i.e. when there's an error.

The issue should be straight forward to fix in this case, but there's other logic errors in the change which may need to be reviewed (L93, maybe lint.vim on L67)

@bradleyfalzon
Copy link
Contributor Author

@mattn - can you look over this and verify if this is a problem?

@mattn
Copy link
Contributor

mattn commented Apr 28, 2016

No

        exec '!go run ' . go#util#Shelljoin(go#tool#Files())

bang command should be set v:shell_errors always. maybe error occur at another part.

mattn added a commit to mattn/vim-go that referenced this issue Apr 28, 2016
@bradleyfalzon
Copy link
Contributor Author

Can you double check the logic in coverage.vim too, line 57 and 93: https://github.com/fatih/vim-go/pull/801/files#diff-4

@mattn
Copy link
Contributor

mattn commented Apr 28, 2016

Ah, sorry. My fix is bad. However I found some bug from your points. Thanks

@mattn
Copy link
Contributor

mattn commented Apr 28, 2016

strange. I can't reproduce it . do you install vimproc? or not?

@bradleyfalzon
Copy link
Contributor Author

These are the steps I used to reproduce:

main.go

package main

import "fmt"

func main() {
    fmt.Println("done")
}

main_test.go

package main

import "testing"

func TestMain(t *testing.T) {
    main()
}

Checkout vim 1.6 in vim-go:


git checkout v1.6
vim main.go
:GoCoverage

You should then see the main func statements change to the colour green.

Close the file and checkout later commit:

git checkout f830d0dc170adfd7854532d571d2a0ecf818633e
vim main.go
:GoCoverage

The test should pass, but the statements should not change to the colour green this time.

The following patch then fixes the issue:

diff --git a/autoload/go/coverage.vim b/autoload/go/coverage.vim
index e19ba78..3dbc918 100644
--- a/autoload/go/coverage.vim
+++ b/autoload/go/coverage.vim
@@ -54,7 +54,7 @@ function! go#coverage#Buffer(bang, ...)
         return
     endif

-    if go#util#ShellError() != 0
+    if !go#util#ShellError()
         call go#coverage#overlay(l:tmpname)
     endif

@bradleyfalzon
Copy link
Contributor Author

The proposed patch could easily be go#util#ShellError() == 0, I just used !go#util#ShellError() as an example.

@mattn
Copy link
Contributor

mattn commented Apr 28, 2016

Thanks figure out. Could you please try below.
put following line into above on

+    if !go#util#ShellError()

like

    let g:vimgo_debug = type(go#util#ShellError())
    if !go#util#ShellError()

And check with :echo g:vimgo_debug. What value returned?

@mattn
Copy link
Contributor

mattn commented Apr 28, 2016

Ah, one more. please check actual value of go#util#ShellError() also.

@mattn
Copy link
Contributor

mattn commented Apr 28, 2016

Hmm, I can't reproduce.

@bradleyfalzon
Copy link
Contributor Author

Yes the test passes, but the coverage isn't shown.

I think you maybe debugging in the wrong area, look at the lines that changed:

From:

if !v:shell_error
    call go#coverage#overlay(l:tmpname)
endif

To:

if go#util#ShellError() != 0
    call go#coverage#overlay(l:tmpname)
endif

In the first case, the call to overlay only occurs when v:shell_error == 0, but in the second case, it's only called when v:shell_error == 1 - I believe this is the error.

@bradleyfalzon
Copy link
Contributor Author

This is what should happen:
screen shot 2016-04-28 at 11 01 43 am

This is what happens:
screen shot 2016-04-28 at 11 02 49 am

Ignore the colour scheme I'm using, but that's the behaviour.

mattn added a commit to mattn/vim-go that referenced this issue Apr 28, 2016
mattn added a commit to mattn/vim-go that referenced this issue Apr 28, 2016
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

No branches or pull requests

2 participants