ApprovalTests.cpp icon indicating copy to clipboard operation
ApprovalTests.cpp copied to clipboard

Add support for Catch2 v3 and devel branch

Open claremacrae opened this issue 5 years ago • 13 comments

A user reports that the current behaviour is:

ApprovalTests/include/ApprovalTests.v.10.3.0.hpp:1928:10: fatal error: 'catch2/catch.hpp' file not found
#include <catch2/catch.hpp>

We would need to work out which new headers to include, and how to detect when we need to use them.

claremacrae avatar Sep 15 '20 10:09 claremacrae

At least on macOS, and with my setup, it compiles if you replace this with the following includes:

#include <catch2/catch_test_case_info.hpp>
#include <catch2/reporters/catch_reporter_bases.hpp>
#include <catch2/catch_reporter_registrars.hpp>

However, I think you would also need to wrap that into some kind of macro that checks whether you're using Catch2 v3, and I currently don't know how to do that.

timuraudio avatar Sep 15 '20 11:09 timuraudio

The above build error happened with https://github.com/catchorg/Catch2/commit/24b83edf

When I used https://github.com/catchorg/Catch2/releases/tag/v3.0.0-preview1 I believe I was able to use catch2/catch.hpp correctly - so maybe diffing these two revisions might show up what has changed.

claremacrae avatar Sep 15 '20 11:09 claremacrae

At least on macOS, and with my setup, it compiles if you replace this with the following includes:

#include <catch2/catch_test_case_info.hpp>
#include <catch2/reporters/catch_reporter_bases.hpp>
#include <catch2/catch_reporter_registrars.hpp>

Super - thanks @timuraudio

However, I think you would also need to wrap that into some kind of macro that checks whether you're using Catch2 v3, and I currently don't know how to do that.

That's OK - this is really helpful and I think I should be able to figure something out.

claremacrae avatar Sep 15 '20 11:09 claremacrae

This is more work than I first realised, as they've changed the name of the catch2 include.... and <catch2/catch.hpp> no longer exists.

So all the ApprovalTests code that uses the Catch2 single header needs to look something like this:

#if ???
    #include <catch2/catch.hpp>
#else
    #include <catch2/catch_all.hpp>
#endif

When I was on the Catch2 Discord, I had a chat about how to maintain code which is compatible with both v2 and v3, and the options offered were:

  • "don't" support both
  • use __has_include to check which header is available - but that's only C++17

The only other thing that I can think of is to introduce a new integration, so that users can do one of:

#define APPROVALS_CATCH_EXISTING_MAIN
#include "ApprovalTests.hpp"

or this - note the added V3

#define APPROVALS_CATCHV3_EXISTING_MAIN
#include "ApprovalTests.hpp"

claremacrae avatar Nov 14 '20 21:11 claremacrae

I'm not happy with any of those options.

claremacrae avatar Nov 14 '20 21:11 claremacrae

Other suggestions gratefully received!

claremacrae avatar Nov 14 '20 21:11 claremacrae

The way the Compiler Explorer does this is to treat Catch2 as 2 libraries:

  • catch2v2
  • catch2

claremacrae avatar Mar 29 '21 15:03 claremacrae

Putting on hold until catch2 v3 is officially released...

claremacrae avatar Aug 30 '21 15:08 claremacrae

Catch2 v3 seems to be officially released. Any updates on this?

gerboengels avatar Jun 08 '22 08:06 gerboengels

At least on macOS, and with my setup, it compiles if you replace this with the following includes:

#include <catch2/catch_test_case_info.hpp>
#include <catch2/reporters/catch_reporter_bases.hpp>
#include <catch2/catch_reporter_registrars.hpp>

I played around with it a bit and unfortunately it seems this is not so easy in 2022 anymore. I took a look at catch2 v3.5.0 and there some renames have taken place. E.g. Catch::TestEventListenerBase is now called Catch::EventListenerBase. After fixing this, some other classes raised compilation errors, namely some catch2 matchers and something related to IConfig.

I dawns on me that I am not very well familiar with either the catch2 or ApprovalTest libraries to provide a meaningful fix. If someone with more experience wants to tackle this, I am happy to support e.g. via a pairing, testing, review or as a rubber duck.

Laguna1989 avatar Dec 24 '23 21:12 Laguna1989

In fact it is not that hard. Seems that if -DCATCH_CONFIG_DISABLE=ON is used as a cmake argument, the errors vanish.

However when executing the "Catch2_Tests" target with catch2 v3.5.0, there are no test found. :(

https://github.com/Laguna1989/ApprovalTests.cpp/tree/feature/catch3.5

Let's see if this evolves into to a meaningful contribution (after the holidays).

Laguna1989 avatar Dec 24 '23 22:12 Laguna1989

Update:

  • disabling config is not an option, as this just sets the CATCH_REGISTER_LISTENER and CATCH_REGISTER_REPORTER to empty macros, leading to no tests being registered at all
  • The errors mentioned above are just an issue with the matcher in the ApprovalsTest.cpp test code, not an implementation issue.
    • The matcher Catch::Matchers::Contains is no longer supporting strings in catch2 v3.5. There was the new matcher introduced: Catch::Matchers::ContainsSubstring, which needs to be used for strings.

I created a PR for catch2 v3 integration on my fork and it seems the tests are passing for both catch2 v2 and catch2 v3. :tada:

Unfortunately the clang-format job fails with the following error and I don't know what causes the error or how to fix this. I commented out the needs: clang-format in the workflow file to get automatic verification.

Error: Error: fatal: You are not currently on a branch.

Is this a known issue? As soon as the clang-format job is working, the PR should be good to go into review.

Edit 1: Also the python test seems to be failing :thinking:

Edit 2: And only some gcc jobs are started, clang jobs are mostly ignored?

Laguna1989 avatar Dec 25 '23 08:12 Laguna1989

Happy new year! I did manage to fix one of the python tests about include name prefixes. However the next one is failing because of the ApprovalTests.cpp.StarterProject being outdated. This will need to be solved in a separate PR as it is a different repository.

The clang-format job still fails. It might be the pipeline running on my forked repo (potentially with different permissions?) instead of the official ApprovalTests.cpp repo.

I will create a PR for the official ApprovalTests repo now: #220

Laguna1989 avatar Jan 02 '24 10:01 Laguna1989

This is done and will be released soon. Thanks to @Laguna1989 !

claremacrae avatar Mar 12 '24 18:03 claremacrae