cmcfarlen commented on code in PR #13658:
URL: https://github.com/apache/trafficserver/pull/13658#discussion_r3973338095


##########
lib/CMakeLists.txt:
##########
@@ -43,14 +44,38 @@ if(BUILD_TESTING)
   target_compile_options(Catch2 INTERFACE -Wno-error=parentheses)
   target_compile_options(Catch2WithMain INTERFACE -Wno-error=parentheses)
 
-  # By default, as of v3.9.0, Catch2 runs tests in random order. Our tests are
-  # written with the expectation that they are run in declared order, with 
setup
-  # done in one TEST_CASE expected to carry over into the next TEST_CASE. This
-  # macro allows us to add common command line arguments to all tests. For now,
-  # we ensure declaration order execution via --order decl.
+  # Register one ctest test per Catch2 TEST_CASE/SCENARIO rather than one per
+  # executable. ctest then names the failing case instead of the binary, can
+  # schedule cases in parallel, and can run a single case by name. Because each
+  # case gets its own process, cases must not depend on state set up by an
+  # earlier case in the same executable.
+  #
+  # ctest names are "<NAME>.<case name>" so the NAME given here stays a usable
+  # -R filter prefix even where it differs from the target name.
+  #
+  # Discovery is PRE_TEST so that configuring/building does not require running
+  # the test executables.
   macro(add_catch2_test)
-    cmake_parse_arguments(CATCH2_TEST "" "NAME" "COMMAND" ${ARGN})
-    add_test(NAME ${CATCH2_TEST_NAME} COMMAND ${CATCH2_TEST_COMMAND} --order 
decl)
+    cmake_parse_arguments(CATCH2_TEST "" "NAME" "COMMAND;ENVIRONMENT" ${ARGN})
+
+    list(GET CATCH2_TEST_COMMAND 0 _catch2_command)
+    if(_catch2_command MATCHES "^\\$<TARGET_FILE:(.+)>$")
+      set(_catch2_target "${CMAKE_MATCH_1}")
+    else()
+      set(_catch2_target "${_catch2_command}")
+    endif()
+

Review Comment:
   Fixed in 29169dfa6e, though not by forwarding the extra args.
   
   Per-case discovery appends the case name to the command line, so there is 
nowhere for caller-supplied runner arguments to go — `EXTRA_ARGS` would change 
their meaning rather than preserve it. And no call site passes more than one 
element today (checked all ~85; every `COMMAND` is either a bare target or a 
single `$<TARGET_FILE:...>`).
   
   So the macro now rejects it at configure time instead of dropping it 
silently:
   
   ```
   CMake Error: add_catch2_test(test_xdebug): COMMAND must be a single test 
executable
   Call Stack (most recent call first):
     plugins/xdebug/CMakeLists.txt:26 (add_catch2_test)
   ```
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to