Using NiceMock?

[Rachel Blum]

There seem to be a bunch of tests that use mocks, but have unexpected uninteresting calls.

I'd prefer people used NiceMock<> for these, because the (irrelevant) warnings often clutter up the build logs. Is that a good plan, or am I missing a reason that speaks against NiceMock?

[Peter Kasting]

On Tue, Aug 9, 2016 at 5:30 PM, Rachel Blum <gr...@chromium.org> wrote:

There seem to be a bunch of tests that use mocks, but have unexpected uninteresting calls.

I'd prefer people used NiceMock<> for these, because the (irrelevant) warnings often clutter up the build logs. Is that a good plan, or am I missing a reason that speaks against NiceMock?

Not really an answer to your question, but I've rarely seen tests that seem better, more readable, less fragile, etc. to me using mocks than they would avoiding mocks (and using subclasses, dependency injection, registering as observers, etc.).

So my personal bias is that rather than using NiceMock in some of these cases, we should see if we can avoid mocks entirely.

It is quite possible that I am speaking out of ignorance of good mocking practices here; however, I've seen this opinion expressed before by various others (Brett Wilson comes to mind).

PK 

[dan...@chromium.org]

On Tue, Aug 9, 2016 at 5:30 PM, Rachel Blum <gr...@chromium.org> wrote:

There seem to be a bunch of tests that use mocks, but have unexpected uninteresting calls.

I'd prefer people used NiceMock<> for these, because the (irrelevant) warnings often clutter up the build logs. Is that a good plan, or am I missing a reason that speaks against NiceMock?

I would expect a test author to either EXPECT all the correct calls or use NiceMock. I do not enjoy log spams.

 

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

[Matt Giuca]

On Wed, 10 Aug 2016 at 10:39 Peter Kasting <pkas...@chromium.org> wrote:

On Tue, Aug 9, 2016 at 5:30 PM, Rachel Blum <gr...@chromium.org> wrote:

There seem to be a bunch of tests that use mocks, but have unexpected uninteresting calls.

I'd prefer people used NiceMock<> for these, because the (irrelevant) warnings often clutter up the build logs. Is that a good plan, or am I missing a reason that speaks against NiceMock?

Not really an answer to your question, but I've rarely seen tests that seem better, more readable, less fragile, etc. to me using mocks than they would avoiding mocks (and using subclasses, dependency injection, registering as observers, etc.).

So my personal bias is that rather than using NiceMock in some of these cases, we should see if we can avoid mocks entirely.

I've run into this issue several times (it seems contentious on Chromium). I think it's harmful to have a blanket ban on mocking (which we have in some parts of the code base) or general advice to "avoid mocks entirely". Like many tools, mocking can be misused but there are cases where it is simply the right tool for the job. (Not getting into Gmock versus other mocking tools, just talking about mocking as a concept.)

I think the best rule of thumb is: avoid mocks if you can. If you can use subclasses, dependency injection, fakes, for_test methods, etc, go ahead. Mocking should be a last resort. But sometimes, testing that some code performs an exact sequence of operations, when the results of those operations do not leave any directly observable state, is best done with a mock. Two major use categories come to mind: graphics drawing operations and network requests.

A few months ago, I had a CL adding a test for graphical drawing, but gmock was banned in that particular part of the code base (ui/views). I was told by the reviewer that I couldn't use it but it was OK to record the drawing operations and then test that the expected operations were drawn correctly. So I did that. The result was that I essentially wrote my own small and crappy mocking toolkit with all the drawbacks of mocking but without the advantages of the years of engineering work on Gmock. IMO we ended up with much more complex, less readable tests, just so we could blindly follow a blanket "don't use Gmock, because mocking is bad" edict.

 

It is quite possible that I am speaking out of ignorance of good mocking practices here; however, I've seen this opinion expressed before by various others (Brett Wilson comes to mind).

PK 

--

[Peter Kasting]

On Tue, Aug 9, 2016 at 6:51 PM, Matt Giuca <mgi...@chromium.org> wrote:

I think the best rule of thumb is: avoid mocks if you can. If you can use subclasses, dependency injection, fakes, for_test methods, etc, go ahead. Mocking should be a last resort. But sometimes, testing that some code performs an exact sequence of operations, when the results of those operations do not leave any directly observable state, is best done with a mock.

This seems like the right advice to me.

A few months ago, I had a CL adding a test for graphical drawing, but gmock was banned in that particular part of the code base (ui/views). I was told by the reviewer that I couldn't use it but it was OK to record the drawing operations and then test that the expected operations were drawn correctly. So I did that. The result was that I essentially wrote my own small and crappy mocking toolkit with all the drawbacks of mocking but without the advantages of the years of engineering work on Gmock. IMO we ended up with much more complex, less readable tests, just so we could blindly follow a blanket "don't use Gmock, because mocking is bad" edict.

I disagree with blanket bans.

I think in a case like this, you have some pretty solid evidence (in the form of the test code that resulted) to escalate to Chrome leadership.  Ideally, we would tell owners not to institute blanket bans like this.  Less ideally, you could get an exception for your case.  The least desirable state long-term would be to leave things as you describe.  Someday someone is going to have to update those tests, and it would be better to use Gmock, which at least the person can research how to use, than to be using a totally custom thing which must be grokked before being modified.

PK

[Matt Giuca]

Thanks, that's good to hear.

I hope I didn't offend the people involved by bringing this up in a public forum. I deliberately avoided specifics and names because I don't think it's the fault of the people involved, just an institutional problem (in some parts of the codebase). I'll discuss offline to see if you think it's worth refactoring this specific case.

 

PK

[Adam Rice]

I prefer StrictMock over NiceMock, since it will catch more bugs. The default behaviour is useless since another engineer looking at the warnings scrolling past won't know whether they indicate a problem or not.

Back when I used to use gmock, I used to write

EXPECT_CALL(...).Times(AtLeast(0));

to indicate that a method might be called but it didn't matter.

--

[Václav Brožek]

Using EXPECT_CALL(...).Times(AtLeast(0)); is exactly what GMock advises against: "Do not suppress [uninteresting calls] by blindly adding an EXPECT_CALL() if you don't mean to enforce the call." There are two reasons for that I can see:

• It adds clutter to the tests and makes hard to distinguish what is meant to be tested and what is just to silence logs.
• It often puts restrictions on implementation of the tested code, as opposed to just the public API interaction, making the test fragile.
  (This is not the case of Times(AtLeast(0)), though.)

I'd also challenge the statement that StrictMock catches more bugs. StrictMock is more strict and reports changes in the code interactions as errors, regardless of they are indeed bugs or intended. Filling the tests with StrictMock is introducing restrictions beyond what should be tested and doing changes later will come with overhead of fixing broken tests potentially without any real bugs discovered.

It is fine to use StrictMock locally when introducing the test if you prefer test failures to fishing for warnings in the logs, but I'd suggest thinking twice before lending CLs with StrictMocks.

One of the issues people voiced about GMock is that it is often not used how it was intended. Therefore following the advice of GMock itself (against using EXPECT_CALL without meaning to enforce the calls) seems like a good thing to do.

Cheers,
Vaclav

[Greg Thompson]

On Wed, Aug 10, 2016 at 3:52 AM Matt Giuca <mgi...@chromium.org> wrote:

On Wed, 10 Aug 2016 at 10:39 Peter Kasting <pkas...@chromium.org> wrote:

On Tue, Aug 9, 2016 at 5:30 PM, Rachel Blum <gr...@chromium.org> wrote:

There seem to be a bunch of tests that use mocks, but have unexpected uninteresting calls.

I'd prefer people used NiceMock<> for these, because the (irrelevant) warnings often clutter up the build logs. Is that a good plan, or am I missing a reason that speaks against NiceMock?

Not really an answer to your question, but I've rarely seen tests that seem better, more readable, less fragile, etc. to me using mocks than they would avoiding mocks (and using subclasses, dependency injection, registering as observers, etc.).

So my personal bias is that rather than using NiceMock in some of these cases, we should see if we can avoid mocks entirely.

I've run into this issue several times (it seems contentious on Chromium). I think it's harmful to have a blanket ban on mocking (which we have in some parts of the code base) or general advice to "avoid mocks entirely". Like many tools, mocking can be misused but there are cases where it is simply the right tool for the job. (Not getting into Gmock versus other mocking tools, just talking about mocking as a concept.)

I think the best rule of thumb is: avoid mocks if you can. If you can use subclasses, dependency injection, fakes, for_test methods, etc, go ahead. Mocking should be a last resort. But sometimes, testing that some code performs an exact sequence of operations, when the results of those operations do not leave any directly observable state, is best done with a mock. Two major use categories come to mind: graphics drawing operations and network requests.

A few months ago, I had a CL adding a test for graphical drawing, but gmock was banned in that particular part of the code base (ui/views). I was told by the reviewer that I couldn't use it but it was OK to record the drawing operations and then test that the expected operations were drawn correctly. So I did that. The result was that I essentially wrote my own small and crappy mocking toolkit with all the drawbacks of mocking but without the advantages of the years of engineering work on Gmock. IMO we ended up with much more complex, less readable tests, just so we could blindly follow a blanket "don't use Gmock, because mocking is bad" edict.

^^^ This.

I think it hurts test readability when each test harness re-invents this particular wheel differently. I am opposed to a blanket ban on GMock.

[Gabriel Charette]

On Wed, Aug 10, 2016 at 6:06 AM Greg Thompson <g...@chromium.org> wrote:

On Wed, Aug 10, 2016 at 3:52 AM Matt Giuca <mgi...@chromium.org> wrote:

On Wed, 10 Aug 2016 at 10:39 Peter Kasting <pkas...@chromium.org> wrote:

On Tue, Aug 9, 2016 at 5:30 PM, Rachel Blum <gr...@chromium.org> wrote:

There seem to be a bunch of tests that use mocks, but have unexpected uninteresting calls.

I'd prefer people used NiceMock<> for these, because the (irrelevant) warnings often clutter up the build logs. Is that a good plan, or am I missing a reason that speaks against NiceMock?

Not really an answer to your question, but I've rarely seen tests that seem better, more readable, less fragile, etc. to me using mocks than they would avoiding mocks (and using subclasses, dependency injection, registering as observers, etc.).

So my personal bias is that rather than using NiceMock in some of these cases, we should see if we can avoid mocks entirely.

I've run into this issue several times (it seems contentious on Chromium). I think it's harmful to have a blanket ban on mocking (which we have in some parts of the code base) or general advice to "avoid mocks entirely". Like many tools, mocking can be misused but there are cases where it is simply the right tool for the job. (Not getting into Gmock versus other mocking tools, just talking about mocking as a concept.)

I think the best rule of thumb is: avoid mocks if you can. If you can use subclasses, dependency injection, fakes, for_test methods, etc, go ahead. Mocking should be a last resort. But sometimes, testing that some code performs an exact sequence of operations, when the results of those operations do not leave any directly observable state, is best done with a mock. Two major use categories come to mind: graphics drawing operations and network requests.

A few months ago, I had a CL adding a test for graphical drawing, but gmock was banned in that particular part of the code base (ui/views). I was told by the reviewer that I couldn't use it but it was OK to record the drawing operations and then test that the expected operations were drawn correctly. So I did that. The result was that I essentially wrote my own small and crappy mocking toolkit with all the drawbacks of mocking but without the advantages of the years of engineering work on Gmock. IMO we ended up with much more complex, less readable tests, just so we could blindly follow a blanket "don't use Gmock, because mocking is bad" edict.

^^^ This.

I think it hurts test readability when each test harness re-invents this particular wheel differently. I am opposed to a blanket ban on GMock.

+1, if your goal is unit-testing response flow in a Delegate say, then mocking is the perfect tool and writing your own mock (with overrides that register the calls made and the arguments passed, etc.) always feels like re-inventing the wheel for no reason.