llvm dev - Mar 2021 - [cfe-dev] Rotten Green Tests project

The overall concept seems interesting to me. Anything that helps reduce
problems in tests that could obscure bugs etc is worth a good
consideration, in my opinion.

On Fri, 26 Feb 2021 at 18:47, via cfe-dev <cfe-dev at lists.llvm.org>
wrote:
> Well... actually... it is ON by default; however, I turn it off in
> lit.  So, if you run `check-llvm` or use `llvm-lit` to run unittests,
> they won't report rotten green tests.  However, if you run a program
> directly, it will report them (and cause the test program to exit with
> a failure status).  This seemed like a reasonable balance that would
> make RGT useful while developing a test, without interfering with
> automation.
>
> When writing googletest unit tests, I almost always run the test
executable directly. This is because it's by far the easiest way to run the
test and debug in Visual Studio ("Set startup project" -> F5). I
wouldn't
be happy if this started showing up false test failures in some form or
other, unless someone can point at an equally simple way of doing the same
thing.

James
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20210301/2a57ec07/attachment.html>

Thanks for the feedback, David!
> Initial gut reaction would be this is perhaps a big enough
> patch/divergence from upstream gtest that it should go into
> upstream gtest first
I was not aware that googletest itself was open source, as
opposed to something Google-internal that you guys had permission
to put into LLVM.  Although it makes sense for it to be open
source on its own.

As I mentioned in the writeup, it would be great to get feedback
from people who actually understood googletest, try it on other
platforms I don't have available (e.g., Mac) and ideally help solve
a couple of the problems I have run into.

In the meantime, obviously I can keep patching up our unittests
where RGT has found something.
> & maybe sync'ing up with a more recent gtest into LLVM? Though
> I realize that's a bit of a big undertaking (either/both of those
> steps). 
A quick skim through the LLVM git log suggests that we've done
some things to connect with LLVM-ish ways of doing things, e.g.,
using raw_ostream instead of std::ostream.  Other things, such
as new targets/environment (I saw mention of MinGW) probably also
should have been done upstream.  Perhaps it is not obvious to
non-Googlers that there *is* an upstream project (did I mention 
I was not aware?).

The last upgrade was to 1.8.0 in Jan 2017 by Chandler.  I see
upstream has tagged 1.8.1 and 1.10.0 since then (no 1.9.0).
Chandler's log message does say "minimizing local changes" which
sounds somewhat hopeful.  There have been changes since then.
Looking at a handful of diffs, I see some things guarded by a
conditional with LLVM in the name, and more where the change
was simply made with no effort to identify it as local.  This
suggests we (LLVM) have made some effort to make future merges
not-horrible, but it would still be a chunk of work.

As a 10% project, I think I'd prefer to concentrate on fixing
our own unittests first, but work on integrating with upstream
googletest rather than pursue the RGT infrastructure as an LLVM
mod, when I've gotten our test fixes sorted out.
> How does this compare to other local patches to gtest we have?
No clue.  I'd have to do a diff against the upstream project, and
sort out which are due to LLVM's stripping-down procedure and 
which are actual features & fixes we've done.  However, as a guess
it's overall significantly bigger than anything else done locally.

I have tried to minimize RGT's intrusion into the base googletest
code (well, LLVM's base googletest code).  Most of it is in a new
header and .cc file.  After proper integration with the upstream
project, that might not remain true, though.

Thanks,
--paulr

Thanks, James!
> When writing googletest unit tests, I almost always run the test
> executable directly. This is because it's by far the easiest way
> to run the test and debug in Visual Studio ("Set startup project"
> -> F5).
This is exactly the use-case I had in mind.
> I wouldn't be happy if this started showing up false test
> failures in some form or other, unless someone can point at an
> equally simple way of doing the same thing.
None of them show up as individual *test* failures; what happens
is you get a report at the end with a list of rotten assertions.
The test *program* will have a failure exit status, but the list
of passing/failing tests is unaffected.

At the moment, the number of false positives appears to be small-ish.

A DISABLED test will show up as rotten, unless you run the test with
--gtest_also_run_disabled_tests; LLVM has 3 test programs like this,
out of 56, and Clang has two out of 22.

There are also some cases where EXPECT_NONFATAL_FAILURE has another 
EXPECT as an argument, and that doesn't seem to work with RGT (but I 
haven't figured out whether this is a case of Don't Do That, or 
something else). There are two LLVM unittest programs like this.

There might be more, I haven't analyzed all the failure reports to
track down exactly what's going on.  Most of the rest are likely
issues with the test programs themselves.

--paulr