-
Notifications
You must be signed in to change notification settings - Fork 9
Description
This is definitely non-urgent, but probably needs to be resolved at some point in the future.
The number and complexity of tests have reached a point at which AppVeyor starts timing out (it seems that it has a limit of 1 hour or so per build).
One reasonable solution is to only build some limited subset of the tests when building for AppVeyor.
To do that we could add a CMake boolean setting (a.k.a. "option") called `BUILD_FOR_APPVEYOR``. E.g.
option(BUILD_FOR_APPVEYOR "Limit tests to the AppVeyor subset" OFF)
This boolean setting would only be used if BUILD_TESTING
is enabled and would limit the built tests to only the essential subset used by AppVeyor.
Alternatively BUILD_TESTING
could be changed from a boolean to a string with three values, e.g. NONE
, ALL
and APPVEYOR
. Then in CMakeLists.txt it could be set with something like this:
set(BUILD_TESTING NONE CACHE STRING "Which tests to build (NONE, ALL, APPVEYOR)")
Then the CMakeLists.txt files in the tests/ subdirectories, could use BUILD_TESTING
(+ optionally with BUILD_FOR_APPVEYOR
) in order to choose which (if any) tests to build.
To me, the "recipe" tests seem like candidates for the non-essential tests. And I also want to turn examples/connection_pool/
into yet another "recipe" test, but have been postponing this because if the AppVeyor limits. And there are probably quite a few other tests that are "non-essential" and could be skipped when building for AppVeyor.
So what do you think about this approach to solving the AppVeyor issue?
BTW, on a side but related matter, most settings, like BUILD_MYSQL_CONNECTOR
are being set via option(...)
at the beginning of CMakeLists.txt:
Line 37 in 54ca880
option(BUILD_MYSQL_CONNECTOR "Build MySQL Connector" OFF) |
BUILD_TESTING
is not being set via option(BUILD_TESTING ...)
If I understand correctly, the only real difference that this makes, is that this way BUILD_TESTING
is not cached, so it is not persisted across CMake runs (I might be wrong about this, though). I wonder if this (BUILD_TESTING
not being set via option()
) is intentional or just an omission?