<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Mar 1, 2021 at 7:25 AM <<a href="mailto:paul.robinson@sony.com">paul.robinson@sony.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Thanks for the feedback, David!<br>
<br>
> Initial gut reaction would be this is perhaps a big enough<br>
> patch/divergence from upstream gtest that it should go into<br>
> upstream gtest first<br>
<br>
I was not aware that googletest itself was open source, as<br>
opposed to something Google-internal that you guys had permission<br>
to put into LLVM.  Although it makes sense for it to be open<br>
source on its own.<br>
<br>
As I mentioned in the writeup, it would be great to get feedback<br>
from people who actually understood googletest, try it on other<br>
platforms I don't have available (e.g., Mac) and ideally help solve<br>
a couple of the problems I have run into.<br>
<br>
In the meantime, obviously I can keep patching up our unittests<br>
where RGT has found something.<br>
<br>
> & maybe sync'ing up with a more recent gtest into LLVM? Though<br>
> I realize that's a bit of a big undertaking (either/both of those<br>
> steps). <br>
<br>
A quick skim through the LLVM git log suggests that we've done<br>
some things to connect with LLVM-ish ways of doing things, e.g.,<br>
using raw_ostream instead of std::ostream.  Other things, such<br>
as new targets/environment (I saw mention of MinGW) probably also<br>
should have been done upstream.  Perhaps it is not obvious to<br>
non-Googlers that there *is* an upstream project (did I mention <br>
I was not aware?).<br>
<br>
The last upgrade was to 1.8.0 in Jan 2017 by Chandler.  I see<br>
upstream has tagged 1.8.1 and 1.10.0 since then (no 1.9.0).<br>
Chandler's log message does say "minimizing local changes" which<br>
sounds somewhat hopeful.  There have been changes since then.<br>
Looking at a handful of diffs, I see some things guarded by a<br>
conditional with LLVM in the name, and more where the change<br>
was simply made with no effort to identify it as local.  This<br>
suggests we (LLVM) have made some effort to make future merges<br>
not-horrible, but it would still be a chunk of work.<br>
<br>
As a 10% project, I think I'd prefer to concentrate on fixing<br>
our own unittests first, but work on integrating with upstream<br>
googletest rather than pursue the RGT infrastructure as an LLVM<br>
mod, when I've gotten our test fixes sorted out.<br></blockquote><div><br>Sure - that sounds good to me. Fixes to existing tests, no matter the means used to find them seem like general goodness, but yeah - when it comes to upstreaming the infrastructure to prevent regressing this invariant, upstream to gtest and sync'd to LLVM sounds like the best path.<br><br>& generally I'd be in favor of the functionality, if the false positives can be worked out, etc. No doubt upstream gtest review will likely have some ideas about how such a thing can/should be integrated, how its results should be rendered to the user, etc.<br><br>(my ultimate favourite idea that I'd love to see more of is mutation testing - randomized and/or coverage-directed mutation of code under test that verifies tests fail if the code is mutated - this ensures it's more than just coverage (that lines of code were executed) but that their behavior is observed and validated - but that's a bunch of work to implement/plumb into test systems, etc)<br><br>- Dave</div></div></div>