-
Notifications
You must be signed in to change notification settings - Fork 179
Add UTPP_INCLUDE_TESTS_IN_BUILD CMake option #115
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
Add UTPP_INCLUDE_TESTS_IN_BUILD CMake option #115
Conversation
It would be nice if we could also have an option to skip the build part since end users don't need the tests at all. We currently comment out that part: https://gitweb.gentoo.org/repo/gentoo.git/tree/dev-libs/unittest++/unittest++-1.6.1.ebuild#n26 |
I would prefer to see the tests built even if they aren't being run, because if the tests fail to compile that may catch an issue with the package on the end system. It's not as good of a check as actually running them, but it's better than nothing. Think of it as building an "example client" of the library to ensure that everything is okay. I previously removed the usage of CTest because it added complexity to the CMake files for no discernible gain. I've seen CTest be useful for projects that are maintaining large CI matrix builds and submitting results to a CDash dashboard, but no such infrastructure exists for this project, so this is simpler for me as a maintainer. My recommendation is to use the new flag and leave the compiling of the tests in. Alternately, you could download the release (starting with the latest release, v1.6.1) from https://github.com/unittest-cpp/unittest-cpp/releases and use |
Well ebuilds are for end users. End users do not need to compile examples or tests. This is what Gentoo developers do. If someone wants to build the tests, they can but it should not be unconditional.
Fair enough, I just asked out of curiosity :)
I did not see that you had a configure script in recent releases. What does it do differently? |
The configure script is generated by autoreconf, and in turn will generate a Makefile with an |
If you are intending to switch to using the released ./configure script, I'll close this pull request. If you'd like to stick with CMake, I'll make the necessary changes to this PR so you can stop patching the CMake file. Please advise. |
@sbraz could use some additional input on this. I would like to eliminate the need for Gentoo to have to run |
Well what we could do is:
|
Helps address #104.
UTPP_SKIP_TESTS_AS_BUILD_STEP with a default value of OFF was an unfortunate choice, so this changes it to UTPP_RUN_TESTS_AS_BUILD_STEP with a default value of ON.
As part of this, changed the option name to `UTPP_INCLUDE_TESTS_IN_BUILD`. The test target is still added to the build and can be built/run separately using the CMake --target option.
4d7ae2e
to
6b69ed7
Compare
@sbraz I don't want to overcomplicate the build file with any more flags than needed. The latest push includes a single option, |
Well we'll still need to run sed because we don't want to build tests and run them during the same phase. |
Why bother with sed at that point? If the tests fail then the build phase fails. If the tests pass then the build phase passes, and the test phase passes. They take a very very short time to run, so running them twice -- while admittedly wasteful -- is 8000 of little harm. If I were to change it any further, it would be to change the option to be something like |
Merging this as-is, as I can see the utility in it as currently implemented. |
Hi, sorry I didn't reply before. |
Understood. I will happily consider a pull request from another contributor to address this. |
This PR helps address #104 by giving the downstream project the ability to bypass tests at build time. The default behavior remains the same.