[test] Unify path defines and add CPPINTEROP_BIN_DIR env override#1037
[test] Unify path defines and add CPPINTEROP_BIN_DIR env override#1037conrade-ctc wants to merge 1 commit into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1037 +/- ##
=======================================
Coverage 86.56% 86.56%
=======================================
Files 23 23
Lines 5621 5621
=======================================
Hits 4866 4866
Misses 755 755 🚀 New features to boost your workflow:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| DispatchInitializer() { | ||
| if (!Cpp::LoadDispatchAPI(CPPINTEROP_LIB_PATH)) { | ||
| std::abort(); | ||
| std::string libPath = TestUtils::GetCppInterOpLibPath(); |
There was a problem hiding this comment.
warning: no header providing "std::string" is directly included [misc-include-cleaner]
unittests/CppInterOp/DispatchInit.cpp:9:
+ #include <string>| @@ -0,0 +1,40 @@ | |||
| #ifndef CPPINTEROP_UNITTESTS_LIBCPPINTEROP_TESTPATHS_H | |||
| #define CPPINTEROP_UNITTESTS_LIBCPPINTEROP_TESTPATHS_H | |||
There was a problem hiding this comment.
warning: header guard does not follow preferred style [llvm-header-guard]
| #define CPPINTEROP_UNITTESTS_LIBCPPINTEROP_TESTPATHS_H | |
| #ifndef GITHUB_WORKSPACE_UNITTESTS_CPPINTEROP_TESTPATHS_H | |
| #define GITHUB_WORKSPACE_UNITTESTS_CPPINTEROP_TESTPATHS_H |
unittests/CppInterOp/TestPaths.h:39:
- #endif // CPPINTEROP_UNITTESTS_LIBCPPINTEROP_TESTPATHS_H
+ #endif // GITHUB_WORKSPACE_UNITTESTS_CPPINTEROP_TESTPATHS_HThere was a problem hiding this comment.
I think these need to be ignored, right? Maybe there is a clang-tidy config update so this doesn't get flagged @vgvassilev?
|
I had a more localized change originally, but there were some inconsistencies across the repos that I thought might be nice to tackle now since we're migrating to a consolidated repo setup, and consistency across that set seemed like a good idea. Let me know if you think that's not the right approach. @vgvassilev @Vipul-Cariappa @aaronj0 |
| "CPPINTEROP_DIR=\"${CMAKE_CURRENT_SOURCE_DIR}/../../\"" | ||
| "CPPINTEROP_BINARY_DIR=\"${CMAKE_CURRENT_BINARY_DIR}/../../\"" | ||
| "CPPINTEROP_SRC_DIR=\"${CMAKE_CURRENT_SOURCE_DIR}/../../\"" | ||
| "CPPINTEROP_BIN_DIR=\"${CMAKE_BINARY_DIR}\"" |
There was a problem hiding this comment.
I believe that change breaks ROOT. @aaronj0, can you review?
Dispatch tests baked the library location into a CPPINTEROP_LIB_PATH macro and TracingTests carried both CPPINTEROP_DIR (source tree) and CPPINTEROP_BINARY_DIR (artifacts) defines. Collapse the two artifacts-prefix concepts into a single CPPINTEROP_BIN_DIR that a matching environment variable overrides at runtime, so relocated test binaries can find libclangCppInterOp: - Add TestPaths.h with GetCppInterOpDirPath()/GetCppInterOpLibPath(): env CPPINTEROP_BIN_DIR wins, else the compile-time default, else empty (no hard dependency on the macro being defined). - Drop CPPINTEROP_LIB_PATH; the dispatch tests derive the library path from the prefix and fail with a clear message instead of aborting when it is unknown. - Rename the TracingTests source-tree macro to CPPINTEROP_SRC_DIR and route its artifacts lookups through the shared helper. Co-developed-with-the-help-of: Claude Code (Claude Opus 4.8)
conrade-ctc
force-pushed
the
unify_path_defines_and_env
branch
from
8348d51 to
0b148c7
Compare
2 participants
Summary
CPPINTEROP_LIB_PATHfor the dispatch tests,CPPINTEROP_BINARY_DIRfor TracingTests) into a singleCPPINTEROP_BIN_DIR, overridable at runtime by an environment variable of the same name so relocated test binaries can locatelibclangCppInterOp.unittests/CppInterOp/TestPaths.h(dependency-free) withGetCppInterOpDirPath()/GetCppInterOpLibPath(): env var wins, else the compile-time default, else empty — no hard dependency on the macro being defined.CPPINTEROP_LIB_PATH; the dispatch tests now derive the library path from the prefix and fail with a clear message instead ofstd::abort()when the location is unknown.CPPINTEROP_SRC_DIR(it reads.cpp/.hsources, which never live in an artifacts prefix) and route its artifacts lookups through the shared helper.(this is related to compiler-research/cppyy-backend#211 as well)
Test plan
check-cppinterop: 7/7 passing, includingDispatchTestsand theDispatchTests-isolationldd contract.🤖 Done with the help of Claude Code (Claude Opus 4.8)