Add test tags as cmake label

[uyha]

Description

This allows people running ctest -L 'tag' to run tests matching the tags

GitHub Issues

https://github.com/catchorg/Catch2/issues/1590, https://github.com/catchorg/Catch2/issues/2613

[codecov[bot]]

Codecov Report

Merging #2690 (3cf405b) into devel (733b901) will not change coverage. The diff coverage is n/a.

Additional details and impacted files

@@           Coverage Diff           @@
##            devel    #2690   +/-   ##
=======================================
  Coverage   91.46%   91.46%           
=======================================
  Files         194      194           
  Lines        8188     8188           
=======================================
  Hits         7489     7489           
  Misses        699      699           

[uyha]

@horenmar sorry for pinging, but could you have a look at this PR?

[horenmar]

I want to do #2676 first, and that waits until I have a free weekend that I can spend on it. 🤷

[horenmar]

Well, I finally had time to fix the other PR and unblock this one. I also looked around various other attempts and old issues, and there is one more problem that is not handled by this, and I am not sure it can be handled with this approach, see #1658.

Basically with a long test name like "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Donec eu est ut lacus luctus volutpat. Duis convallis massa non neque eleifend porta vel est.", --list-tests will linebreak the test name roughly like this

  Lorem ipsum dolor sit amet, consectetur adipiscing elit. Donec eu est ut
  lacus luctus volutpat. Duis convallis massa non neque eleifend porta vel est.

This will break the registration script, as it does not understand that it needs to reconstruct the two lines into one test, and in general it cannot figure this out -> if we have test cases without tags, there is no way to tell whether the two lines above are two different test cases, or one looong test case.

And even if it was possible to figure this out (either a different indentation width for subsequent lines, or a line break marker), we still cannot figure out what whitespace separator is supposed to be there (in the example above, it is a space, but it could also be a tab, or even some weirder WS char).

This leads me to conclude that parsing --list-tests will not work, and the script has to do this differently.

As I see it, there are two options

1. Ask for the xml output and parse it with ad-hoc regexes.

This is kinda brittle, but it would work, and IIRC the XML reporter output is already tested for being unchanged, so we could just make it so that the registration script needs to be updated if the XML reporter output changes.

2. Add a JSON reporter and parse that -> CMake has builtin support for JSON since 3.19, so we could use that for work with the structured output.

This requires more changes to Catch2 and forces newer CMake versions.

[uyha]

Add a JSON reporter and parse that -> CMake has builtin support for JSON since 3.19, so we could use that for work with the structured output.

ok, so I guess I should open a new PR following this direction. Are you ok with requiring CMake 3.19?

[uyha]

for adding a JSON reporter, I guess the most straightforward approach is to follow the XML reporter, right? or is there a specification for JSON reporter also?

[LecrisUT]

2. Add a JSON reporter and parse that -> [CMake has builtin support for JSON](https://cmake.org/cmake/help/latest/command/string.html#id2) since 3.19, so we could use that for work with the structured output.

This requires more changes to Catch2 and forces newer CMake versions.

This seems reasonable and modern. But what's the plan on how to get the json parser? Presumably using library, but then would it be a hard dependency? Would it be bundled and/or use FetchContent?

[uyha]

But what's the plan on how to get the json parser?

I think a custom JSON writer will be written, similar to the XML writer.

[LecrisUT]

I think a custom JSON writer will be written, similar to the XML writer.

Maybe the reporter is doable (probably already have a string sanitizer for quotations etc.), but the parser sounds like could be a bit tricky.

[uyha]

I don't think we need a JSON parser in Catch2 code, we only need to use the CMake JSON parser for parsing the tests and tags.

[LecrisUT]

Oh yeah, I misread the previous comment, only the reporter is needed. Isn't there a widely used json format for test-suite data junit? I've recently seen that github action is annotating warnings and some errors directly in the code source. Maybe if there's a format that github action can pick up, it would be nice to have these tests reported in source as well.

[nilsvu]

Hi @uyha, thanks for working on this. Do you have plans to complete this feature? The Catch2+CTest community would thank you :)

[uyha]

@nilsvu yes, you can follow the progress of https://github.com/catchorg/Catch2/pull/2706 for the JSON reporter, after the reporter is done, then this should be trivial.

[uyha]

@horenmar This PR is complete now, as soon as #2706 is complete, this PR should be good to merge too.

[horenmar]

Also the add_command(set ${_TEST_LIST} ${tests}) around line 183 is currently broken, this is the result in the script file:

set( tests_TESTS [==[[
  {
    "class-name" : "",
    "name" : "@Script[C:\\EPM1A]=x;\"SCALA_ZERO:\"",
    "source-location" :
    {
      "filename" : "/mnt/c/ubuntu/Catch2-uyha/tests/TestScripts/DiscoverTests/register-tests.cpp",
      "line" : 11
    },
    "tags" : [ "script regressions" ]
  },
  {
    "class-name" : "",
    "name" : "Some test",
    "source-location" :
    {
      "filename" : "/mnt/c/ubuntu/Catch2-uyha/tests/TestScripts/DiscoverTests/register-tests.cpp",
      "line" : 12
    },
    "tags" : []
  },
  {
    "class-name" : "",
    "name" : "Let's have a test case with a long name. Longer. No, even longer. Really looooooooooooong. Even longer than that. Multiple lines worth of test name. Yep, like this.",
    "source-location" :
    {
      "filename" : "/mnt/c/ubuntu/Catch2-uyha/tests/TestScripts/DiscoverTests/register-tests.cpp",
      "line" : 13
    },
    "tags" : []
  },
  {
    "class-name" : "",
    "name" : "And now a test case with weird tags.",
    "source-location" :
    {
      "filename" : "/mnt/c/ubuntu/Catch2-uyha/tests/TestScripts/DiscoverTests/register-tests.cpp",
      "line" : 16
    },
    "tags" : [ "foo,bar", "tl;dr", "tl;dw" ]
  },
  {
    "class-name" : "",
    "name" : "Test with tags",
    "source-location" :
    {
      "filename" : "/mnt/c/ubuntu/Catch2-uyha/tests/TestScripts/DiscoverTests/register-tests.cpp",
      "line" : 17
    },
    "tags" : [ "a", "b", "c" ]
  },
  {
    "class-name" : "",
    "name" : "Test with different tags",
    "source-location" :
    {
      "filename" : "/mnt/c/ubuntu/Catch2-uyha/tests/TestScripts/DiscoverTests/register-tests.cpp",
      "line" : 18
    },
    "tags" : [ "a", "d", "e" ]
  },
  {
    "class-name" : "",
    "name" : "Test with more tags",
    "source-location" :
    {
      "filename" : "/mnt/c/ubuntu/Catch2-uyha/tests/TestScripts/DiscoverTests/register-tests.cpp",
      "line" : 19
    },
    "tags" : [ "b", "c", "g" ]
  }
]]==] [==[@Script[C:\EPM1A]=x]==] [==["SCALA_ZERO:"]==] [==[Some test]==] [==[Let's have a test case with a long name. Longer. No, even longer. Really looooooooooooong. Even longer than that. Multiple lines worth of test name. Yep, like this.]==] [==[And now a test case with weird tags.]==])

This has two issues

1. The JSON should not be there.
2. The file file with the semicolon in name is split into two elements in the list, which it is not supposed to be.

[horenmar]

Two more general thoughts

1. The iterative handling of JSON in CMake is slooooow. That is because every time there is a call like this string(JSON test_spec GET "${tests}" "${index}"), CMake has to reparse the whole ${tests} string as JSON, check that it is a list, retrieve the element at index and then promptly forget the json parsed representation. For tags it is unlikely to matter, but for tests this might eventually be an issue. Catch2's SelfTest has over 400 test cases, which leads to ~140k JSON file, and I would not consider that test suite to be particularly large. However, fixing this is not required for this to be merged.
2. The changes here introduce hard dependency on relatively new CMake versions (3.19 for JSON support, potentially more around the label handling), so in the end the new script should live side by side with the old one for some time.

[Bidski]

Is there any progress on this?

[uyha]

Is there any progress on this?

I currently am not working on this (no idea when I will resume), so I hope someone can pick this up or maybe I will find the time to work on this at some point.

[LecrisUT]

I could probably take a look and take-over. A todo list of what needs to be done and/or some discussion of how this feature should be tested would help a lot.

[uyha]

I think only the comments from @horenmar above and that should be it.