Skip to content

Adds find_package(Catch2) to test_nodes CMakeLists#486

Closed
tatatupi wants to merge 3 commits intomasterfrom
fix-catch2-config
Closed

Adds find_package(Catch2) to test_nodes CMakeLists#486
tatatupi wants to merge 3 commits intomasterfrom
fix-catch2-config

Conversation

@tatatupi
Copy link
Copy Markdown
Collaborator

@tatatupi tatatupi commented Aug 5, 2025

Type of change

  • Bug fix
  • Documentation/refactoring

Description

In case you have Catch2 installed and the correct system variable for Catch2_DIR, the test_nodes lib won't find the Catch2 package without the explicit directive to see it.
This PR includes find_package(Catch2) to test_nodes CMakeLists.txt to fix that.
And it also adds instructions in README.txt, to create the Catch2_DIR system variable.

Testing

  • Qt version tested:
  • Existing tests still pass

Breaking changes?

No

Related issue

None.

@tatatupi
Copy link
Copy Markdown
Collaborator Author

tatatupi commented Aug 5, 2025

needs more elaboration

@tatatupi tatatupi closed this Aug 5, 2025
@paceholder
Copy link
Copy Markdown
Owner

CMakeListst.txt - line 50
add_subdirectory(external) -- there we run the script external/CMakeLists.txt which already searches for catch2.

This is maybe not very clear from the name (external), maybe we should move the fings outside the external directory

@tatatupi
Copy link
Copy Markdown
Collaborator Author

tatatupi commented Aug 6, 2025

The problem is that I have Catch2 3.6.0 installed, and the configuration won't work due to the different version. I propose we revert the find_package(Catch2 2.13.7 QUIET) with an explicit version.

And when the system package is found, there should be a find_package(Catch2) inside the test CMakeLists.

Is there a reason you removed the explicit Catch2 version from find_package?

@tatatupi tatatupi reopened this Aug 6, 2025
@tatatupi tatatupi closed this Sep 5, 2025
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.

2 participants