stretchr / testify

A toolkit with common assertions and mocks that plays nicely with the standard library
MIT License
23.15k stars 1.59k forks source link

unnecessary/incorrect log call in AssertExpectations #1543

Closed win-arturo closed 7 months ago

win-arturo commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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.