[cmake-developers] [CMake] Setup/tear down steps for CTest

Thu Sep 8 06:08:46 EDT 2016

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/cmake-developers/attachments/20160908/b49a0686/attachment-0001.html>