Proposal to ban Gmock in Blink

[Albert J. Wong (王重傑)]

Since Adam brought up unit tests in another thread, I'd like to propose, right off the bat, banning Gmock.

Gmock is a C++ mock object framework similar to EasyMock in Java.

There is a huge appeal to having a good mocking framework to reduce boilerplate, and Gmock can be wonderful when used well.  However, experience in Chromium has shown that the class APIs there were often too wide and object relations were too tangled for effective use of Gmock. What ended up happening is unit test writers used Gmock as an obfuscated method for creating "fakes" rather than "mocks." This lead to test code that was fragile, hard to maintain, and hard to read.  Here is a relevant chromium-dev thread.

Blink's class structures and APIs feel as wide and interconnected as Chromium's so I believe that Gmock would likely meet similar abuse in this project.  There might be a point when it becomes clear that a generalized mock framework would aid unit testing. At that point, it would also likely be clear when mocks are wanted instead of fakes and Gmock is probably the best candidate for a C++ mocking framework.  However, until that time, I suggest banning Gmock.

-Albert

P.S. Full disclosure: I'm the one that added Gmock into Chromium. I actually love Gmock in isolation but something about its application in Chromium just went bad.

[James Robinson]

On Mon, Apr 22, 2013 at 4:15 PM, Albert J. Wong (王重傑) <ajw...@chromium.org> wrote:

Since Adam brought up unit tests in another thread, I'd like to propose, right off the bat, banning Gmock.

Gmock is a C++ mock object framework similar to EasyMock in Java.

There is a huge appeal to having a good mocking framework to reduce boilerplate, and Gmock can be wonderful when used well.  However, experience in Chromium has shown that the class APIs there were often too wide and object relations were too tangled for effective use of Gmock. What ended up happening is unit test writers used Gmock as an obfuscated method for creating "fakes" rather than "mocks." This lead to test code that was fragile, hard to maintain, and hard to read.  Here is a relevant chromium-dev thread.

Blink's class structures and APIs feel as wide and interconnected as Chromium's so I believe that Gmock would likely meet similar abuse in this project.  There might be a point when it becomes clear that a generalized mock framework would aid unit testing. At that point, it would also likely be clear when mocks are wanted instead of fakes and Gmock is probably the best candidate for a C++ mocking framework.  However, until that time, I suggest banning Gmock.

What do you suggest for the unit tests in place in Blink that are currently using gmock?

(It sounds like you suspect we're starting from a clean slate here in Blink for unit testing, but we aren't.  By my rough count we have about 30k lines of C++ unit tests in the tree today).

- James

[Brett Wilson]

+1 to not using Gmock.

I only count 4 test files that currently use it. Maybe we can
eventually rewrite them, but for the foreseeable future we can at
least avoid adding more.

Brett

[James Robinson]

Personally I think Gmock is fine when it's used to actually mock out an interface in a unit test, as opposed to creating a fake.  It definitely can be hard to use at times but hand-rolled equivalents are no better, in my experience.

Maybe in Blink we should try to have a stronger distinction between integration tests and unit tests?  Gmock never makes sense in integration tests, but it can be useful in actual unit tests.  The observations from the chromium-dev@ thread appeared mostly to be based on the assumption that most tests were bigger than true unit tests.  We could have different targets with different include rules for unit vs integration tests.

I would hope that as we add more tests in Blink we can have good discipline about what is a unit test and what isn't and refactor things to make unit testing possible.

- James

[Albert J. Wong (王重傑)]

Agreed.  The problem isn't Gmock itself.  When used to produce a simple mock, Gmock is actually quite good aside from the medium learning curve.  I used to use it a lot for internal Google code and it worked great there.

It's possible that the problems Chromium had could be avoided with better reviews and education on proper unit testing, mocking, and Gmock anti-patterns.  Someone would have to figure out how to get all that in place though.

-Albert

[Peter Kasting]

On Mon, Apr 22, 2013 at 5:08 PM, Albert J. Wong (王重傑) <ajw...@chromium.org> wrote:

Agreed.  The problem isn't Gmock itself.  When used to produce a simple mock, Gmock is actually quite good aside from the medium learning curve.  I used to use it a lot for internal Google code and it worked great there.

It's possible that the problems Chromium had could be avoided with better reviews and education on proper unit testing, mocking, and Gmock anti-patterns.  Someone would have to figure out how to get all that in place though.

I basically agree with this.  I'm worried about "solving" the problem in Blink by banning gmock when not only is gmock itself not really the problem but Chromium doesn't ban gmock.  I would prefer a solution that (a) moves towards harmony between Chromium and Blink and (b) solves the Chromium problems as well.

PK

[Mattias Nissler]

Maybe we should add a presubmit that enforces review by a to-be-defined group of people experienced with gmock's peculiarities for all changes that introduce/modify gmock usage? Admittedly, that's slightly on the heavy-weight process side, but it'd (a) discourage people to abuse gmock without thinking and (b) help spread gmock best practices in the team.

PK