<div dir="ltr"><div>On Fri, Apr 24, 2020 at 11:17 AM Balázs Benics via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>Hi all,<br><br>On a Phabricator review (<a href="https://reviews.llvm.org/D78704" target="_blank">https://reviews.llvm.org/D78704</a>) the `<span style="font-family:monospace">GTEST_SKIP</span>` was<br>mentioned as a way to express requirements for a test. In that patch, I wanted<br>to skip the test if clang was not built with Z3.<br><br>I think this `<span style="font-family:monospace">GTEST_SKIP</span>` macro express the intention much better than the<br>following code that I saw throughout the codebase:<br><br>```<br><span style="font-family:monospace">#define CHECK_UNSUPPORTED() \<br>  do { \<br>    if (isUnsupportedOSOrEnvironment()) \<br>      return; \<br>  } while (0);</span><br>```<br><br><br>There are several places where we might benefit from the `GTEST_SKIP` macro:<br>- <a href="https://github.com/llvm/llvm-project/blob/master/llvm/unittests/ExecutionEngine/MCJIT/MCJITTestAPICommon.h#L29-L33" target="_blank">llvm/unittests/ExecutionEngine/MCJIT/MCJITTestAPICommon.h:29</a><br>- <a href="https://github.com/llvm/llvm-project/blob/master/llvm/unittests/Support/ThreadPool.cpp#L81-L85" target="_blank">llvm/unittests/Support/ThreadPool.cpp:81</a><br>- <a href="https://github.com/llvm/llvm-project/blob/master/llvm/unittests/Support/MemoryTest.cpp#L89-L94" target="_blank">llvm/unittests/Support/MemoryTest.cpp:89</a><br><br>It is possible that the upgrade would introduce other benefits as well - which<br>I'm not aware of.<br></div></div></blockquote><div>I know there are some minor features unavailable in our old gtest that I've wanted to use and worked around.</div><div>Unfortunately I can't remember what these cases were, but I think there are at least minor benefits.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Should we upgrade googletest to version 1.10.0?<br></div></blockquote><div>+1 from me.<br></div><div><br></div><div>A thing to be aware of for anyone doing the upgrade: we have a few customizations e.g.</div><div>llvm/utils/unittest/googletest/include/gtest/internal/custom/raw-ostream.h, and also small (I hope clearly marked) modifications within gtest itself.</div><div><br></div><div>Some of these you'll notice (replacing them with clean sources will break the build), but others affect assertion failure message quality, and I'm not sure if they themselves are well tested.</div><div>It's probably worthwhile to trawl the history of the gtest directories to find these...</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><br>/CC Chandler<br><br>Balazs.</div></div>
_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
</blockquote></div></div>