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.

Sign up for GitHub

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

Jump to bottom

go1.7 vet flags "possible formatting directive" when Error is used #339

Closed
prashantv opened this issue Aug 21, 2016 · 3 comments
Closed

go1.7 vet flags "possible formatting directive" when Error is used #339

prashantv opened this issue Aug 21, 2016 · 3 comments

Comments

@prashantv
Copy link

When the assert.Error method is used with formatting arguments, go vet complains as of 1.7:

flag_test.go:61: possible formatting directive in Error call

For more information and a repro, see:
uber-go/zap#135

The specific check that is run is here:
https://golang.org/src/cmd/vet/print.go#L587

I'm not sure if there's a good fix for this, but I wanted to flag this, as I typically:

  • recommend adding context to the assert.Error calls
  • suggest usage of assert.Error instead of assert.NotNil

However, this "correct" usage of the library leading to a vet failure may mean users will either avoid one or the other.
@ernesto-jimenez
Copy link
Member

@prashantv this will require creating methods like Errorf and Equalf as counterparts of Error and Equal with formatted messages.

Let me look into it :)

ernesto-jimenez added a commit that referenced this issue Sep 24, 2016
@ernesto-jimenez
Fixes #339 - Add assertionf assertions like Errorf and Equalf
a0fdc96
@ernesto-jimenez ernesto-jimenez mentioned this issue Sep 24, 2016
Merged
@ernesto-jimenez
Copy link
Member

@prashantv what does #356 look like?

ernesto-jimenez added a commit that referenced this issue Sep 25, 2016
@ernesto-jimenez
Fixes #339 - Add assertionf assertions like Errorf and Equalf
39e0258
@prashantv
Copy link
Author

Thanks @ernesto-jimenez, left some comments but overall looks good!

prashantv added a commit to uber/tchannel-go that referenced this issue Oct 4, 2016
@prashantv
Only lint on 1.6
25aab0e 
We can't lint on 1.7 till we work around
stretchr/testify#339
@prashantv prashantv mentioned this issue Oct 4, 2016
Merged
prashantv added a commit to uber/tchannel-go that referenced this issue Oct 4, 2016
@prashantv
Only lint on 1.6 (#510)
438d2fd 
We can't lint on 1.7 till we work around
stretchr/testify#339
prashantv added a commit to uber/tchannel-go that referenced this issue Dec 21, 2016
@prashantv
Only lint on 1.6 (#510)
0874dad 
We can't lint on 1.7 till we work around
stretchr/testify#339
@astj astj mentioned this issue Jan 24, 2017
Merged
ernesto-jimenez added a commit that referenced this issue May 29, 2017
@ernesto-jimenez
Fixes #339 - Add assertionf assertions like Errorf and Equalf
509ffd4
@emou emou mentioned this issue Aug 23, 2017
Closed
@mcastelino mcastelino mentioned this issue Sep 20, 2017
Closed
@mcastelino mcastelino mentioned this issue Oct 9, 2017
Merged
ghost pushed a commit to hyperledger/fabric-ca that referenced this issue Oct 9, 2017
[FAB-5300] Updated github.com/stretchr/testify pkg
086e651 
Updated github.com/stretchr/testify package to the latest version
to fix the problem described in the issue
stretchr/testify#339

Change-Id: Ib030abdb5e435448e97966c4a6c3dd8bd4a6b9fd
Signed-off-by: Anil Ambati <aambati@us.ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@ernesto-jimenez @prashantv