ci: add FetchContent consumer regression test - #349
Merged
Conversation
Adds a minimal standalone CMake project (test/cmake_integration/consumer) that consumes Graaf via FetchContent, plus a verify_fetchcontent CI job that configures, builds, runs, and installs it, then asserts no graaf.pc leaks into the consumer's install tree. This is a build-system regression, not something GTest can observe -- both #310 (a configure-time crash) and #312's fix (avoiding incorrect pkgconfig metadata when Graaf is a subproject) only surface when Graaf is genuinely consumed as a FetchContent dependency. Verified locally that this setup passes against current main and fails the same way #310 originally did when reverted to the pre-#312 CMakeLists.txt. Kept as its own CI job rather than a ctest case, so the fast unit test suite stays fast and a failure here is unambiguous about what broke.
test/CMakeLists.txt globs *.cpp recursively (file(GLOB_RECURSE TEST_SOURCES "./*.cpp")), which swept up cmake_integration/consumer's own main() into the Graaf_test binary. Since gtest_main is a static library, the linker satisfied the already-defined `main` symbol from that file instead of extracting gtest_main's real one from the archive -- silently replacing the whole test runner. `ctest` then found and ran zero real tests, though it still exited 0 in compile_and_test; only the stricter ctest_coverage target caught it, which is why coverage failed while compile_and_test showed green. Moving the consumer project to a top-level cmake_integration/ directory (sibling to test/, examples/, perf/) avoids the glob entirely. Verified locally: all 752 real tests are discovered and pass again via ctest, and the FetchContent consumer check still passes from its new location.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #349 +/- ##
=======================================
Coverage 99.58% 99.58%
=======================================
Files 59 59
Lines 2622 2622
Branches 136 136
=======================================
Hits 2611 2611
Misses 11 11 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
#310 (a CMake configure-time crash) and #312 (the fix, which also avoids publishing incorrect pkgconfig metadata) both only surface when Graaf is genuinely consumed as a
FetchContentdependency by a separate project. That's a build-system regression class that GTest has no visibility into -- it happens before any C++ code compiles or runs -- so there was previously no automated coverage that would catch a regression here.What Changed
test/cmake_integration/consumer/: a minimal standalone CMake project thatFetchContents Graaf viaSOURCE_DIR(no network needed) and links a trivial executable against it.verify_fetchcontentCI job (pull-request.yml) that configures, builds, runs, and installs that consumer project, then asserts nograaf.pcleaks into its install tree.Kept as its own CI job rather than a
ctestcase, so the fast unit test suite stays fast and a failure here is unambiguous about what broke (matches the pattern used by fmt/spdlog/Catch2/Abseil for this exact "does our CMake packaging work" concern).Verification
Ran the exact CI steps locally against current
main: configure/build/run/install all succeed, and no.pcfile leaks into the consumer's install tree.Also confirmed the check actually catches the original regression: reverted to the pre-#312
CMakeLists.txt(the unconditionalconfigure_file(${CMAKE_SOURCE_DIR}/...)) in a throwaway copy and reran the same steps -- the configure step fails immediately with the same error originally reported in #310 (File .../packaging/graaf.pc.in does not exist).