[cmake-developers] [CMake] Setup/tear down steps for CTest
Craig Scott
craig.scott at crascit.com
Thu Sep 8 10:15:17 EDT 2016
I should also point out that another reason for not implementing the
"skipping tests if the setup fails logic" relates to the current behaviour
of DEPENDS. At the moment, if test B depends on test A, test B still
executes if test A fails. This is both useful and unexpected at the same
time. It is unexpected because I'd initially have thought of DEPENDS as
meaning I can't run test B if test A fails, after all, B depends on A which
I'd interpret to mean if A fails, then something B requires isn't working.
Conversely, this is also useful because until now, DEPENDS was the only way
to get cleanup functionality to run after other tests, and if those other
tests fail, we still want the cleanup to occur.
Current behaviour of DEPENDS can't change because there would be too much
out there in the wild relying on the existing behaviour. I'm wondering if
there's merit in adding a DEPENDS_ON_SUCCESS test property or something
similar which would implement the perhaps more intuitive behaviour of not
running dependent tests when a dependee fails. If that was done, then
implementing the "don't run fixture tests if any fixture setup fails" logic
would be trivial.
On Thu, Sep 8, 2016 at 6:08 PM, Craig Scott <craig.scott at crascit.com> wrote:
> Merge request implementing this feature is now up for review here:
>
> https://gitlab.kitware.com/cmake/cmake/merge_requests/88
>
> I ended up going with FIXTURE_... test property names rather than
> GROUP_... since it seemed more specific. I have not implemented the logic
> for skipping regular tests if any of a fixture's setup tests fail as that
> would require more change than I wanted to bite off for this initial
> implementation. If it is really required, I guess it could be done, but my
> primary concern first is not to introduce new bugs. ;)
>
>
>
> On Thu, Sep 1, 2016 at 9:17 AM, Craig Scott <craig.scott at crascit.com>
> wrote:
>
>> Actually, we can't really re-use the RESOURCE_LOCK for the proposed
>> RESOURCE_SETUP and RESOURCE_CLEANUP functionality since that would force
>> all the tests using that resource to be serialised. So yes, a separate
>> GROUP or something similar would seem to be needed. Let me amend my earlier
>> proposal (which is an evolution of Ben's) to something like this:
>>
>>
>> add_test(NAME setup-foo ...)
>> set_tests_properties(setup-foo PROPERTIES GROUP_SETUP foo)
>>
>> add_test(NAME cleanup-foo ...)
>> set_tests_properties(cleanup-foo PROPERTIES GROUP_CLEANUP foo)
>>
>> add_test(NAME use-foo ...)
>> set_tests_properties(use-foo PROPERTIES GROUP foo)
>>
>>
>> The logic would be as follows:
>>
>> - Any test cases with a GROUP_SETUP property for a group will be run
>> before any test cases with GROUP or GROUP_CLEANUP for that same group. The
>> order of these setup test cases can be controlled with the existing DEPENDS
>> test property.
>> - If any of the group's setup test cases fail, all other test cases
>> for that group will be skipped. All cleanup test cases for the group
>> probably should still be run though (it could be hard to try to work out
>> which cleanup tests should run, so maybe conservatively just run all of
>> them).
>> - If all setup test cases passed, then run all test cases for that
>> group. Regardless of the success or failure of these test cases, once they
>> are all completed, run all the cleanup test cases associated with the group.
>> - Ordering of cleanup test cases can again be controlled with the
>> existing DEPENDS test property.
>>
>> What the above buys us is that CTest then knows definitively that if it
>> is asked to run a test case from a particular group, it must also run the
>> setup and cleanup test cases associated with that group, regardless of
>> whether those setup/cleanup test cases are in the set of test cases CTest
>> was originally asked to run. At the moment, CTest could theoretically do
>> that for the setup steps based on DEPENDS functionality, but not the
>> cleanup. The above proposal is very clear about the nature of the
>> dependency and gives the symmetry of both setup and cleanup behaviour.
>>
>> I'm not tied to the terminology of "GROUP" for tying a set of test cases
>> to their setup/cleanup tasks, so I'm happy to consider alternatives. I'm
>> also wondering whether simply GROUP for a test property is too generic for
>> the test cases that require the setup/cleanup (as opposed to the test cases
>> that ARE the setup/cleanup).
>>
>>
>> On Thu, Sep 1, 2016 at 10:50 AM, Craig Scott <craig.scott at crascit.com>
>> wrote:
>>
>>> In my original thinking, I was of the view that if a setup/cleanup step
>>> needed to be executed for each test rather than for the overall test run as
>>> a whole, then perhaps the test itself should handle that rather than CMake.
>>> The existing RESOURCE_LOCK functionality could then be used to prevent
>>> multiple tests from running concurrently if they would interfere with each
>>> other. Existing test frameworks like GoogleTest and Boost Test already have
>>> good support for test fixtures which make doing this per-test setup/cleanup
>>> easy. The problem I want to solve is where a group of tests share a common
>>> (set of) setup/cleanup steps and CMake knows to run them when asked to run
>>> any test cases that require them. The specific problem motivating this work
>>> was running ctest --rerun-failed, where we need CMake to add in any
>>> setup/cleanup steps required by any of the tests that will be rerun. With
>>> that in mind, see further comments interspersed below.
>>>
>>>
>>> On Fri, Aug 26, 2016 at 12:08 AM, Ben Boeckel <ben.boeckel at kitware.com>
>>> wrote:
>>>
>>>> On Tue, Aug 23, 2016 at 08:00:09 +0200, Rolf Eike Beer wrote:
>>>> > Am Dienstag, 23. August 2016, 10:06:01 schrieb Craig Scott:
>>>> > > So how would you want the feature to work? I'd suggest an initial
>>>> set of
>>>> > > requirements something like the following:
>>>> > >
>>>> > > - Need to support the ability to define multiple setup and/or
>>>> tear down
>>>> > > tasks.
>>>> > > - It should be possible to specify dependencies between setup
>>>> tasks and
>>>> > > between tear down tasks.
>>>> > > - Individual tests need to be able to indicate which setup
>>>> and/or tear
>>>> > > down tasks they require, similar to the way DEPENDS is used to
>>>> specify
>>>> > > dependencies between test cases.
>>>> > > - When using ctest --rerun-failed, ctest should automatically
>>>> invoke any
>>>> > > setup or tear down tasks required by the test cases that will be
>>>> re-run.
>>>> > > - Setup or tear down tasks which reference executable targets
>>>> should
>>>> > > substitute the actual built executable just like how
>>>> add_custom_command()
>>>> > > does.
>>>> >
>>>> > -need a way to mark if 2 tests with the same setup/teardown can share
>>>> those or
>>>> > if they need to run for every of them
>>>>
>>>> Proposal:
>>>>
>>>> add_test(NAME setup-foo ...)
>>>> set_tests_properties(setup-foo PROPERTIES
>>>> SETUP_GROUP foo
>>>> SETUP_STEP SETUP_PER_TEST) # Also SETUP_ONCE.
>>>> add_test(NAME use-foo ...)
>>>> set_tests_properties(use-foo PROPERTIES
>>>> SETUP_GROUP foo) # implicit depends on all SETUP_GROUP foo /
>>>> SETUP_STEP SETUP_* tests.
>>>> add_test(NAME use-foo2 ...)
>>>> set_tests_properties(use-foo2 PROPERTIES
>>>> SETUP_GROUP foo)
>>>> add_test(NAME teardown-foo2 ...)
>>>> set_tests_properties(teardown-foo2 PROPERTIES
>>>> SETUP_GROUP foo
>>>> SETUP_STEP TEARDOWN) # implicit depends on all non-TEARDOWN steps
>>>>
>>>> Multiple setup/teardown steps could be done with DEPENDS between them.
>>>>
>>>
>>> I like the idea of tests being associated with a group and the group
>>> itself is where the setup/cleanup steps are attached/associated. That said,
>>> it would seem that RESOURCE_LOCK already more or less satisfies this
>>> concept. I'm wondering if we can't just somehow attach setup/cleanup steps
>>> to the named resource instead. That would be a more seamless evolution of
>>> the existing functionality and have little impact on any existing code.
>>> Basically all we'd need to do is add the ability to associate the
>>> setup/cleanup steps with a RESOURCE_LOCK label.
>>>
>>> It's still not clear to me whether the setup/cleanup tasks should be
>>> considered test cases themselves, but I can see benefits with taking that
>>> path. It would mean all we'd need is to be able to mark a test case as
>>> "this is a setup/cleanup step for RESOURCE_LOCK label XXX", maybe something
>>> like this:
>>>
>>> set_tests_properties(setup-foo PROPERTIES RESOURCE_SETUP foo)
>>> set_tests_properties(teardown-foo PROPERTIES RESOURCE_CLEANUP foo)
>>>
>>> If multiple setup/cleanup steps are defined for a particular resource,
>>> then dependencies between those test cases would determine their order and
>>> where there are no dependencies, the order would be undefined as is already
>>> the case for test cases.
>>>
>>> For the initial implementation at least, I think something like the
>>> SETUP_PER_TEST concept is more complexity than I'd want to tackle. Maybe it
>>> could be supported later, but in the first instance I think once per
>>> group/resource is already a significant win and worth focusing on at the
>>> start (see my motivation at the top of this email).
>>>
>>>
>>>
>>>>
>>>> > -the default for each test is "no s/t", which means it can't be run
>>>> with any
>>>> > of the above in parallel (especially for compatibillity)[1]
>>>> > -need a way to tell if a test doesn't care about those
>>>>
>>>> Making RESOURCE_LOCK a rwlock rather than a mutex might make sense here.
>>>> SETUP_STEP bits have a RESOURCE_LOCK_WRITE group_${group}, otherwise it
>>>> is RESOURCE_LOCK_READ group_${group}.
>>>>
>>>
>>> Not sure I follow what problem this solves and without a strong
>>> motivation, I'd be reluctant to add this sort of complexity to the existing
>>> RESOURCE_LOCK functionality. It's currently quite clean and easy to
>>> understand. If a test uses some resource, it specifies it in RESOURCE_LOCK.
>>> The proposal above to add setup/cleanup logic to a resource doesn't require
>>> differentiating readers and writers (but I'm happy to consider examples
>>> which do demonstrate the need).
>>>
>>>
>>>>
>>>> > 1) think of a database connector test: the test that will check what
>>>> happens
>>>> > if no DB is present will fail if the setup step "start DB" was run,
>>>> but not
>>>> > the teardown
>>>>
>>>> RESOURCE_LOCK on that group_${group} can fix that I think.
>>>>
>>>
>>> And this is indeed precisely the motivating situation that got me into
>>> this thread. We currently use the RESOURCE_LOCK to prevent concurrent
>>> access to a DB instance, with starting up a clean instance at the beginning
>>> and shutting it down again at the end of all tests being what I want to
>>> move into the proposed setup/cleanup tasks. The current functionality
>>> requires us to use both RESOURCE_LOCK and DEPENDS to specify the same thing
>>> and it doesn't cover the ctest --rerun-failed scenario. With the proposal
>>> above to use RESOURCE_SETUP and RESOURCE_CLEANUP test properties, this
>>> could create an implicit dependency on those setup/cleanup test cases just
>>> by using RESOURCE_LOCK on the test cases which use that resource (i.e. no
>>> need for the separate DEPENDS to be specified as it does now).
>>>
>>>
>>> --
>>> Craig Scott
>>> Melbourne, Australia
>>> http://crascit.com
>>>
>>
>>
>>
>> --
>> Craig Scott
>> Melbourne, Australia
>> http://crascit.com
>>
>
>
>
> --
> Craig Scott
> Melbourne, Australia
> http://crascit.com
>
--
Craig Scott
Melbourne, Australia
http://crascit.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/cmake-developers/attachments/20160908/c1590e59/attachment-0001.html>
More information about the cmake-developers
mailing list