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

Adds the test name to the errors #42

Closed
wants to merge 1 commit into from
Closed

Adds the test name to the errors #42

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Apr 15, 2017

Error messages omit the table element name (tt.name), making it difficult (impossible?) to identify which table entry failed. This patch adds the name to the error message.

@googlebot
Copy link
Collaborator

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@coveralls
Copy link

coveralls commented Apr 15, 2017

Coverage Status

Coverage remained the same at 95.699% when pulling 345374a on serussell:master into 56f3b6d on cweill:master.

@ghost
Copy link
Author

ghost commented Apr 15, 2017 via email

@googlebot
Copy link
Collaborator

CLAs look good, thanks!

@cweill
Copy link
Owner

cweill commented Apr 16, 2017

Hi @serussell, I wasn't aware of this issue: the test name should always be printed per test case.

Normally the test name is printed if you are using subtests, and if you are not the test should be printed in the test output.

Which Go version are you using? Before I can accept your pull request, can you please provide an example of the generated code that is missing the test name?

@ghost
Copy link
Author

ghost commented Apr 16, 2017

Yes, the pull request may not be the correct solution. There appears to be an issue using the errors generated by gotest with vim-go (and possibly nvim). When there is only one failure, it does not print all lines of the output and misses the line that contains the test entry line:

With two errors:

cmd/leg/main_test.go|39| makeAlias() got1 = 1, want 2
||     --- FAIL: Test_makeAlias/no_sep (0.00s)
cmd/leg/main_test.go|39| makeAlias() got1 = 1, want 0

With one:

cmd/leg/main_test.go|39| makeAlias() got1 = 1, want 2

On the command line, all lines are printed:

➜  leg go test .
--- FAIL: Test_makeAlias (0.00s)
    --- FAIL: Test_makeAlias/no_sep (0.00s)
        main_test.go:39: makeAlias() got1 = 1, want 2
FAIL
FAIL    repos.seanrussell.us/legume/cmd/leg     0.002s

So vim-go's GoTest() is stripping out the lines that don't actually reference the error. By putting the relevant test reference information on a different line, they're getting stripped out. But this isn't an issue with gotests, because GoTest() is omitting only the last test's lines containing the reference information.

Debugging the interaction between gotests and vim-go is beyond my immediate ability, so I'll keep my patch local for the moment since it makes generated errors useful for vim-go. You can disregard the patch.

Thanks!

edit: Formatting

@ghost ghost closed this Apr 16, 2017
@cweill
Copy link
Owner

cweill commented Apr 20, 2017

Sorry to hear that your bug is in a another castle. It may be worth reporting to the vim-go maintainers if you haven't already.

@ghost
Copy link
Author

ghost commented Apr 21, 2017

For people encountering the same issue, I filed this ticket; vim-go#1262.

In summary, vim-go's error parsing regexp is expecting all of the metadata for the report to be on one line, not split across several lines. The way errors are formatted by go's testing package is usually congruent to this regexp, but when used the way gotests uses testing, namely testing.T.Run(), testing does split metadata and the regexp fails to match.

No response yet from the vim-go maintainers, so for the moment executing gotests (and more generally, testing.T.Run()) from within vim is unfortunately not an option.

This pull request was closed.
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 this pull request may close these issues.

3 participants