Any objections to my importing GoogleMock to go with GoogleTest in LLVM?

I have some concrete use cases in testing the pass manager where it will allow the tests of this API to be more thorough, less verbose, and easier to maintain. I’m not claiming to be the biggest fan of some features in GoogleMock, but on the whole, I think it’s better than the alternative and will allow more careful testing of C++ APIs where the interesting part is the API itself.

Could you maybe give an example or two to whet our testing appetite?

Emphatic +1

One of the major problems with unit testing (in general and in LLVM in
particular) is that it's difficult to isolate coupled components from each
other. A good mocking framework makes this much easier and thus can help us
create more unit tests in the long run. GoogleMock is popular,
battle-proven and is the natural companion for GTest.

Eli

I don’t really have a strong opinion, but it ended up being fairly controversial in Chromium:
https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/-KH_IP0rIWQ

It might occupy a space like GTest does for us today, which is used for things like Support where we often can’t write a good lit test.

Personally, I rather not do this, without very clear and compelling reasons.

I understand that this could be very useful for your bringup (and so could be very useful locally), but once the passmanager is the default, it will get lost of in-tree testing by just about everything in the compiler.

I'm not really excited about dragging another out of tree project in unless there is a compelling reason to do so.

-Chris

Keep in mind that I am a maintainer for gmock so this would not change the external project decencies of LLVM.

Is gmock written with more portability in mind than gtest? In my experience, bringing up a new platform for gtest is a huge amount of pain (unless the code has been improved recently - I last tried it about 18 months ago), because the code has very poor abstractions and an incomprehensible nest of #ifdefs for any platform-specific code, mostly testing the wrong thing. Being unable to get the test suite to run was the blocker for even starting to port some of the sanitizers that used gtest last time I tried.

David

It would honestly be simpler for me to write the tests after pulling it in
and point at them. The GoogleMock project has some good examples as well.

I think that's exactly the role for it. It's not going to suddenly become a
panacea (it never was) and is much less likely to become misused in LLVM
than a project like Chrome because *so* much of the code is readily tested
via nice integration tests with lit and FileCheck

Writing a more thought-out reply....

> I have some concrete use cases in testing the pass manager where it will
allow the tests of this API to be more thorough, less verbose, and easier
to maintain. I'm not claiming to be the biggest fan of some features in
GoogleMock, but on the whole, I think it's better than the alternative and
will allow more careful testing of C++ APIs where the interesting part is
the API itself.

Personally, I rather not do this, without very clear and compelling
reasons.

I understand that this could be very useful for your bringup (and so could
be very useful locally), but once the passmanager is the default, it will
get lost of in-tree testing by just about everything in the compiler.

I would much rather have it in the tree than just use it locally. I think
it will also make subsequent iterations much easier to test and show are
correct. I think it would also allow significantly more precise regression
testing in the future.

This also isn't the first time I've wanted it in LLVM and in Clang. It's
just the first time I've been working on something large enoguh to feel
like importing it would be worth the cost.

My feeling is that both gtest and gmock suffer from the same flaw: they can
easily be overused or misused in circumstances where there are clearly
better ways to go about things. However, I feel like within LLVM we have
been really good at pushing back against that and using integration tests
with excellent tool support (how I love FileCheck) much more prevalently.
As long as we continue to code review unittests with an eye toward
skepticism, I think there is very little risk of things getting out of
hand. I think adding gmock to gtest doesn't shift that risk in any
significant way.

However, when we are adding interfaces or generic utilities to LLVM
(admittedly, not the common case) I don't think we do ourselves any favors
by using only half of the available tools to write unit tests for them.

I'm not really excited about dragging another out of tree project in
unless there is a compelling reason to do so.

It helps that gmock is a sibling of gtest. It doesn't really pull in very
much new stuff and like gtest it has strictly managed its dependencies down
to zero. I'm happy to do the importing, the fixing, and to even police
unittests for misuses. I actually don't expect it to be widely used, but I
expect it to make tests significantly more comprehensible and brief in the
limited cases where it applies.

I don't know about what problems you hit, but I would not expect them with
gmock. It is in many ways simpler than gtest, and most notably relies on
gtest for essentially all of its interesting platform dependencies. Of
course, I can't be certain, but this isn't what would worry me.

+1 for showing an example of something you can accomplish that wouldn't otherwise be (easily) possible.

Why not post a patch on reviews?

It is always worth reevaluating.

I agree in principle, but it leads me to a different conclusion. We have other great testing support, which means that the mocking should only be used sparingly. Given that it will not be used much, the cost of carrying it around (and for people to learn how to use/maintain it) is high.

I’ve said this before, but I’m not a fan of our current use of gtest for unit testing. I have never had the unit tests catch a bug, but I have had to update the tests countless times. At least for my purposes, the unit tests cause significantly more harm than good - and it certainly isn’t because I write perfect code. :slight_smile:

There is definitely a culture/religion around testing and TDD, and I am well aware that many projects don’t have proper tests (which LLVM doesn’t suffer from). However, there is a pragmatic balance to be struck here, and I personally think that adding gmock and pushing the unit tests stuff even further is a bad use of testing time (i.e., increases test cycles for make check) and maintenance time (updating tests given that we don’t have a stable API).

-Chris

Agree on this point specifically. There are any number of unit tests
using functions that are otherwise unused or deeply internal, and it
feels too often like googletest is being used as a means to freeze
chunks of the internal API for consumption by external projects we don't
know about.

Looking through the revision history for cfe/unittests/ is revealing --
out of a thousand commits I couldn't find a single one removing a test
by someone outside Google in the last year.

Studying the commits, you can see the pattern: Once a test is in, it
becomes something the rest of the platform works around. I don't think
this is intentional but it's become an unfortunate consequence.

To take an example from something I was working on last week, there are
parts of the Rewrite/MemoryBuffer interface that need changes both for
performance and to fix Windows crashers, but the unit tests specify so
much internal behaviour it's challenging to do so without modifying the
tests. (We've been told from a young age not to remove tests, they're
there for a reason!)

If the purpose of the unit tests is to keep parts of the internal API
stable for external projects, I'd like that to be made more clear, and
inversely that it's OK to liberally remove unit tests that get in the
way of real work, that should also be explained.

Mock tests are notorious for modelling a rigid interface and behaviour.
Wouldn't they just amplify the problems I've cited?

Alp.

However, when we are adding interfaces or generic utilities to LLVM
(admittedly, not the common case) I don't think we do ourselves any favors
by using only half of the available tools to write unit tests for them.

I agree in principle, but it leads me to a different conclusion. We have
other great testing support, which means that the mocking *should* only be
used sparingly. Given that it will not be used much, the cost of carrying
it around (and for people to learn how to use/maintain it) is high.

I think the cost of carrying it around is essentially zero. I'm happy to do
any of the maintenance. People who don't know how to use it or want to
learn how to use it don't need to use it. If it isn't making their job of
writing tests sufficiently easier to justify, then they don't use it. I see
this as a good pattern.

I’ve said this before, but I’m not a fan of our current use of gtest for
unit testing. I have never had the unit tests catch a bug, but I have had
to update the tests countless times. At least for my purposes, the unit
tests cause significantly more harm than good - and it certainly isn’t
because I write perfect code. :slight_smile:

I seem to recall code review spotting a bug that would have been caught by
a unittest were one written. Also, I don't see a lot of patches going in
over the last 2 years that had to re-shuffle unittests. They happen, but
they are very rare. So while I agree this can be a problem, I don't see it
being a problem in practice.

Even so, you aren't the only one we're trying to optimize for. A lot of
people have written unittests using the framework, and I think the
incremental cost of making it a slightly more powerful framework (by adding
one complementary library) is really low.

There is definitely a culture/religion around testing and TDD, and I am
well aware that many projects don’t have proper tests (which LLVM doesn’t
suffer from). However, there is a pragmatic balance to be struck here, and
I personally think that adding gmock and pushing the unit tests stuff even
further is a bad use of testing time (i.e., increases test cycles for make
check) and maintenance time (updating tests given that we don’t have a
stable API).

These two things (adding gmock and pushing unittests further) are not
necessarily related, and I don't plan to do the latter. I'm asking if doing
the former would cause significant problems for any consumers of LLVM, and
I don't hear any statements to that effect except for David Chisnall's
which I responded to specifically.

I'm not trying to make LLVM use unittests everywhere, I'm just trying to
get a tool added to the toolbox so that a unittest I'm already writing can
be written more simply and in a more maintainable fashion.

You're welcome to ignore me and keep writing eloquent emails, but you still haven't shown an exact use case - why not write a unit test which demonstrates the benefit and post a patch for review? for those who are not familiar with gtest/gmock it makes it very clear.. Then the discussion moves from opinions and "feelings" to tangibles

I think the cost of carrying it around is essentially zero. I'm happy to do
any of the maintenance. People who don't know how to use it or want to learn
how to use it don't need to use it. If it isn't making their job of writing
tests sufficiently easier to justify, then they don't use it. I see this as
a good pattern.

That is not the case. If the test finds any bug at all, people have to
look at the testcase and see if it is failing.

Even if not bug is found, someone doing refactoring has to change the
test to use the new apis.

Cheers,
Rafael

Yes, but this is relatively rare in both cases. I looked at the maintenance
burden of unittest/... and it doesn't look like this is a common occurrence
in LLVM. I also expect the results of using these tools to be easier for
maintainers rather than harder in most cases. (See my reply to Sean and C.
Bergstrom...)

I'm not ignoring you, i'm working on exactly that. But it takes a bit more
time, and so I was trying to respond promptly to the emails which weren't
asking for a specific example concurrently with working on a demo of what
I'd like to do.

On IRC, Rafael indicated he would like to understand the specific use case
I had in mind better. I'm still working on a concrete example, but figured
I'd clarify more than my initial mail did.

I'm working on the pass manager. I need a way to test that specific passes
in the pass manager are being run and invalidated appropriately based on
the dependencies between them and the caching infrastructure. Verifying
this can be done in two ways:

1) I can add non-trivial code which essentially dumps the state at each
point, along with a command line tool and collection of fake passes, and
use FileCheck against this code to verify things. This works for asserts
builds but not for no-asserts builds, and generally feels like working
around a missing feature in our testing infrastructure. This is the same
reasons we don't use FileCheck and state serialization to test our DenseMap
implementation.

2) I can write a unittest using gtest with a completely custom collection
of N passes written for every single test, each with a different set of
integer output parameters that are incremented and decremented at the right
points, and then a verification of their final value. This will work, but
will be hard to debug (the failure is detected long after it happens) and
very verbose.

3) I can use gmock to write a specific set of expected events for a pass
and verify that they happen. It was specifically designed to make verifying
this kind of interaction explicit with little boilerplate and decent error
messages when it fails.

I'll try to come up with an example of #3 this evening or tomorrow.

-Chandler