-
Notifications
You must be signed in to change notification settings - Fork 10.3k
Uninteresting call to ON_CALL method #2078
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
Comments
If you don't care about calls to a specific function would just do:
|
@EmilyBjoerk , I use NiceMock to eliminate all warnings and write tests for the behavior I actually care. I think this is a better approach as you keep your tests small and straight to the point with which they should test without flooding with "EXPECT_CALL's". |
It is honestly ridiculous that you cannot suppress this noisy warning by doing an explicit ON_CALL. The noisiness of the warning is self-defeating; the fact that there is apparently no way to suppress it (while still using GoogleMock) encourages doing precisely the thing it warns against. It's not that people don't appreciate the downsides of overconstrained tests. I'm sure many devs have experienced fragile test suites that break at small changes and so discourage such changes from being made. Such fragility is also not the end of the world: you can deal with it, it's just more work to have to repair tests often. Avoiding noise in the output of a test suite is not an unimportant thing. It's not too much to ask that we should be able to write tests that are not overconstrained but that also run free of noise. It isn't just about aesthetics; it really matters to the usability and quality of a testing framework and test suites written with it. If you have Rather than admonishing the viewer of test output for caring about noise and implying that they shouldn't, a good warning should include how to fix it. Okay, maybe the default output of an "uninteresting" mock method is actually the wrong output; I'm not saying the warning is useless. But then, if you explicitly specify that the call is uninteresting and confirm what the right output is, you still get the noise? Arrgh. It may annoy me enough that I fork Google Test and fix it myself, but it will be a lot more work for me to do that than for the Google Mock devs to do it (like, probably a factor of 10 more work, and I might end up with a "fix" that is incomplete and only suppresses the noise in the test suite I'm working on). |
@daira, I totally agree with you. Actually, when I saw an "uninteresting call" while having an ON_CALL for it, I thought something was wrong with my test, otherwise it's very intuitive to expect suppressing these warnings when I catch them with ON_CALL. |
I ran into the same problem with the very noisy warning. So my pull request #4749 was the simplest solution that I could come up with. |
I have read https://github.com/google/googletest/blob/master/googlemock/docs/CookBook.md#knowing-when-to-expect and understand and agree with the motivation behind uninteresting calls.
However, even though I have added an
ON_CALL(Foo, Bar).WillByDefault(Return());
I get a uninteresting mock function call warning. The call to Foo.Bar() is clearly not uninteresting as it is mentioned in the test, the call is allowed (and expected) but not verified and thus I believe it should not emit a warning.Consider code under test like so:
Having small concise tests that test specific cases is best practice for testing. Hence I would like some cases that test the connection aspect, and some test cases that test the limiting of the cache size. The former would just use EXPECT_CALL, the latter doesn't care about the connect call but should allow it without warnings. Because warnings are bad. I can use NiceMock but then I loose the benefits intended by the default behaviour of uninteresting calls in all my tests. I can add EXPECT_CALL to the the tests on user cache size but this too is against the recommendations as it adds an unnecessary constraint that is not verified in that test case.
The text was updated successfully, but these errors were encountered: