statically link GTest, update to 1.18.0, fetch it with CPM - #705
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## mainline #705 +/- ##
=========================================
Coverage 83.97% 83.97%
=========================================
Files 75 75
Lines 6908 6908
Branches 559 559
=========================================
Hits 5801 5801
Misses 1107 1107 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| # As of v1.14.0, googletest did literal string replacements on CMAKE_CXX_FLAGS, | ||
| # and doesn't respect CMAKE_MSVC_RUNTIME_LIBRARY. | ||
| # | ||
| # TODO(jameslamb): remove this once this project uses at least GTest v1.18.0 |
There was a problem hiding this comment.
I'd love to use the latest GTest. Is there a disadvantage for upgrading?
There was a problem hiding this comment.
Oh sure! I'm not aware of any disadvantage. Looks like it still has a CMake minimum of 3.16 (build link), and CI could tell us if it has any compiler support limitations or other problems like that (but I really doubt it for that project).
Also... statically linking it makes it even safer to upgrade this project to a different version 😁
I'll try that here.
There was a problem hiding this comment.
I just tried locally on my Mac and tests seem to work fine with the latest version (1.18.0, released 3 weeks ago).
Pushed f91ecf0 with that upgrade + removing the MSVC stuff.
|
Thanks! |
Closes #704
Proposes:
Notes for Reviewers
How I tested this
Using the reproducible example from #704.