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

Make building tests an option in CMake #385

Merged
merged 2 commits into from
Jan 15, 2023

Conversation

FabienPean
Copy link
Contributor

@FabienPean FabienPean commented Dec 10, 2022

Tests were built by default with CMake, it is now an option set to ON by default to preserve previous behavior.

@FabienPean FabienPean marked this pull request as ready for review December 10, 2022 20:06
@fghoussen
Copy link
Collaborator

Can you put the previous code into a cmake function and call that function if the option is ON

@FabienPean
Copy link
Contributor Author

Is that the change you meant (should the function be defined there) ? It looks a bit weird, did you mean refactor the previous code to follow the "Examples" part of the CMakeLists instead ?

@fghoussen
Copy link
Collaborator

Would be good to have equivalent of function(examples list_name) and function(pexamples list_name) but not sure if this can be easy done for all tests.

@FabienPean
Copy link
Contributor Author

    enable_testing()
    set(CMAKE_CTEST_COMMAND ctest -V)   

had to be moved in global scope of the script because the CMake function creates a new scope. To keep it fully apart, it should be using the macro/endmacro couple instead of the function/endfunction.

Now with regard to the CI, for the failing job macos_latest_cmake_python, it looks like a problem resulting from the transition to macOS 12.

@fghoussen
Copy link
Collaborator

CI seems broken for other reasons: we'll have to fix it in a dedicated PR before merging this.

@sylvestre sylvestre merged commit af09793 into opencollab:master Jan 15, 2023
@FabienPean FabienPean deleted the patch-2 branch January 15, 2023 19:33
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.

3 participants