Should we officially deprecate GMock?

[Albert J. Wong (王重傑)]

I added gmock because I saw it being used very effectively inside Google for creating small, surgical, unittests. Unfortunately, I think it hasn't been working nicely within Chromium.

My observation has been that many Chromium "unittests" are really integration tests that exercise multiple classes with wide interfaces and complex interactions. As such, it is hard to really mock out the whole interface without adding a bunch of behavior into the mock.  Thus, the "mock object" ceases being a mock and becomes a fragile, hard to read, "fake". (definition of terms: http://googletesting.blogspot.com/2008/06/tott-friends-you-can-depend-on.html)

Even worse, because it is easy to add behavior into a Gmock object, I've seen people script a mock with a long sequence of calls/results.  The test now has moved beyond asserting invariants about API behavior to asserting the specific call sequence the current implementation has. (aka a Characterization Test).

Gmock is a framework for removing boilerplate out of unittests that are structured around the use of mock objects. It really only works well in this usage. Since we don't follow a purist "mock-based unittest" philosophy, my gut feeling is that Gmock adds more complexity than it saves and can encourage bad testing patterns (eg., characterization test, or partial mocking of objects).

What are people's thoughts?  Is my perception wrong?  Do find Gmock to be a net benefit for Chromium testing stability and maintenance?  If not, should we ban further usage and try to migrate current tests away from it?

-Albert

[Drew Wilson]

I don't object to banning gmock in new tests - I've definitely seen a tendency for my gmock tests to become overly brittle due to over-specification of API calls.

However, I would disagree with a decision to disable *working* tests, merely because they use gmock:

http://code.google.com/p/chromium/issues/detail?id=134675

So I think deprecating gmock is fine, but I'm not a big fan of losing test coverage in support of a dogmatic policy.

 

-Albert

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

[Jeffrey Yasskin]

On Wed, Jun 27, 2012 at 11:26 AM, Albert J. Wong (王重傑) <ajw...@chromium.org> wrote:

I appreciate gmock's support for EXPECT_THAT(object, ComplexPredicate(...)), and I think it helps readability in tests that use it (http://code.google.com/searchframe#search/&exact_package=chromium&q=EXPECT_THAT%20-file:gtest%20-file:gmock&type=cs), especially when, as for Pointee and ElementsAre, one EXPECT_THAT can replace a series of ASSERT_EQ and EXPECT_EQ.

I'm more ambivalent about the actual mock objects. I think they can be used well, but doing so takes a lot of discipline.

Jeffrey

[Erik Wright]

[from the right address]

Personally I'd much rather see more unit tests, which would presumably result in more appropriate uses of GMock. It's the integration tests that are killing us, IMHO.

[Peter Kasting]

On Wed, Jun 27, 2012 at 11:26 AM, Albert J. Wong (王重傑) <ajw...@chromium.org> wrote:

Gmock is a framework for removing boilerplate out of unittests that are structured around the use of mock objects. It really only works well in this usage. Since we don't follow a purist "mock-based unittest" philosophy, my gut feeling is that Gmock adds more complexity than it saves and can encourage bad testing patterns (eg., characterization test, or partial mocking of objects).

What are people's thoughts?  Is my perception wrong?  Do find Gmock to be a net benefit for Chromium testing stability and maintenance?  If not, should we ban further usage and try to migrate current tests away from it?

As someone who never did work in google3, I find gmock confusing.  I've always been secretly glad that it hasn't bled over into any of the areas I own or support.  I understand what most unittests are doing, but the same isn't true of the (very few) gmock tests I've happened to see.

I have anecdotally heard a few other engineers complain about gmock, either from a comprehensibility perspective or for some of the reasons you mention.  I believe Ben wasn't too happy about gmock in Chromium; maybe he has more concrete feedback.

PK

[Ben Goodger (Google)]

+1. The gmock usage I've encountered in refactoring has got in the way and has often represented a more esoteric way of verifying conditions are met.

I do think that integration tests are a reality of a product as complex as the one we have. Unit tests are good, but we will always have full system integration tests.

On Wed, Jun 27, 2012 at 11:26 AM, Albert J. Wong (王重傑) <ajw...@chromium.org> wrote:

-Albert

--

[John Abd-El-Malek]

+1

You've put this in much better terms than I would have. My impression with gmock has been that it's misused in most test. A lot of the places I see it used are calling foo() and ensuring that bar() is called. These tests are not that useful. It seems to me that many pieces of the code that have good interfaces around each component get by with thorough unit tests without a need for gmock. But classes that are heavily intertwined and which pass concrete objects as delegates, instead of interfaces, use gmock instead of adding proper interfaces to separate components.

gmock is also not that readable and adds a maintenance burden. Even if someone who writes a test knows it well, it imposes a cost on everyone else who wants to touch a code that's covered by gmock, since they might have to change a 7 line gmock statement.

I'm very supportive of banning gmock from any more tests. If we just say existing usage are fine, then they'll never be converted though. We would need to set a deadline, say a quarter or two, after which these tests would be disabled.

On Wed, Jun 27, 2012 at 11:26 AM, Albert J. Wong (王重傑) <ajw...@chromium.org> wrote:

-Albert

--

[Ryan Sleevi]

+1 to the general utility of EXPECT_THAT and predicates, particularly Pointee(). I find them very useful, especially because of their close integration with GTest printers. Looking at the GTest implementation file, there are some hints that matchers and EXPECT_THAT were planned to be moved to GTest, but that has not yet happened. That would be my only regret if we forbid GMock.

[Gregg Tavares (社用)]

We use gmock extensively in the GPU backend to validate that our code is calling the GL driver in the correct sequence for a given condition and that our code does not call the underlying driver when such a call would be unsafe.

Please don't remove gmock 

PS: unless of course you are volunteering to refactor all the code and not lose any of what's being tested. In that case knock yourself out ;-)

[Ben Goodger (Google)]

I think the answer is probably a bit more nuanced than just removing it completely.

There may be cases where it's useful, what we are saying is there are definitely cases where it isn't. So OWNERS will be responsible for exercising appropriate judgment as always. For instance, I am making this decision for integration tests in src/chrome, src/ash, src/ui etc. But that doesn't imply an overnight change to a lot of existing code.

What it means in practice is something like this. I've been forbidding its use in these areas for some time in code review. We may be able to add some additional protection via DEPS. It also means I may disable parts of tests and file bugs where I find inappropriate usage. I'll leave it up to other OWNERS to figure out if/how to deal with this.

-Ben

[Randy Smith]

On Wed, Jun 27, 2012 at 3:11 PM, John Abd-El-Malek <j...@chromium.org> wrote:

+1

You've put this in much better terms than I would have. My impression with gmock has been that it's misused in most test. A lot of the places I see it used are calling foo() and ensuring that bar() is called. These tests are not that useful. It seems to me that many pieces of the code that have good interfaces around each component get by with thorough unit tests without a need for gmock. But classes that are heavily intertwined and which pass concrete objects as delegates, instead of interfaces, use gmock instead of adding proper interfaces to separate components.

gmock is also not that readable and adds a maintenance burden. Even if someone who writes a test knows it well, it imposes a cost on everyone else who wants to touch a code that's covered by gmock, since they might have to change a 7 line gmock statement.

I'm very supportive of banning gmock from any more tests. If we just say existing usage are fine, then they'll never be converted though. We would need to set a deadline, say a quarter or two, after which these tests would be disabled.

Speaking as an owner of one of the systems that have such intertwined classes, I think you'll lose test coverage if you ban gmock and set a deadline by which tests can't use it anymore.  If you're attempting to create an actual unit test (i.e. test just one class) for a class that has the type of intertwinedness, something like gmock is very useful.  You do end up with a Characterization test (which term I hadn't heard before, but makes sense to me) and that's not ideal.  But I'm inclined to think it's better than nothing while you're attempting to peel apart the ball of string.  And setting a deadline is attempting to impose priority and planning from the outside, which is sometimes necessary, but often doesn't go well.

I have no objections to banning it for new tests, though I'd rather have best practices spelled out, reviewers educated, and it's use restricted to those cases for which it has value.  When I first encountered gmock, I had the sense that its use was actively encouraged for Chromium, and as a result it's shown up in a lot of download tests since then.  (Ooops.)  A best practice document would help new people not make my mistake, and would also help guide people in rewriting existing tests (including if you ban gmock, as my natural instinct at this point if you do would be to do some level of simple reimplementation.)

-- Randy

[Darin Fisher]

+1 to banning gmock.  Not sure about how the process would go for migrating existing tests.  Hmm...

[Gregg Tavares (社用)]

So what are we suggesting should be done instead? If you have a use case that exactly matches gmock then why not use it? If people are using it wrong that should be reflected in reviews but outright banning it seems a little extreme.

...confused...

[Darin Fisher]

Well, I'd like to study the good uses of gmock to learn more.  It sounds like you may have one.  Personally, I've only seen bad uses, so my opinion is pretty biased :-/

I think mocking is a great thing, but the auto-generated mocks and associated macros in gmock are not always so intuitive.  Maybe it is just an education thing, but I'd really prefer if our codebase were as intuitive as possible.

Maybe we just need good rules for when to use gmock and when not to use it.

-Darin

[John Abd-El-Malek]

Without knowing more about your tests, it seems that this can also be done by adding an abstraction layer around the code which calls the GL driver. Tests can swap in a different implementation.

The issue with gmock is that more often than not it is abused. If we sit in a limbo state where existing usage is forever maintained, we end up with the worst of both worlds (maintenance/readability/poor testing of gmock still in the code, while people have to support two different ways of testing).

I'll give a few examples of things that we've tried to stop using, and there were never enough time to fix:

-ScopedAllowIO: it was agreed that writing to disk on UI/IO is bad, and we banned that and added ScopedAllowIO temporarily for the existing usage. In the past few years, instead of these being fixed, we went from ~20 to ~90 instances. It's a class tragedy of the commons.

-ui_tests: we added a much better testing framework in the form of browser_tests. We sat for years supporting both. Nearly half of them were disabled. I spent 2 months earlier this year rewriting them to browser_tests.

From this experience, and other constructs like this as well, it has always been too low of a priority to switch to the new system. The only way to enforce this is by setting a deadline. This deadline doesn't have to be near-term, but as long as it's set and agreed to, then developers have flexibility on finding the time to do this.

On Wed, Jun 27, 2012 at 3:51 PM, Gregg Tavares (社用) <gm...@google.com> wrote:

[James Robinson]

On Wed, Jun 27, 2012 at 4:04 PM, Darin Fisher <da...@chromium.org> wrote:

Well, I'd like to study the good uses of gmock to learn more.  It sounds like you may have one.  Personally, I've only seen bad uses, so my opinion is pretty biased :-/

To use an example from WebKit, I've found gmock to be helpful when writing these tests: http://trac.webkit.org/browser/trunk/Source/WebKit/chromium/tests/WebCompositorInputHandlerImplTest.cpp for this class: http://trac.webkit.org/browser/trunk/Source/WebKit/chromium/src/WebCompositorInputHandlerImpl.cpp.  The code is small (few hundred lines of logic) but rather dense and the tests are fairly small.

Would folks mind looking over my terrible code and seeing if this is a good or bad use of gmock and, if not, how tests for this should be constructed?  I think having something concrete to discuss is more helpful than the abstract.  I'd be happy to attempt rewriting this test in some other form if people think it would be an improvement and update the thread with my experience.

- James

[Darin Fisher]

Hmm, perhaps the problem occurs when folks try to mock concrete classes.  It seems like mocking interfaces is a good thing, and gmock simply helps one create mocks.  Perhaps this thread just boils down to us needing to be more disciplined about introducing proper interfaces to help separate classes (or groups of classes).  Perhaps there are also some gmock anti-patterns to identify.

[Andrew Scherkus]

My experience with gmock is that they're very awesome tests that make you feel all warm and fuzzy inside (hey look! everything was called correctly!) until the next person comes along and touches the code.

I'll offer an example of code I wrote that I absolutely loved at the time when I wrote it but grown to hate:

http://src.chromium.org/viewvc/chrome/trunk/src/webkit/media/buffered_resource_loader_unittest.cc?view=markup

http://src.chromium.org/viewvc/chrome/trunk/src/webkit/mocks/mock_weburlloader.h?view=markup

I didn't want to use the "real" implementation of WebURLLoader (AssociatedURLLoader) because doing so required that I set up a WebFrame etc... when all I wanted to do is simulate a few tricky HTTP responses. Seems easy enough task for gmock, right?

In reality, this test is an integration test between my code and AssociatedURLLoader (even though there's an interface between the two) and I should have identified some other way to simulate those tricky HTTP responses without using a mocked WebURLLoader. I've been bitten by bugs where AssociatedURLLoader has changed but my mock failed to reflect the new behaviour (not good!).

Andrew

[Darin Fisher]

Hmm.. in that case WebURLLoader is an "input" interface.  Perhaps we should look at the differences between mocking output and input interfaces?  It seems like in gman's example of wanting to monitor resultant GL calls that what we have is a pure output mock example.  Hmm...

[Harald Alvestrand]

I find mockers extremely useful.

Before gMock, I rolled my own mocks, with my own verification mechanisms. I was 100% guaranteed that nobody else could figure out what they did without actually reading the code for those mocks. After gMock, I can put most of the stuff someone reading the test needs to know in the test itself.

gMock is extremely useful for smoking out places where you think you have an interface, but actually have a dependency on an implementation, or you think you have an unit test, but in fact have an integration test (because it depends on half a dozen other classes that can't be dependency-injected or mocked).

There are things I don't like about gMock - in particular the need to declare expectations for stuff you don't care about in order to avoid log spam; if I do it restrictively, the tests are brittle; if I do it in a lax way, I need comments to reassure my reviewer that I've got reasons for being lax.

But I don't want to go back to not using *any* mocking framework, and I don't have a better candidate.

             Harald

[Mattias Nissler]

On Thu, Jun 28, 2012 at 5:18 AM, Darin Fisher <da...@google.com> wrote:

Hmm.. in that case WebURLLoader is an "input" interface.  Perhaps we should look at the differences between mocking output and input interfaces?  It seems like in gman's example of wanting to monitor resultant GL calls that what we have is a pure output mock example.  Hmm...

Many interfaces have both input and output parts, what about them?

 

On Jun 27, 2012 7:58 PM, "Andrew Scherkus" <sche...@chromium.org> wrote:

My experience with gmock is that they're very awesome tests that make you feel all warm and fuzzy inside (hey look! everything was called correctly!) until the next person comes along and touches the code.

I'll offer an example of code I wrote that I absolutely loved at the time when I wrote it but grown to hate:

http://src.chromium.org/viewvc/chrome/trunk/src/webkit/media/buffered_resource_loader_unittest.cc?view=markup

http://src.chromium.org/viewvc/chrome/trunk/src/webkit/mocks/mock_weburlloader.h?view=markup

I didn't want to use the "real" implementation of WebURLLoader (AssociatedURLLoader) because doing so required that I set up a WebFrame etc... when all I wanted to do is simulate a few tricky HTTP responses. Seems easy enough task for gmock, right?

In reality, this test is an integration test between my code and AssociatedURLLoader (even though there's an interface between the two) and I should have identified some other way to simulate those tricky HTTP responses without using a mocked WebURLLoader. I've been bitten by bugs where AssociatedURLLoader has changed but my mock failed to reflect the new behaviour (not good!).

That's true for any mock-based testing. If you change the semantics of the interface that you mock out, you'll have to update the mocks regardless of you whether you use gmock or not.

 

Andrew

On Wed, Jun 27, 2012 at 7:31 PM, Darin Fisher <da...@chromium.org> wrote:

Hmm, perhaps the problem occurs when folks try to mock concrete classes.  It seems like mocking interfaces is a good thing, and gmock simply helps one create mocks. 

+1 to that. If you look at chrome/browser/policy, which is one of the areas I care about, you'll see that we use gmock-generated mocks mostly for mocking out interface dependencies passed to the constructor of the class that we're testing.

One example I very much like is this: Testing that an observer gets notified at the right times. With gmock, you generate a mock implementation of the observer and set up your expectations. I find that much simpler than implementing a custom mock observer that either (a) needs to be told what it's supposed to expect or (b) records all events, requiring you to look at the state afterwards to determine whether the right events fired.

I agree that gmock-ing concrete classes is a bad idea in many cases as you'll get a weird mix of mocked and non-mocked behavior.

Perhaps this thread just boils down to us needing to be more disciplined about introducing proper interfaces to help separate classes (or groups of classes).  Perhaps there are also some gmock anti-patterns to identify.

On Jun 27, 2012 4:20 PM, "James Robinson" <jam...@google.com> wrote:

On Wed, Jun 27, 2012 at 4:04 PM, Darin Fisher <da...@chromium.org> wrote:

Well, I'd like to study the good uses of gmock to learn more.  It sounds like you may have one.  Personally, I've only seen bad uses, so my opinion is pretty biased :-/

To use an example from WebKit, I've found gmock to be helpful when writing these tests: http://trac.webkit.org/browser/trunk/Source/WebKit/chromium/tests/WebCompositorInputHandlerImplTest.cpp for this class: http://trac.webkit.org/browser/trunk/Source/WebKit/chromium/src/WebCompositorInputHandlerImpl.cpp.  The code is small (few hundred lines of logic) but rather dense and the tests are fairly small.

Would folks mind looking over my terrible code and seeing if this is a good or bad use of gmock and, if not, how tests for this should be constructed?  I think having something concrete to discuss is more helpful than the abstract.  I'd be happy to attempt rewriting this test in some other form if people think it would be an improvement and update the thread with my experience.

- James

I think mocking is a great thing, but the auto-generated mocks and associated macros in gmock are not always so intuitive.  Maybe it is just an education thing, but I'd really prefer if our codebase were as intuitive as possible.

Maybe we just need good rules for when to use gmock and when not to use it.

-Darin

On Wed, Jun 27, 2012 at 3:51 PM, Gregg Tavares (社用) <gm...@google.com> wrote:

So what are we suggesting should be done instead? If you have a use case that exactly matches gmock then why not use it? If people are using it wrong that should be reflected in reviews but outright banning it seems a little extreme.

...confused...

On Wed, Jun 27, 2012 at 3:41 PM, Darin Fisher <da...@chromium.org> wrote:

+1 to banning gmock.  Not sure about how the process would go for migrating existing tests.  Hmm...

On Wed, Jun 27, 2012 at 12:11 PM, John Abd-El-Malek <j...@chromium.org> wrote:

+1

You've put this in much better terms than I would have. My impression with gmock has been that it's misused in most test. A lot of the places I see it used are calling foo() and ensuring that bar() is called. These tests are not that useful. It seems to me that many pieces of the code that have good interfaces around each component get by with thorough unit tests without a need for gmock. But classes that are heavily intertwined and which pass concrete objects as delegates, instead of interfaces, use gmock instead of adding proper interfaces to separate components.

I see the main benefit of gmock in being able to easily create mocks for interfaces, instead of having to roll a mock implementation of my own every time or maintain a generic mock implementation that can become complex very quickly.

 

gmock is also not that readable and adds a maintenance burden. Even if someone who writes a test knows it well, it imposes a cost on everyone else who wants to touch a code that's covered by gmock, since they might have to change a 7 line gmock statement.

I'm very supportive of banning gmock from any more tests. If we just say existing usage are fine, then they'll never be converted though. We would need to set a deadline, say a quarter or two, after which these tests would be disabled.

On Wed, Jun 27, 2012 at 11:26 AM, Albert J. Wong (王重傑) <ajw...@chromium.org> wrote:

I added gmock because I saw it being used very effectively inside Google for creating small, surgical, unittests. Unfortunately, I think it hasn't been working nicely within Chromium.

My observation has been that many Chromium "unittests" are really integration tests that exercise multiple classes with wide interfaces and complex interactions. As such, it is hard to really mock out the whole interface without adding a bunch of behavior into the mock.  Thus, the "mock object" ceases being a mock and becomes a fragile, hard to read, "fake". (definition of terms: http://googletesting.blogspot.com/2008/06/tott-friends-you-can-depend-on.html)

Even worse, because it is easy to add behavior into a Gmock object, I've seen people script a mock with a long sequence of calls/results.  The test now has moved beyond asserting invariants about API behavior to asserting the specific call sequence the current implementation has. (aka a Characterization Test).

Gmock is a framework for removing boilerplate out of unittests that are structured around the use of mock objects. It really only works well in this usage. Since we don't follow a purist "mock-based unittest" philosophy, my gut feeling is that Gmock adds more complexity than it saves and can encourage bad testing patterns (eg., characterization test, or partial mocking of objects).

I think there's lots of truth in this analysis, but I wouldn't draw the conclusion that gmock is inherently a bad idea. Let me elaborate: I agree that gmock-generated mocks are usually not the right solution for testing complex interactions of lots of classes, because the mock becomes too complex and thus require crazy call expectation and action sequences if implemented in gmock. However, for writing focused unit tests of classes that use clean interfaces for their dependencies, gmock is hugely helpful to quickly declare mocks for the dependencies.

Given that we seem to agree that we want to have integration-level tests that test complex interaction of a set of classes, we should then ask ourselves the question whether in addition to integration tests we want to have "small, surgical unittests" (as Albert put it very aptly) for individual classes or not. Thoughts?

Personally, I'm very much in favor of testing at the class level as well, because you'll be forced to think about interaction between the class you implement and the rest of the world, which in turn will help you understand the invariants you put in and document them in the test. Also, in my experience you'll get small and focused tests when you follow a test-driven development approach for free.

[Greg Thompson]

On Thu, Jun 28, 2012 at 5:38 AM, Harald Alvestrand <h...@google.com> wrote:

There are things I don't like about gMock - in particular the need to declare expectations for stuff you don't care about in order to avoid log spam; if I do it restrictively, the tests are brittle; if I do it in a lax way, I need comments to reassure my reviewer that I've got reasons for being lax.

Have you looked at NiceMock to suppress the log spam?

On the larger topic: many of the comments in this thread boil down to the fact that white box tests (those that test implementation rather than interface) are inherently fragile.  Perhaps GMock makes it easier to inadvertently write a white box test rather than a black box test.  For white box tests, I think it works pretty well.

[Ben Goodger (Google)]

On Thu, Jun 28, 2012 at 5:08 AM, Mattias Nissler <mnis...@chromium.org> wrote:

One example I very much like is this: Testing that an observer gets notified at the right times. With gmock, you generate a mock implementation of the observer and set up your expectations. I find that much simpler than implementing a custom mock observer that either (a) needs to be told what it's supposed to expect or (b) records all events, requiring you to look at the state afterwards to determine whether the right events fired.

I agree that gmock-ing concrete classes is a bad idea in many cases as you'll get a weird mix of mocked and non-mocked behavior.

I can see how this would be convenient. Perhaps we should start with a restriction on mocking classes that are not designed to be subclassed?

-Ben

[Drew Wilson]

On Thu, Jun 28, 2012 at 5:08 AM, Mattias Nissler <mnis...@chromium.org> wrote:

On Thu, Jun 28, 2012 at 5:18 AM, Darin Fisher <da...@google.com> wrote:

Hmm.. in that case WebURLLoader is an "input" interface.  Perhaps we should look at the differences between mocking output and input interfaces?  It seems like in gman's example of wanting to monitor resultant GL calls that what we have is a pure output mock example.  Hmm...

Many interfaces have both input and output parts, what about them?

 

On Jun 27, 2012 7:58 PM, "Andrew Scherkus" <sche...@chromium.org> wrote:

My experience with gmock is that they're very awesome tests that make you feel all warm and fuzzy inside (hey look! everything was called correctly!) until the next person comes along and touches the code.

I'll offer an example of code I wrote that I absolutely loved at the time when I wrote it but grown to hate:

http://src.chromium.org/viewvc/chrome/trunk/src/webkit/media/buffered_resource_loader_unittest.cc?view=markup

http://src.chromium.org/viewvc/chrome/trunk/src/webkit/mocks/mock_weburlloader.h?view=markup

I didn't want to use the "real" implementation of WebURLLoader (AssociatedURLLoader) because doing so required that I set up a WebFrame etc... when all I wanted to do is simulate a few tricky HTTP responses. Seems easy enough task for gmock, right?

In reality, this test is an integration test between my code and AssociatedURLLoader (even though there's an interface between the two) and I should have identified some other way to simulate those tricky HTTP responses without using a mocked WebURLLoader. I've been bitten by bugs where AssociatedURLLoader has changed but my mock failed to reflect the new behaviour (not good!).

That's true for any mock-based testing. If you change the semantics of the interface that you mock out, you'll have to update the mocks regardless of you whether you use gmock or not.

I'd say the major drawback to using gmock is there's no natural migration path from building a mock with gmock to building a fake. As a result, people start out using gmock when they are building tests, and as they add more complex tests, they naturally evolve into a situation where they need a fake instead, but at any given point it's always easier to just add a few more call expectations than to stop using gmock and rewrite using a fake. It's like a built-in anti-pattern.

I find that when using my own, hand-rolled mock, it's generally easier to migrate this to being a fake. So for that reason, I'd support banning gmock for new tests, even though there's *slightly* more work in cases where you truly only need a mock.

[Drew Wilson]

On Thu, Jun 28, 2012 at 9:20 AM, Drew Wilson <atwi...@chromium.org> wrote:

On Thu, Jun 28, 2012 at 5:08 AM, Mattias Nissler <mnis...@chromium.org> wrote:

On Thu, Jun 28, 2012 at 5:18 AM, Darin Fisher <da...@google.com> wrote:

Hmm.. in that case WebURLLoader is an "input" interface.  Perhaps we should look at the differences between mocking output and input interfaces?  It seems like in gman's example of wanting to monitor resultant GL calls that what we have is a pure output mock example.  Hmm...

Many interfaces have both input and output parts, what about them?

 

On Jun 27, 2012 7:58 PM, "Andrew Scherkus" <sche...@chromium.org> wrote:

My experience with gmock is that they're very awesome tests that make you feel all warm and fuzzy inside (hey look! everything was called correctly!) until the next person comes along and touches the code.

I'll offer an example of code I wrote that I absolutely loved at the time when I wrote it but grown to hate:

http://src.chromium.org/viewvc/chrome/trunk/src/webkit/media/buffered_resource_loader_unittest.cc?view=markup

http://src.chromium.org/viewvc/chrome/trunk/src/webkit/mocks/mock_weburlloader.h?view=markup

I didn't want to use the "real" implementation of WebURLLoader (AssociatedURLLoader) because doing so required that I set up a WebFrame etc... when all I wanted to do is simulate a few tricky HTTP responses. Seems easy enough task for gmock, right?

In reality, this test is an integration test between my code and AssociatedURLLoader (even though there's an interface between the two) and I should have identified some other way to simulate those tricky HTTP responses without using a mocked WebURLLoader. I've been bitten by bugs where AssociatedURLLoader has changed but my mock failed to reflect the new behaviour (not good!).

That's true for any mock-based testing. If you change the semantics of the interface that you mock out, you'll have to update the mocks regardless of you whether you use gmock or not.

I'd say the major drawback to using gmock is there's no natural migration path from building a mock with gmock to building a fake. As a result, people start out using gmock when they are building tests, and as they add more complex tests, they naturally evolve into a situation where they need a fake instead, but at any given point it's always easier to just add a few more call expectations than to stop using gmock and rewrite using a fake. It's like a built-in anti-pattern.

I find that when using my own, hand-rolled mock, it's generally easier to migrate this to being a fake. So for that reason, I'd support banning gmock for new tests, even though there's *slightly* more work in cases where you truly only need a mock.

As an example of the former, take a look at:

http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/webui/sync_setup_handler_unittest.cc

This test started out with pretty simple "let's have ProfileSyncService just say it's not logged in yet" behavior:

  EXPECT_CALL(*mock_pss_, sync_initialized()).WillRepeatedly(Return(false));

 But then as we wanted to build more complex tests (like "let's start out not-signed-in, then sign in") it's just soooooo easy to do this:

  EXPECT_CALL(*mock_pss_, sync_initialized())
      .WillOnce(Return(false))
      .WillRepeatedly(Return(true));

 But now you have this brittle code that relies on the calling class only calling "sync_initialized()" once prior to signin. It's clear that there are better ways to do this, but there are no *easier* ways to do this, and I think that's my biggest criticism of gmock - the easiest way to solve a given problem with gmock is often not the right way, and that's a dangerous trait in a framework.

[Mattias Nissler]

That's not true. You first say:

EXPECT_CALL(*mock_pss_, sync_initialized()).WillRepeatedly(Return(false));

at the point where your test proceeded to the logged in state, you say:

EXPECT_CALL(*mock_pss_, sync_initialized()).WillRepeatedly(Return(true));

And that's much better as it allows you to put that statement at exactly the point where you have switched to logged-in state.

[Fred Akalin]

On Thu, Jun 28, 2012 at 10:13 AM, Mattias Nissler <mnis...@google.com> wrote:

That's not true. You first say:

EXPECT_CALL(*mock_pss_, sync_initialized()).WillRepeatedly(Return(false));

at the point where your test proceeded to the logged in state, you say:

EXPECT_CALL(*mock_pss_, sync_initialized()).WillRepeatedly(Return(true));

And that's much better as it allows you to put that statement at exactly the point where you have switched to logged-in state.

It's not obvious, but you can't do that.  From http://code.google.com/p/googlemock/wiki/ForDummies :

Important note: Google Mock requires expectations to be set before the mock functions are called, otherwise the behavior is undefined. In particular, you mustn't interleave EXPECT_CALL()s and calls to the mock functions.

[Mattias Nissler]

Aha, I wasn't aware of that advice in the guide. A while back, I actually read the gmock source and figured that it's safe to use this pattern in certain cases. In the general case, I think it's good if you throw in a

Mock::VerifyAndClearExpectations(mock_pss_);

line before you configure new expectations, which I understand essentially resets the mock the fresh state, so you can start over declaring new expectations and making new calls. I'll still double-check this.

[Mattias Nissler]

I checked with the googlemock author and he confirmed that once you put in the Mock::VerifyAndClearExpectations call, it's safe. 

[Albert J. Wong (王重傑)]

Pulling up back up a level, I think it's also uninteresting to debate, in a vacuum, whether or not GMock itself is a learnable/good tool.

Google has internally used it for years over a large complex C++ code base in a manner that keeps test maintainable, easy to understand, and fast.  In Chromium, though we clearly have some happy users, this has not been the case in a number of situations.  This says the issues are not intrinsic to GMock, but in how it interacts with Chromium's coding and testing style.

From this thread, the main positives I hear are that 

  a) it gives a consistent, easy, method for creating mocks.

  b) since it is consistent, it can be easier to read that ad-hoc mock implementations

The negatives are:

  a) it has been often misapplied in situations where mocks are inappropriate (eg., integration tests).

  b) as tests grow, people just add complexity to the mock rather than refactor the API to be testable.

  c) people who aren't versed in it have to learn a new meta-language.

For (a), I think we could probably successfully address this with a set of anti-patterns as Darin suggested.  Before creating this set though, I think we should settle issues (b) and (c) because they are harder to solve.

For (b), this is more of a question of reviewer and developer discipline with writing tests rather than a GMock issue.

Test are effectively a set of assertions (implicit or explicit) about what behavior you want to guarantee in your production code.  If your and you reviewer aren't asking "oh, you just added another assumption about behavior into this test...am I okay with that?" we've already got a problem. This is true with or without GMock.

There is an argument that GMock makes it easier to add too much behavior, but I'd venture the fundamental issue is not thinking that all code in a test is effectively an assertion about behavior.

Creating adding a set of anti-patterns, and having had this uber-thread as negative-reinforcement, probably goes a long way to solving this issue.

As for (c), requiring developers to learn the meta-language, this was the kicker that made me think GMock might not be worth its weight. It means that GMock, as a tool, is a burden on users and non-users alike.  I think this is actually the core issue to evaluate.

Do we want to retain this tax in order to have a consistent mock system inside our "true" unittests?

-Albert

[Mattias Nissler]

On Thu, Jun 28, 2012 at 8:44 PM, Albert J. Wong (王重傑) <ajw...@chromium.org> wrote:

Pulling up back up a level, I think it's also uninteresting to debate, in a vacuum, whether or not GMock itself is a learnable/good tool.

Google has internally used it for years over a large complex C++ code base in a manner that keeps test maintainable, easy to understand, and fast.  In Chromium, though we clearly have some happy users, this has not been the case in a number of situations.  This says the issues are not intrinsic to GMock, but in how it interacts with Chromium's coding and testing style.

From this thread, the main positives I hear are that 

  a) it gives a consistent, easy, method for creating mocks.

  b) since it is consistent, it can be easier to read that ad-hoc mock implementations

The negatives are:

  a) it has been often misapplied in situations where mocks are inappropriate (eg., integration tests).

  b) as tests grow, people just add complexity to the mock rather than refactor the API to be testable.

  c) people who aren't versed in it have to learn a new meta-language.

For (a), I think we could probably successfully address this with a set of anti-patterns as Darin suggested.  Before creating this set though, I think we should settle issues (b) and (c) because they are harder to solve.

For (b), this is more of a question of reviewer and developer discipline with writing tests rather than a GMock issue.

Test are effectively a set of assertions (implicit or explicit) about what behavior you want to guarantee in your production code.  If your and you reviewer aren't asking "oh, you just added another assumption about behavior into this test...am I okay with that?" we've already got a problem. This is true with or without GMock.

There is an argument that GMock makes it easier to add too much behavior, but I'd venture the fundamental issue is not thinking that all code in a test is effectively an assertion about behavior.

Creating adding a set of anti-patterns, and having had this uber-thread as negative-reinforcement, probably goes a long way to solving this issue.

As for (c), requiring developers to learn the meta-language, this was the kicker that made me think GMock might not be worth its weight. It means that GMock, as a tool, is a burden on users and non-users alike.  I think this is actually the core issue to evaluate.

Do we want to retain this tax in order to have a consistent mock system inside our "true" unittests?

So it's down to deciding what is the bigger burden: Having to learn gmock or having to understand custom mock implementations for each new existing unittest you're looking at (and FWIW my experience is I have to look at a lot of them when changing something non-trivial). If we use gmock responsibly, the former is hopefully a one-time effort, while the latter you cannot ever get done with.

If we don't abuse gmock for complex stuff, the call expectations should hopefully be rather straightforward to understand thus reducing the gmock-WTF-ness for newcomers.

In general, I think the rule should be to only use gmock if the effort to understand the gmock solution is lower or comparable to the effort to understand the hand-coded mock (assuming you have a basic idea what EXPECT_CALL does).

[Scott Hess]

I think an issue with this approach is that gmock is somewhat
comparable to using C++ templates. A master craftsman can use C++
templates to build a relatively sane interface which developers
untutored in templates can use very successfully. But often when
templates are used in more one-off fashion, it can be more challenging
to figure out what is going on simply because many/most developers
don't use templates enough to internalize the knowledge.

Likewise, if I am having to touch some test code because of a change I
made which is outside the generally-accepted scope of that code, I
would far rather have to read a 25-line function written in
straightforward C++ versus a single gmock statement which is broken
across 8 or 10 lines. The gmock statement may be shorter, and may be
easier to understand for someone experienced with gmock, but you can't
assume that Chromium developers are experienced with gmock, while you
can assume they are experienced with C++.

-scott

[Drew Wilson]

On Thu, Jun 28, 2012 at 10:56 AM, Mattias Nissler <mnis...@google.com> wrote:

On Thu, Jun 28, 2012 at 7:38 PM, Fred Akalin <aka...@chromium.org> wrote:

On Thu, Jun 28, 2012 at 10:13 AM, Mattias Nissler <mnis...@google.com> wrote:

That's not true. You first say:

EXPECT_CALL(*mock_pss_, sync_initialized()).WillRepeatedly(Return(false));

at the point where your test proceeded to the logged in state, you say:

EXPECT_CALL(*mock_pss_, sync_initialized()).WillRepeatedly(Return(true));

And that's much better as it allows you to put that statement at exactly the point where you have switched to logged-in state.

It's not obvious, but you can't do that.  From http://code.google.com/p/googlemock/wiki/ForDummies :

Important note: Google Mock requires expectations to be set before the mock functions are called, otherwise the behavior is undefined. In particular, you mustn't interleave EXPECT_CALL()s and calls to the mock functions.

Aha, I wasn't aware of that advice in the guide. A while back, I actually read the gmock source and figured that it's safe to use this pattern in certain cases. In the general case, I think it's good if you throw in a

Mock::VerifyAndClearExpectations(mock_pss_);

And I've done this as well. The problem then being that you have to verify and reset *all* expectations (which is a bummer if you are using common code to setup your expectations for actions taken later in the object's lifecycle, since those expectations won't be satisfied yet). Basically, you can't just verify and reset expectations for a single API function. Again, not a huge issue, but it's clunky, and the *easiest* solution in gmock (just add a single "WillOnce" clause rather than resetting all expectations) is not the *right* one.

Anyhow, I'm fine with whatever we decide to do with gmock. I recall that google3 has its own share of misuses of gmock, so I'm pessimistic that somehow we're going to educate/review our way towards no further misuses in Chromium, but I don't think it's such a critical issue that I feel dogmatic about it.

[Mattias Nissler]

-----------------------------------------------------------

Google Germany GmbH

Dienerstr. 12

80331 München

AG Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Graham Law, Katherine Stephens

Tax ID:- 48/725/00206 

VAT ID:- DE813741370

On Fri, Jun 29, 2012 at 3:06 AM, Drew Wilson <atwi...@chromium.org> wrote:

On Thu, Jun 28, 2012 at 10:56 AM, Mattias Nissler <mnis...@google.com> wrote:

On Thu, Jun 28, 2012 at 7:38 PM, Fred Akalin <aka...@chromium.org> wrote:

On Thu, Jun 28, 2012 at 10:13 AM, Mattias Nissler <mnis...@google.com> wrote:

That's not true. You first say:

EXPECT_CALL(*mock_pss_, sync_initialized()).WillRepeatedly(Return(false));

at the point where your test proceeded to the logged in state, you say:

EXPECT_CALL(*mock_pss_, sync_initialized()).WillRepeatedly(Return(true));

And that's much better as it allows you to put that statement at exactly the point where you have switched to logged-in state.

It's not obvious, but you can't do that.  From http://code.google.com/p/googlemock/wiki/ForDummies :

Important note: Google Mock requires expectations to be set before the mock functions are called, otherwise the behavior is undefined. In particular, you mustn't interleave EXPECT_CALL()s and calls to the mock functions.

Aha, I wasn't aware of that advice in the guide. A while back, I actually read the gmock source and figured that it's safe to use this pattern in certain cases. In the general case, I think it's good if you throw in a

Mock::VerifyAndClearExpectations(mock_pss_);

And I've done this as well. The problem then being that you have to verify and reset *all* expectations (which is a bummer if you are using common code to setup your expectations for actions taken later in the object's lifecycle, since those expectations won't be satisfied yet). Basically, you can't just verify and reset expectations for a single API function. Again, not a huge issue, but it's clunky, and the *easiest* solution in gmock (just add a single "WillOnce" clause rather than resetting all expectations) is not the *right* one.

Good point. 

Anyhow, I'm fine with whatever we decide to do with gmock. I recall that google3 has its own share of misuses of gmock, so I'm pessimistic that somehow we're going to educate/review our way towards no further misuses in Chromium, but I don't think it's such a critical issue that I feel dogmatic about it.

I agree. I've made all the points I wanted to make. Happy to accept whatever informed decision we make.

[Dominic Mazzoni]

I used gmock for one test file in Chromium. It made sense for me at the time because I had written several tests using gmock at Google before, and it was unquestionably more concise than not using it.

What about a more limited alternative to gmock that saves everyone from rolling their own mock objects but eliminates 90% of the complexity?

For example, suppose I have the following interface:

class InputFile {

  bool Open(const char* filename) = 0;

  bool Read(size_t count, void* data) = 0;

  void Close() = 0;
};

Gmock lets me write this:

class MockInputFile : public InputFile {

  MOCK_METHOD1(Open, bool(const char *));

  MOCK_METHOD2(Read, bool(size_t, void*))

  MOCK_METHOD0(Close, void);

};

I think this is pretty concise, easy to read, and useful. In particular, for interfaces that have a large number of methods, it's hard to imagine anything more concise than this. I think it be pretty cool if we could preserve this part.

However, gmock also provides zillions of predicates and complicated matching functions that you can use to assert what happens when you call one of these methods. This is where most of the complexity and potential trouble comes from. What if we threw that out and instead just kept track of what methods were called in a really simple data structure, and let each test write whatever assertions made sense?

- Dominic

[Munjal Doshi]

I think gmock is one of those things which can be very useful but is very easy to abuse.

But I have used it in tests, in Chrome and in google3, to great effect. So I would not vote for an outright ban but would support some ways to avoid the ab-use. I totally agree that GMock makes it easy to exercise some corner cases and increase test coverage (e.g. mock url fetcher, set it to respond with 400. I know URL fetcher has good test classes, but this is still much easier to setup).

Several things I have noticed in Chromium code in bad uses of GMock:

- Lack of shared mocks for some very common classes

- Use of gmock for integration tests (which is inappropriate in most cases IMHO).

It might be useful to do some case studies of simple and complex components, and see what is the best way to do unit testing and integration testing of those.

-Munjal

[Fred Akalin]

On Tue, Jul 3, 2012 at 3:48 PM, Munjal Doshi <mun...@chromium.org> wrote:

I think gmock is one of those things which can be very useful but is very easy to abuse.

But I have used it in tests, in Chrome and in google3, to great effect. So I would not vote for an outright ban but would support some ways to avoid the ab-use. I totally agree that GMock makes it easy to exercise some corner cases and increase test coverage (e.g. mock url fetcher, set it to respond with 400. I know URL fetcher has good test classes, but this is still much easier to setup).

But isn't this itself an abuse, especially if URLFetcher already has test classes?  You want a stub (see the link in Albert's original e-mail for definitions) but you're using a mock library instead.

 

Several things I have noticed in Chromium code in bad uses of GMock:

- Lack of shared mocks for some very common classes

Wouldn't shared mocks for common classes encourage people to use those mocks even when inappropriate?  It's very easy to go "oh, look, there's already a mock for this" and use that when something else might be a better solution.

[Chris Bentzel]

Did chromium-dev ever reach concensus about gmock?

I am writing a test which could use gmock (an interface is dependency injected into an object, and I want to set up expectations and results). For now, I'll just roll my own test implementation of the interface, but gmock would reduce the amount of boilerplate here.

--

[Mattias Nissler]

I think there was agreement was that for some cases it's considered useful (the interface you're mocking should be pure virtual and the mock setups should be straightforward enough), while in other cases manually created "fakes" are better (e.g. for mocking complex behavior or doing special for-tests class specializations).

[Wez]

+1