llvm dev - Nov 2013 - [LLVMdev] Any objections to my importing GoogleMock to go with GoogleTest in LLVM?

Writing a more thought-out reply....

On Tue, Nov 12, 2013 at 9:16 PM, Chris Lattner <clattner at apple.com>
wrote:
>
> On Nov 12, 2013, at 7:04 PM, Chandler Carruth <chandlerc at
google.com>
> wrote:
>
> > 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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20131114/37e7fc86/attachment.html>

On Nov 14, 2013, at 3:16 AM, Chandler Carruth <chandlerc at google.com>
wrote:
> 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.
It is always worth reevaluating.
> 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 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. :-)

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20131114/15e5f4cb/attachment.html>

On 14/11/2013 18:58, Chris Lattner wrote:
>
> 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.
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.

-- 
http://www.nuanti.com
the browser experts

On Thu, Nov 14, 2013 at 10:58 AM, Chris Lattner <clattner at apple.com>
wrote:
> On Nov 14, 2013, at 3:16 AM, Chandler Carruth <chandlerc at
google.com>
> wrote:
>
> 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. :-)
>
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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20131114/15e6ba8a/attachment.html>