Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added Catch v3 support #2656

Merged
merged 1 commit into from
Feb 28, 2023
Merged

Added Catch v3 support #2656

merged 1 commit into from
Feb 28, 2023

Conversation

xvitaly
Copy link
Contributor

@xvitaly xvitaly commented Feb 28, 2023

Added Catch v3 support with backward compatibility with Catch v2.

@gabime
Copy link
Owner

gabime commented Feb 28, 2023

Thanks. I prefer to keep it simpler by just upgrading the bundled version to catch3. If this works with current tests, we can avoid any cmake changes and ifdefs.

@xvitaly
Copy link
Contributor Author

xvitaly commented Feb 28, 2023

If this works with current tests, we can avoid any cmake changes and ifdefs.

It builds, but the tests fails:

Test project /builddir/build/BUILD/spdlog-1.11.0/redhat-linux-build
    Start 1: spdlog-utests
1/1 Test #1: spdlog-utests ....................Subprocess aborted***Exception:   0.07 sec
spdlog-utests: src/catch2/catch_test_case_info.cpp:147: Catch::TestCaseInfo::TestCaseInfo(Catch::StringRef, const Catch::NameAndTags&, const Catch::SourceLineInfo&): Assertion `inTag' failed.


0% tests passed, 1 tests failed out of 1

Also catch3 is no longer a header-only library. It need to be compiled first.

@xvitaly
Copy link
Contributor Author

xvitaly commented Feb 28, 2023

@gabime
Copy link
Owner

gabime commented Feb 28, 2023

So I think we need to use their recommended approach #2

@xvitaly
Copy link
Contributor Author

xvitaly commented Feb 28, 2023

So I think we need to use their recommended approach 2

I will update my PR.

@xvitaly
Copy link
Contributor Author

xvitaly commented Feb 28, 2023

Done. It will use FetchContent() for fetching catch.

tests/CMakeLists.txt Outdated Show resolved Hide resolved
@xvitaly
Copy link
Contributor Author

xvitaly commented Feb 28, 2023

Missing Git on CI. I will add it.

@xvitaly xvitaly marked this pull request as ready for review February 28, 2023 15:59
@gabime gabime merged commit 2a6d3e9 into gabime:v1.x Feb 28, 2023
gabime added a commit that referenced this pull request Feb 28, 2023
@gabime
Copy link
Owner

gabime commented Feb 28, 2023

@xvitaly Sorry, I had to revert this pr, it doens't pass the CI. I realized this only after the merge. Feel free to fix and open new PR.

@xvitaly
Copy link
Contributor Author

xvitaly commented Mar 1, 2023

I had to revert this pr, it doens't pass the CI. I realized this only after the merge

#2659 will prevent issues like this from happening in the future.

seker pushed a commit to seker/spdlog that referenced this pull request Mar 3, 2023
seker pushed a commit to seker/spdlog that referenced this pull request Mar 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants