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

unnecessary/incorrect log call in AssertExpectations #1543

Closed
win-arturo opened this issue Feb 20, 2024 · 2 comments
Closed

unnecessary/incorrect log call in AssertExpectations #1543

win-arturo opened this issue Feb 20, 2024 · 2 comments

Comments

@win-arturo
Copy link

Description

When running tests and making the AssertExpectations call, log of any successful expected call is unnecessarily output.

Step To Reproduce

m := &mock.Mock{}
m.On("someCall")
m.AssertExpectations(t)

Expected behaviour

If the call is made, as expected, no output should be generated. This is in keeping with test first philosophy that a test that requires a human to read its output is not a good test.

Actual behaviour

some_test.go:107: PASS: Get(*mock.IsTypeArgument,string,string)

personally, I think that the log statement is in the wrong place:

expectedCalls := m.expectedCalls()
for _, expectedCall := range expectedCalls {
	satisfied, reason := m.checkExpectation(expectedCall)
	if !satisfied {
		failedExpectations++
	}
	t.Logf(reason) // <-- wrong place
}


expectedCalls := m.expectedCalls()
for _, expectedCall := range expectedCalls {
	satisfied, reason := m.checkExpectation(expectedCall)
	if !satisfied {
		failedExpectations++
	        t.Logf(reason) // <-- correct place
	}
}
@st3penta
Copy link

st3penta commented Feb 20, 2024

Here is a snippet that reproduces the issue on playground, by giving this output:

=== RUN   TestIfy
    prog_test.go:30: PASS:	MockMethod(int)
--- PASS: TestIfy (0.00s)
PASS

But if i run the same test locally with the -v option, i get this one:

=== RUN   TestIfy
--- PASS: TestIfy (0.00s)
PASS

Also, the log statement is already where @win-arturo suggested to put it (here), so the second output seems the one in line with the code.

I guess i'm missing something?

@brackendawson
Copy link
Collaborator

That's because the playground will import the latest release of testify (v1.8.4) and this was fixed after that in #1360, you must have later version locally.

I intend to tag v1.8.5 quite soon, once all the already approved PRs are resolved. If that's not soon enough, you can go get github.com/stretchr/testify@master.

Closing as duplicate of #1358, thank you for reporting.

@brackendawson brackendawson closed this as not planned Won't fix, can't repro, duplicate, stale Feb 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants