Make catch tags work with ctest labels

[pterodragon]

include(Catch)
catch_discover_tests(all_tests)

The above is good to include all the test names into ctest so that ctest -N lists the test names. But it doesn't work with tags. ctest provides the LABEL property for a test so maybe the idea of tags in Catch is close to that. ctest --print-labels lists all the test labels and ctest -L <regex> runs tests with labels matching the regex.

[horenmar]

That sounds like an interesting and useful feature for the contrib/ scripts.

[horenmar]

This has been implemented by @MaciejPatro.

[horenmar]

As it turns out, the original PR for this issue introduced bugs to the original functionality due to various factors, so it was reverted.

There are also problems with adding tags as labels to the tests because of CMake upstream issue

[Quincunx271]

The upstream CMake issue has been merged into CMake 3.18. As such, we can go ahead and pull this feature back online after verifying the CMake version. As I mentioned in #1658, the LABELS should be APPEND-ed, not just set. See the partial implementation here: https://github.com/catchorg/Catch2/pull/1658#issuecomment-502490071 . That implementation should work after some adjustments.

[sourcedelica]

Hi @Quincunx271 - what's the status of this? I tried using the Catch.cmake and CatchAddTests.cmake from your partial implementation (https://github.com/Quincunx271/Catch2/blob/3047ee81052b35f9867b61c15a81c9df2b539621/contrib/CatchAddTests.cmake) with CMake 3.21.0, but when I run ctest I get errors like:

$ ctest -N
Test project /home/eric/work/tw-base/cmake-build-debug
CMake Error at /home/eric/work/tw-base/cmake-build-debug/tw/utils/test/utils-test_tests-b858cb2.cmake:65 (set_property):
  set_property given TEST names that do not exist:

    utils:Binary Decoding
    utils:Binary Encoding
    utils:Binary Encoding Types
    utils:Binary Encoding sequence

Call Stack (most recent call first):
  /home/eric/work/tw-base/cmake-build-debug/tw/utils/test/utils-test_include-b858cb2.cmake:2 (include)
  /home/eric/work/tw-base/cmake-build-debug/tw/utils/test/CTestTestfile.cmake:7 (include)
  /home/eric/work/tw-base/cmake-build-debug/tw/utils/CTestTestfile.cmake:7 (subdirs)
  /home/eric/work/tw-base/cmake-build-debug/tw/CTestTestfile.cmake:8 (subdirs)
  CTestTestfile.cmake:7 (subdirs)

But these tests do exist, for example (after reverting to the upstream versions of Catch.cmake and CatchAddTests.cmake):

$ ctest -N
Test project /home/eric/work/tw-base/cmake-build-debug
  Test   #1: utils:Binary Encoding
  Test   #2: utils:Binary Decoding
  Test   #3: utils:Binary Encoding Types
  Test   #4: utils:Binary Encoding sequence

[Quincunx271]

There may be some mishandled quoting going on there. Check the generated <target>_tests-<hash>.cmake file and see what the set_property(...) calls look like. If they look like this:

set_property(TEST
    utils:Binary Decoding
    ...
)

It might need to look like this:

set_property(TEST
    "utils:Binary Decoding"
    ...
)

That's my best guess.

[sourcedelica]

I got it working with this fix:

diff --git a/contrib/CatchAddTests.cmake b/contrib/CatchAddTests.cmake
index b2a4d886..a91c9ea7 100644
--- a/contrib/CatchAddTests.cmake
+++ b/contrib/CatchAddTests.cmake
@@ -110,9 +110,9 @@ function(get_tags OUT)
         list(APPEND test_list "${prefix}${test}${suffix}")
       endforeach()

-      add_command(set_property TEST
+      add_command(set_tests_properties
         ${test_list}
-        APPEND PROPERTY LABELS "${tag}"
+       PROPERTIES LABELS "${tag}"
       )
     endif()
   endforeach()

Once I got it working I noticed that it is adding the labels in the form [tag]. For example:

$ ctest --print-labels
Test project /home/eric/work/tw-base/cmake-build-debug
All Labels:
  [Admin]
  [AsyncSuccess]
  [AsyncTimeout]
  [Async]
  ... (many more)

Because square brackets are special regex characters and ctest -L uses regexes, you have to use a syntax like ctest -L "\[tag\]":

$ ctest -L "\[Admin\]"
Test project /home/eric/work/tw-base/cmake-build-debug
    Start 61: service:Runtime Config Over UDP
1/5 Test #61: service:Runtime Config Over UDP ...   Passed    1.20 sec
    Start 62: service:Config Watch
2/5 Test #62: service:Config Watch ..............   Passed    1.49 sec
    Start 63: service:Metrics Over UDP
3/5 Test #63: service:Metrics Over UDP ..........   Passed    0.68 sec
    Start 64: service:LMon Controller
4/5 Test #64: service:LMon Controller ...........   Passed    1.29 sec
    Start 65: service:Runtime Config
5/5 Test #65: service:Runtime Config ............   Passed    0.59 sec

100% tests passed, 0 tests failed out of 5

Label Time Summary:
[Admin]      =   5.25 sec*proc (5 tests)
[Config]     =   3.28 sec*proc (3 tests)
[LMon]       =   1.29 sec*proc (1 test)
[Metrics]    =   0.68 sec*proc (1 test)
[Network]    =   3.16 sec*proc (3 tests)

Total Test time (real) =   5.25 sec

I would recommend not including the square brackets in the labels. What do you think?

[Quincunx271]

set_tests_properties will only let each test have a single label, unless you change the label computation to calculate every label for a test and add them all at once. That's why I was using set_property, because it has an APPEND mode.

I agree with removing the square brackets.

[sourcedelica]

Ahh - I see what you mean about the APPEND.

It looks like maybe there is a CMake bug with set_property TEST APPEND PROPERTY LABELS. The test names are already using the bracketed argument syntax so quoting the test name in set_property TEST "[==[name]==]" didn't work. I wouldn't think that it's an escaping problem because set_tests_properties works fine.

Let me see about calculating all labels for a test and adding at once.

[sourcedelica]

Got it working

$ ctest --print-labels
Test project /home/eric/work/tw-base/cmake-build-debug
All Labels:
  Admin
  AsyncSuccess
  AsyncTimeout
  Async
  ... (many more)

$ ctest -L RequestResponse
Test project /home/eric/work/tw-base/cmake-build-debug
    Start 68: service:Relay Envelope
1/4 Test #68: service:Relay Envelope ...........   Passed    0.65 sec
    Start 74: service:Request Response
2/4 Test #74: service:Request Response .........   Passed    0.74 sec
    Start 75: service:Older Request
3/4 Test #75: service:Older Request ............   Passed    0.73 sec
    Start 76: service:Timeout Test
4/4 Test #76: service:Timeout Test .............   Passed    2.38 sec
 
100% tests passed, 0 tests failed out of 4
 
Label Time Summary:
RequestResponse    =   4.49 sec*proc (4 tests)

Total Test time (real) =   4.50 sec

Here's the patch. Basically changing the structure to

get tests for current test exe
for each test
  get tags for test
  convert tags to labels
  set labels on test

diff --git a/contrib/CatchAddTests.cmake b/contrib/CatchAddTests.cmake
index b2a4d886..e564b626 100644
--- a/contrib/CatchAddTests.cmake
+++ b/contrib/CatchAddTests.cmake
@@ -1,6 +1,8 @@
# Distributed under the OSI-approved BSD 3-Clause License.  See accompanyin=
g
# file Copyright.txt or https://cmake.org/licensing for details.

+message("CUSTOM CATCH-ADD-TESTS")
+
set(prefix "${TEST_PREFIX}")
set(suffix "${TEST_SUFFIX}")
set(spec ${TEST_SPEC})
@@ -9,6 +11,7 @@ set(properties ${TEST_PROPERTIES})
set(script)
set(suite)
set(tests)
+set(test_names)

function(add_command NAME)
   set(_args "")
@@ -68,6 +71,7 @@ foreach(line ${output})
     ${properties}
   )
   list(APPEND tests "${prefix}${test}${suffix}")
+  list(APPEND test_names "${test}")
endforeach()

# Create a list of all discovered tests, which users may use to e.g. set
@@ -77,44 +81,34 @@ add_command(set ${TEST_LIST} ${tests})
# Run executable to get list of tags
function(get_tags OUT)
   set(script)
-  execute_process(
-    COMMAND ${TEST_EXECUTOR} "${TEST_EXECUTABLE}" ${spec} --list-tags
-    OUTPUT_VARIABLE tags
-    RESULT_VARIABLE result
-  )
-  if(${result} LESS 0)
-    return() # If we can't figure out the tags, that's fine, don't add lab=
els
-  endif()

-  string(REPLACE "\n" ";" tags "${tags}")
-  set(tags_regex "(\\[([^\\[]*)\\])")
+  foreach(test ${test_names})
+    execute_process(
+      COMMAND ${TEST_EXECUTOR} "${TEST_EXECUTABLE}" "${test}" --list-tags
+      OUTPUT_VARIABLE tags
+      RESULT_VARIABLE result
+    )
+    if(${result} LESS 0)
+      continue() # If we can't figure out the tags, that's fine, don't add=
 labels
+    endif()

-  foreach(tag_spec ${tags})
-    # Note that very long tags line-wrap, which won't match this regex
-    if(tag_spec MATCHES "${tags_regex}")
-      set(tag "${CMAKE_MATCH_1}")
+    string(REPLACE "\n" ";" tags "${tags}")
+    set(tags_regex "(\\[([^\\[]*)\\])")
+    set(clean_tags)

-      execute_process(
-        COMMAND ${TEST_EXECUTOR} "${TEST_EXECUTABLE}" ${spec} "${tag}" --l=
ist-test-names-only
-        OUTPUT_VARIABLE tests
-        RESULT_VARIABLE result
-      )
-      if(${result} LESS 0)
-        continue() # If we can't figure out the related tests, abort for t=
his tag.
+    foreach(tag_spec ${tags})
+      # Note that very long tags line-wrap, which won't match this regex
+      if(tag_spec MATCHES "${tags_regex}")
+        set(tag "${CMAKE_MATCH_1}")
+        string(REGEX REPLACE "\\[|\\]" "" tag ${tag})
+        list(APPEND clean_tags ${tag})
       endif()
+    endforeach()

-      string(REPLACE "\n" ";" tests "${tests}")
-      set(test_list "")
-
-      foreach(test ${tests})
-        list(APPEND test_list "${prefix}${test}${suffix}")
-      endforeach()
-
-      add_command(set_property TEST
-        ${test_list}
-        APPEND PROPERTY LABELS "${tag}"
-      )
-    endif()
+    add_command(set_tests_properties
+      ${prefix}${test}${suffix}
+      PROPERTIES LABELS "${clean_tags}"
+    )
   endforeach()
   set("${OUT}" "${script}" PARENT_SCOPE)
endfunction()

[sourcedelica]

Seems to work ok with CMake 3.16.2, even with tags with spaces in them. What is a case that would break pre-3.18 that the "CMake upstream issue" fixes?

[Quincunx271]

It may have worked, but it was breaking the documented rules. To quote the CMake upstream issue:

The documentation for add_test(NAME ...) states, "The test name may not contain spaces, quotes, or other characters special in CMake syntax."

And also:

While this currently appears to work, set_property(TEST ...) and get_property(TEST ...) do not work with these test names, but set_tests_properties(...) kind of works in a broken way (I noticed it appending to LABELS rather than just overwriting).

So it might actually work pre-3.18, but relies on unintentional behavior.

---

For that matter, there is another issue with the set_test_properties(...) instead of the set_property(TEST ... APPEND ...) that I hadn't previously remembered: I seem to recall that there is some way to already set properties on the test before these added labels appear. If that is the case, then set_test_properties will erase the labels rather than adding to it. It would probably be a good idea to first get_test_property so that we can join the two LABEL lists when we set_test_properties.

[sourcedelica]

Ok - I'll add get_test_property.

Is there any backwards-compatibility logic that will need to be added for pre-3.18 CMake versions?

[Quincunx271]

Is there any backwards-compatibility logic that will need to be added for pre-3.18 CMake versions?

I would probably include a CMake version check that skips the label management if it didn't work. What I mean is, I would work backward to find the earliest CMake version for which the code works, then simply don't add labels if the CMake version is older than that. Or alternatively skip the "work backwards" step and only support this feature for 3.18+

[sourcedelica]

I was starting to think about joining the result of get_test_property but realized that each test is being added (via add_test) in the same exe_tests-SHA1.cmake in which we are adding the labels. There's no opportunity for some other code to add labels before we do, no?

I'll test older versions of CMake and see where it breaks.

[Quincunx271]

Except the user could decide to set properties of their own, e.g. their own labels:

catch_discover_tests(target
  PROPERTIES
    LABEL CustomLabel
)

[sourcedelica]

I see, ok. get_test_property coming... :)

[sourcedelica]

I tried the get_test_property route in the generated code, for example:

get_test_property( [==[utils:Binary Encoding]==] LABELS test_labels)
set_tests_properties( [==[utils:Binary Encoding]==] PROPERTIES LABELS Binary Pack ${test_labels})

But CMake failed with Unknown CMake command "get_test_property"

For example:

$ ctest --print-labels
Test project /home/eric/work/tw-base/cmake-build-debug
CMake Error at /home/eric/work/tw-base/cmake-build-debug/tw/utils/test/utils-test_tests-b858cb2.cmake:64 (get_test_property):
  Unknown CMake command "get_test_property".
Call Stack (most recent call first):
  /home/eric/work/tw-base/cmake-build-debug/tw/utils/test/utils-test_include-b858cb2.cmake:2 (include)
  /home/eric/work/tw-base/cmake-build-debug/tw/utils/test/CTestTestfile.cmake:7 (include)
  /home/eric/work/tw-base/cmake-build-debug/tw/utils/CTestTestfile.cmake:7 (subdirs)
  /home/eric/work/tw-base/cmake-build-debug/tw/CTestTestfile.cmake:8 (subdirs)
  CTestTestfile.cmake:7 (subdirs)

get_property(test_labels TEST [==[utils:Binary Encoding]==] PROPERTIES LABELS) failed with Unknown test: "utils:Binary Encoding" - same problem we had before with set_property(TEST PROPERTIES LABELS).

I had to take another route and add LABELS to catch_discover_tests. There is already a WORKING_DIR argument in the same vein. Otherwise I would have had to parse the PROPERTIES argument and I was not feeling happy about that.

Also I caught a bug in the previous iteration where it was only saving the first label because add_command was generating this:

set_tests_properties( [==[utils:Binary Encoding]==] PROPERTIES LABELS Binary Pack foo bar)

when it should have been

set_tests_properties( [==[utils:Binary Encoding]==] PROPERTIES LABELS "Binary;Pack;foo;bar")

Here's the full patch:

diff --git a/contrib/Catch.cmake b/contrib/Catch.cmake
--- a/contrib/Catch.cmake	(revision 3047ee81052b35f9867b61c15a81c9df2b539621)
+++ b/contrib/Catch.cmake	(date 1627022743690)
@@ -31,6 +31,7 @@
                          [WORKING_DIRECTORY dir]
                          [TEST_PREFIX prefix]
                          [TEST_SUFFIX suffix]
+                         [LABELS arg1...]
                          [PROPERTIES name1 value1...]
                          [TEST_LIST var]
     )
@@ -45,11 +46,11 @@
 
   Additionally, setting properties on tests is somewhat less convenient, since
   the tests are not available at CMake time.  Additional test properties may be
-  assigned to the set of tests as a whole using the ``PROPERTIES`` option.  If
-  more fine-grained test control is needed, custom content may be provided
-  through an external CTest script using the :prop_dir:`TEST_INCLUDE_FILES`
-  directory property.  The set of discovered tests is made accessible to such a
-  script via the ``<target>_TESTS`` variable.
+  assigned to the set of tests as a whole using the ``WORKING_DIR``, ``LABELS``
+  or ``PROPERTIES`` options.  If more fine-grained test control is needed, custom
+  content may be provided through an external CTest script using the
+  :prop_dir:`TEST_INCLUDE_FILES` directory property.  The set of discovered tests
+  is made accessible to such a script via the ``<target>_TESTS`` variable.
 
   The options are:
 
@@ -80,6 +81,10 @@
     every discovered test case.  Both ``TEST_PREFIX`` and ``TEST_SUFFIX`` may
     be specified.
 
+  ``LABELS arg1...``
+    Specifies labels to be added to each test case in addition to the labels
+    discovered by ``catch_discover_tests``.
+
   ``PROPERTIES name1 value1...``
     Specifies additional properties to be set on all tests discovered by this
     invocation of ``catch_discover_tests``.
@@ -98,7 +103,7 @@
     ""
     ""
     "TEST_PREFIX;TEST_SUFFIX;WORKING_DIRECTORY;TEST_LIST"
-    "TEST_SPEC;EXTRA_ARGS;PROPERTIES"
+    "TEST_SPEC;EXTRA_ARGS;LABELS;PROPERTIES"
     ${ARGN}
   )
 
@@ -130,6 +135,7 @@
             -D "TEST_WORKING_DIR=${_WORKING_DIRECTORY}"
             -D "TEST_SPEC=${_TEST_SPEC}"
             -D "TEST_EXTRA_ARGS=${_EXTRA_ARGS}"
+            -D "TEST_LABELS=${_LABELS}"
             -D "TEST_PROPERTIES=${_PROPERTIES}"
             -D "TEST_PREFIX=${_TEST_PREFIX}"
             -D "TEST_SUFFIX=${_TEST_SUFFIX}"
@@ -147,7 +153,7 @@
     "endif()\n"
   )
 
-  if(NOT ${CMAKE_VERSION} VERSION_LESS "3.10.0") 
+  if(NOT ${CMAKE_VERSION} VERSION_LESS "3.10.0")
     # Add discovered tests to directory TEST_INCLUDE_FILES
     set_property(DIRECTORY
       APPEND PROPERTY TEST_INCLUDE_FILES "${ctest_include_file}"
diff --git a/contrib/CatchAddTests.cmake b/contrib/CatchAddTests.cmake
--- a/contrib/CatchAddTests.cmake	(revision 3047ee81052b35f9867b61c15a81c9df2b539621)
+++ b/contrib/CatchAddTests.cmake	(date 1627024069780)
@@ -5,10 +5,12 @@
 set(suffix "${TEST_SUFFIX}")
 set(spec ${TEST_SPEC})
 set(extra_args ${TEST_EXTRA_ARGS})
+set(labels ${TEST_LABELS})
 set(properties ${TEST_PROPERTIES})
 set(script)
 set(suite)
 set(tests)
+set(test_names)
 
 function(add_command NAME)
   set(_args "")
@@ -68,6 +70,7 @@
     ${properties}
   )
   list(APPEND tests "${prefix}${test}${suffix}")
+  list(APPEND test_names "${test}")
 endforeach()
 
 # Create a list of all discovered tests, which users may use to e.g. set
@@ -77,43 +80,33 @@
 # Run executable to get list of tags
 function(get_tags OUT)
   set(script)
-  execute_process(
-    COMMAND ${TEST_EXECUTOR} "${TEST_EXECUTABLE}" ${spec} --list-tags
-    OUTPUT_VARIABLE tags
-    RESULT_VARIABLE result
-  )
-  if(${result} LESS 0)
-    return() # If we can't figure out the tags, that's fine, don't add labels
-  endif()
+
+  foreach(test ${test_names})
+    execute_process(
+      COMMAND ${TEST_EXECUTOR} "${TEST_EXECUTABLE}" "${test}" --list-tags
+      OUTPUT_VARIABLE tags
+      RESULT_VARIABLE result
+    )
+    if(${result} LESS 0)
+      continue() # If we can't figure out the tags, that's fine, don't add labels
+    endif()
 
-  string(REPLACE "\n" ";" tags "${tags}")
-  set(tags_regex "(\\[([^\\[]*)\\])")
+    string(REPLACE "\n" ";" tags "${tags}")
+    set(tags_regex "(\\[([^\\[]*)\\])")
+    set(clean_tags)
+    list(APPEND clean_tags ${labels})
 
-  foreach(tag_spec ${tags})
-    # Note that very long tags line-wrap, which won't match this regex
-    if(tag_spec MATCHES "${tags_regex}")
-      set(tag "${CMAKE_MATCH_1}")
-
-      execute_process(
-        COMMAND ${TEST_EXECUTOR} "${TEST_EXECUTABLE}" ${spec} "${tag}" --list-test-names-only
-        OUTPUT_VARIABLE tests
-        RESULT_VARIABLE result
-      )
-      if(${result} LESS 0)
-        continue() # If we can't figure out the related tests, abort for this tag.
+    foreach(tag_spec ${tags})
+      # Note that very long tags line-wrap, which won't match this regex
+      if(tag_spec MATCHES "${tags_regex}")
+        set(tag "${CMAKE_MATCH_1}")
+        string(REGEX REPLACE "\\[|\\]" "" tag ${tag})
+        list(APPEND clean_tags ${tag})
       endif()
-
-      string(REPLACE "\n" ";" tests "${tests}")
-      set(test_list "")
-
-      foreach(test ${tests})
-        list(APPEND test_list "${prefix}${test}${suffix}")
-      endforeach()
+    endforeach()
 
-      add_command(set_property TEST
-        ${test_list}
-        APPEND PROPERTY LABELS "${tag}"
-      )
+    if(clean_tags)
+       set(script "${script}set_tests_properties([==[${prefix}${test}${suffix}]==] PROPERTIES LABELS \"${clean_tags}\")\n")
     endif()
   endforeach()
   set("${OUT}" "${script}" PARENT_SCOPE)

[sourcedelica]

I tested with CMake versions back to CMake 3.4.3.

Where there are multiple test executables that catch_discover_tests are called for then the label support (including test names and tags with spaces in them) works going back through CMake 3.10.0.

If there are multiple test executables then the second call to catch_discover_tests fails with the following error starting in Catch 3.9.6:

CMake Error at cmake/Catch.cmake:169 (message):
  Cannot set more than one TEST_INCLUDE_FILE
Call Stack (most recent call first):
  CMakeLists.txt:37 (catch_discover_tests)

Label support works for a single call to catch_discover_tests going back through CMake 3.5.2. catch_discover_tests starts failing at CMake 3.4.3 with the error:

CMake Error at cmake/Catch.cmake:102 (cmake_parse_arguments):
  Unknown CMake command "cmake_parse_arguments".
Call Stack (most recent call first):
  CMakeLists.txt:34 (catch_discover_tests)

[sourcedelica]

I posted a question on the CMake Discourse: https://discourse.cmake.org/t/unknown-cmake-command-get-test-property-in-test-include-files/3828 about the get_test_property and {get,set}_property(TEST APPEND PROPERTY LABELS) failures.

[sourcedelica]

From that discussion came this CMake issue: https://gitlab.kitware.com/cmake/cmake/-/issues/22473

[jdumas]

@sourcedelica what's the status on this one? Is your patch ready to be applied, or does it need upstream modifications from CMake first? I tried to apply your patch, but didn't find commit 3047ee81.

[Quincunx271]

I tried to apply your patch, but didn't find commit 3047ee8

It's in my fork: https://github.com/Quincunx271/Catch2/tree/fix-catch-discover-tests-with-labels-from-tags

https://github.com/Quincunx271/Catch2/commit/3047ee81052b35f9867b61c15a81c9df2b539621

[sourcedelica]

@jdumas - it's ready to be applied as is @Quincunx271's branch.

[jdumas]

Ok thanks. Could this be merged into the main repo then? Seems the upstream CMake issue that was blocking this has been fixed now.

[jcn509]

Is there any update on this?