Open zachcran opened 3 weeks ago
I think we should be careful with auto inclusion. Strictly speaking I don't think users have to use CTest with the majority of our features. The bigger potential problem is the inclusion of CTest defines the testing root.
That said the 'ct_add_dir' feature definitely assumes it, so I think it makes sense to auto include it. My points are more that I think this requires more testing than it may have first appeared.
That makes sense that more testing is needed. Are you saying that it would be possible for someone to set up testing for their project without including CTest, then? If so, could you provide an example of how that would look without using ct_add_dir()
?
@zachcran One way to do it would be to duplicate the implementation of ct_add_dir
, but instead of calling add_test
manually run the CMake command it wraps directly. I'm not saying it's a great option, but I think it's theoretically possible.
@ryanmrichard That does sound like a bit of a pain. I would recommend putting more detail in the Hello World tutorial to help a user get their first project set up properly, or add another tutorial beforehand for setting up the project. Hello World is the first tutorial most users are going to look at and it does not give clear, step-by-step instructions to go from no project to a project with a simple first unit test. Following the instructions there, I had to take a couple creative liberties and include CTest to get it to work, so a more step-by-step tutorial is probably in order.
I can take a crack at it if that sounds reasonable to you. The only challenge I foresee is the autogenerated tutorial docs. What is the tool used, and are there any quirks I should watch out for?
There could also be a separate tutorial that explains how to use CMakeTest without CTest, in case someone does not want to include it, but I don't have the knowledge of this code base to do that.
@ryanmrichard That does sound like a bit of a pain. I would recommend putting more detail in the Hello World tutorial to help a user get their first project set up properly, or add another tutorial beforehand for setting up the project. Hello World is the first tutorial most users are going to look at and it does not give clear, step-by-step instructions to go from no project to a project with a simple first unit test. Following the instructions there, I had to take a couple creative liberties and include CTest to get it to work, so a more step-by-step tutorial is probably in order.
I can take a crack at it if that sounds reasonable to you. The only challenge I foresee is the autogenerated tutorial docs. What is the tool used, and are there any quirks I should watch out for?
It's possible ct_add_test
and the like also assume CTest. I suspect it is highly unlikely that someone will want to use CMakeTest without CTest, so if that's the only hold-up for auto-including CTest then I'm fine just auto-including it.
@ryanmrichard As far as I can tell, ct_add_dir
is the only place where CTest is assumed by calling add_test()
from CTest (here). Without including CTest, the test files are still written to the build directory, and I confirmed that I can still run the cmake -B. -S src
command CTest would use on them to run the test and report if it passes. The tests would just have to be run one-by-one manually or have additional code written to run them (like making a test executable or script that you run in the build directory that executes all of the tests... pretty much just recreating CTest).
Is your feature request related to a problem? Please describe. I noticed that my CMake tests are not built unless I include CTest in my CMakeLists.txt. Only then are any tests found when I run
ctest --output-on-failure
in my build directory.Describe the solution you'd like Currently, I add
include(CTest)
before any calls toct_add_dir()
and this fixes the issue. Is it possible to automatically include CTest when the project grabs CMakeTest?Describe alternatives you've considered Manually include CTest in my project before calling
ct_add_dir()
. This is fine as well, but I could not find documentation saying to do so.