Updating GoogleTest to v1.14.0?

I am working on building and running LLVM unit tests, which are built on top of GoogleTest, for the Fuchsia platform. I noticed that the GoogleTest source under third-party/unittest is still on v1.10.0, which lacks some bug fixes related to the Fuchsia platform.

Instead of backporting these fixes to LLVM’s GoogleTest fork, I am wondering if it is better to take this opportunity to update it to the latest release version, which is v1.14.0. This is 3 years newer than what LLVM currently has. Rolling GoogleTest to v1.14.0 will bring a lot of updates and bug fixes, especially the C++ 17 support introduced in the v1.11.0 release.

In addition, it would be great if we can modularize the LLVM’s GoogleTest fork so that third-party/unittest is just a vanilla copy of GoogleTest (with the LLVM-specific custom code in a separate directory) and a script that could be used to update it in the future similar to how we manage Google Benchmark in third-party/benchmark.



[RFC] Updating googletest to non-release tagged version is the last time googletest was attempted to be updated.

Mirroring the discussion there, modern is generally better as long as our supported compilers can build it.


I am in favor. Are you volunteering the work? :slight_smile: That would be really great.
Latest googletest supports NetBSD and Haiku and we can remove

* Added support for NetBSD, Minix and Haiku.

from third-party/unittest/googletest/README.LLVM.

Minix is inactive and it has very old GCC. I don’t think it can build llvm-project.

If no body object, yeah I volunteer to work on this. I hope it is not a lot of work.

No objection from me. Please make sure to test the new googletest on a wide range of platforms (including Windows). It would probably be worth when doing this testing to make sure that the same number of tests were run as before the upgrade, just in case something funky’s going on with the behaviour of the gtest macros.

An upgrade is long overdue.

If you can manage that, it would be great, although I’m not 100% convinced it would be feasible. Some of the customizations are doing things like adding overloads to accept LLVM internal types such as StringRef. But the closer we can get to a straight copy of upstream, the better.

I’ve been working on an enhancement to Google Test for some time so I’m moderately familiar with it; please add me as a reviewer (my Phab handle is probinson). But also please make a note here when you post the patch, as I’m more likely to notice it here than as a Phab email.

BTW googletest: why LLVM is not using system installed googletest? :thinking:

Because not everybody has googletest installed on their system, and it may vary in version between different users who do have it installed.

At the moment ALL leading distributions provides googletest as package.
The sam you can say that “not everybody has installed compiler”.

All LLVM <> googletest interaction can be done in single cmake line

pkg_check_modules(GTEST REQUIRED gtest >= 1.14.0)

in any LLVM subproject which needs googletest.

LLVM has really huge ToDo list. It would be way better to drop from that list maintaining local copy of googletest.

I think the system software should have less dependencies as much as possible.

Windows is a big platform for LLVM as well.

1 Like

Really no one in LLVM should take care of someone build env.

Not as it is right now. There is a bunch of LLVM customizations to the in-tree copy of googletest. Some can be dropped now, some can be worked around, but some would probably require rewriting lots of tests. It’d be good to have at least a clear diff with upstream and work on reducing/isolating it.

1 Like

Because LLVM’s copy has modifications to support its datatypes, such as being able to stream a StringRef to an assertion message.

1 Like

I created a PR for updating GoogleTest to v1.14.0 : Update GoogleTest to v1.14.0 by zeroomega · Pull Request #65823 · llvm/llvm-project · GitHub
Any comments are welcome.

Is it any problem with merge those changed into upstream googletest?

llvm::StringRef does not exist in upstream googletest, or in any non-LLVM client of googletest, so merging a change to support it would be problematic.
If possible, we should use googletest’s existing local customization hooks to add this support locally.
(Edit) Having looked at the patch now, that is mainly what we did. Googletest buries those customization headers deep in its tree so trying to do something like a git submodule is unlikely to work.

So why someone choose to break gtest API?
Why EVERY other project which is using gtest was able to use standard gtest and LLVM not?

And again: did someone discuss some API extensions with gtest maintainer?

LLVM may not be extending gtest in the proper way, it may never have been doing so. That’s water under the bridge at this point.

Continuing to hammer this point in this tone is not going to help matters and only serves to discourage those who might generously offer their time to improve it.

Luckily the thread author has posted a PR already. If you don’t think that’s enough, you could submit a follow up to improve the situation, I’m sure you’ll have plenty of reviewers :slight_smile:


Reviewing the patch, it looks like there are two problems with using a completely unmodified upstream googletest.

  1. LLVM has its own stream implementation, which requires a more intrusive change than the usual googletest customization scheme allows. It’s relatively minimal and it’s possible upstream googletest could be persuaded to support it.
  2. Googletest’s own scheme for supporting customizations involves changing header files deep inside its source tree. There is literally nothing we can do about that.