googletest icon indicating copy to clipboard operation
googletest copied to clipboard

Uninteresting call to ON_CALL method

Open LiSongMWO opened this issue 6 years ago • 4 comments

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:

class UserDatabase{
public:
    UserDatabase(SQLBackend* sql, const std::string& uri, int usersCached){
        sql.connect(uri);
        if(maxUsersCached > MAX_CACHE_SIZE){
             m_users_cached = MAX_CACHE_SIZE;
        } else {
            m_users_cached = usersCached;
        }
    }

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.

LiSongMWO avatar Jan 24 '19 21:01 LiSongMWO

If you don't care about calls to a specific function would just do:

EXPECT_CALL(Foo, Bar()).Times(AnyNumber());

alexv1n avatar May 23 '19 15:05 alexv1n

@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".

koponomarenko avatar Jul 22 '19 15:07 koponomarenko

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 $n$ warnings that you can't suppress across a large suite, then what's the visible difference between that and $n+1$ warnings? How many developers, when they have so many other demands on their time and attention, are going to comb through a long CI output saying, yes, this warning is significant but all these others are noise? It's disrespectful of their time to make them do that. And that $(n+1)^{th}$ warning might actually be a problem with a test that is hiding a bug. This is why one of the important characteristics of a good compiler diagnostic is that there be a way to write the program to avoid it; preferably one that results in clearer code. If a warning does not provide an incentive to remove it by changing the code to be clearer and more likely to be correct, what's the point?

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 avatar Apr 17 '23 13:04 daira

@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.

sadtab avatar Jul 03 '24 20:07 sadtab