test scenario adaption (was Re: [MERGE] repository external reference tests (StackedBranches))

[Martin Pool]

On Wed, Jun 11, 2008 at 11:14 AM, Jonathan Lange <jml at mumak.net> wrote:
> On Thu, Jun 5, 2008 at 5:37 PM, Martin Pool <mbp at canonical.com> wrote:
>> On Thu, Jun 5, 2008 at 5:02 PM, Vincent Ladeuil <v.ladeuil+lp at free.fr> wrote:
>>> Err, I really prefer *this* form or any form capturing the test
>>> parameters in a single place in a single class with subclasses
>>> tweaking the scenarios at will.
>>>
>>> If you look at tests/test_http.py you'll see that you can then
>>> decide test class by test class what parametrization you want to
>>> apply.
>>>
>>> Here you separate between all_repository_format_scenarios() and
>>> formats_to_scenarios(), but using adapter classes leave far more
>>> room for variations.
>>
>> Hello Vincent,
>>
>> Thankyou for reading this too.
>>
>> Parameterized tests are pretty powerful but I think we are all going
>> at them in slightly different ways.  So maybe we can try again to
>> settle a standard way, and avoid confusion or debating it for each
>> patch.
>>
>> It seems to me that what api clients might need to do are
>>
>>  1- provide, under a name, a set of scenarios: "all repository formats"
>>   1a- combine together two sets of scenarios to get the cross
>> product: "all (repository, branch) pairs"
>>   1b- filter the scenarios by a condition like "all repository
>> formats that support external references"
>>  2- specify that some set of tests should be run once for each
>> scenario in a named set; this might be all tests in a class or in a
>> module.
>>
>> Maybe other things need to be on that list?
>>
>
> Regarding 2, I'd say that 'named set' should include arbitrary test
> suites. That's the standard way of managing a set of tests. Making the
> adaptation routines rely on the bzrlib TestLoader just makes life more
> difficult for those of us who are compelled to use other TestLoaders.

That's a good point, ideally this would be orthogonal to TestLoaders too.

At the moment some of our load_tests() routines entangle multiplying
test with pointing to tests is submodules.  I think we can avoid that
by just changing to having each submodule be loaded, and then giving
each on a load_tests() saying how to run it.  This would require
slightly duplicating load_tests, but if these changes can reduce it to
one statement that is not too much of a burden, and maybe makes it
more clear.  Because load_tests is called with a list of tests already
found in the module by the loader it should have no need to run the
loader itself...

>> I believe that people will not normally need to customize _how_ the
>> tests are adapted: just putting a dictionary onto the test object lets
>> it do all necessary adaption during setup or construction.
>
> Also, bzrlib will often have base test case classes that will do the
> necessary legwork.
>
>> Now it seems to me that the first one basically needs to generate a
>> sequence of dicts and a function is enough for that. As Robert says,
>> you can compose them.  On the other hand classes can be more
>> extensible.  But I dislike that TestScenarioAdapter mixes together the
>> job of generating this list and of multiplying it out across tests.
>
> I'm not sure I follow. AIUI, you can set the list of scenarios by
> setting the 'scenarios' attribute directly. This separates generating
> the list from the multiplying.

(s/Adapter/Applier/ above)

That's true for the base class, but some Applier classes like
BranchTestProviderAdapter set this attribute on themselves, thereby
conflating the two.  So these subclasses have a somewhat different
api.

> One thing that confuses me is that TestScenarioApplier has methods for
> adapting single test case instances, but not test suites. To do that,
> one must use the adapt_tests function. To have such a distinction
> between singular and plural seems unnecessary.

No, it has TestScenarioApplier.adapt() which takes a suit.  That name
could be more explicit.

-- 
Martin <http://launchpad.net/~mbp/>