approvals / ApprovalTests.cpp

Native ApprovalTests for C++ on Linux, Mac and Windows
https://approvaltestscpp.readthedocs.io/en/latest/
Apache License 2.0
312 stars 51 forks source link

Add support for Catch2 v3 and devel branch #156

Closed claremacrae closed 4 months ago

claremacrae commented 3 years ago

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.

timuraudio commented 3 years ago

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.

claremacrae commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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:

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 commented 3 years ago

I'm not happy with any of those options.

claremacrae commented 3 years ago

Other suggestions gratefully received!

claremacrae commented 3 years ago

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

claremacrae commented 2 years ago

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

gerboengels commented 2 years ago

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

Laguna1989 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

Update:

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 commented 7 months ago

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

claremacrae commented 4 months ago

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