Add support for catch3.5

Laguna1989 commented 6 months ago

See #156 and #219

This should be small, but it might not be considered small

Feature is small, but contains some catch2 version trickeries. I am happy to join a pairing session in case it is needed.

Description

This PR will add support for catch2 v3 alongside catch2 v2.

The solution

Those are also things to check in a review:

A new cmake option is added APPROVAL_TESTS_USE_CATCH2V3
- I have not seen any other cmake options. This might be a deliberate decision that I am not aware of. I used the option approach as it is considered "modern" cmake. If this is an issue, please raise a review finding and let me know how it should be implemented.
- I do use cmake CPM to pull catch2 v3.5. This avoid duplication of catch2 code. I would suggest to use CPM also to pull catch2 v2, but I wanted to keep the PR minimal. If it should be added, let me know and I will add it. Instead if catch2 sources are expected to be duplicated in this repo, I can also add them.
Catch2 API has changed between v2 and v3. This needs to be accounted for in some places.
- Whenever dispatching was needed, I tried to put this into a separate file (catch2_include.h) or a separate function (check_contains) to avoid duplication
- Potentially catch2 API might change between minor versions, which will cause issues for users. I have tested specifically for catch2 v3.5.0, but other users of ApprovalTests.cpp might not be able to select the catch2 version as they like, which could cause issues further down the line.
My IDE and clang-format desperately wanted to reformat some of the files a bit differently. If this creates too much visual clutter in the PR, let me know and I will match the previous formatting.

claremacrae commented 6 months ago

Wow. Thank you very much indeed @Laguna1989.

I'm next pairing with Llewellyn on Tuesday 16th January - 17:00 UK time...

Might you be able to join us for that session, so that we can review and understand the changes?

Laguna1989 commented 6 months ago

@claremacrae That should work for me. I have blocked the date in my calendar.

Laguna1989 commented 6 months ago

I figured out the clang format issue. Seems that the clang-format 9 from the build server is formatting things differently than my local clang-format 14. I did fix that in the last commit and also slightly extended the clang-format github action so it shows the diff. That will make it easier to track similar issues.

-> link to successful clang format run

Laguna1989 commented 5 months ago

Wow. Thank you very much indeed @Laguna1989.

I'm next pairing with Llewellyn on Tuesday 16th January - 17:00 UK time...

Might you be able to join us for that session, so that we can review and understand the changes?

Hi @claremacrae : What are the circumstances for the pairing session? Could you kindly send me the meeting details for tomorrows session?

claremacrae commented 5 months ago

Hi @claremacrae : What are the circumstances for the pairing session? Could you kindly send me the meeting details for tomorrow's session?

Thanks. Sure - I've replied to the email address in your GitHub profile.

claremacrae commented 5 months ago

Those are also things to check in a review:

Thank you!

A new cmake option is added APPROVAL_TESTS_USE_CATCH2V3

I have not seen any other cmake options. This might be a deliberate decision that I am not aware of. I used the option approach as it is considered "modern" cmake. If this is an issue, please raise a review finding and let me know how it should be implemented.

The docs about CMake with this project are rendered in:

https://approvaltestscpp.readthedocs.io/en/latest/generated_docs/CMakeIntegration.html

And the source file is:

https://github.com/approvals/ApprovalTests.cpp/blob/master/doc/CMakeIntegration.md

isidore commented 5 months ago

done in #221

approvals / ApprovalTests.cpp