Code review comment for lp:~bzr/bzr-svn/stacks

> Sure, I don't disagree with that. I think the parametrization isn't done
  right here.

Not telling me in what way it's not done right, doesn't help me much ;)

One issue I can think of is about how these tests are named, because it has
an impact on how -s will work with them:

- if they are named as bt.test_config, -s bt.test_config includes them
  (while respecting BZR_PLUGIN_PATH and friends)

- if they are named as bp.svn, -s bp.svn includes them but -s bt.test_config
  doesn't anymore

In fact, they cannot be part of -s bt.test_config without some help from
svn/__init__.py otherwise even the load_tests() there is ignored so no
tests/ file has a way to add them.

There may be a trick to duplicate the tests under both hierarchies but same
care should be taken if both are selected.

The parametrization is about deciding which tests should be run with which
scenario.

The store scenario is:

  ('subversion', {'get_store': store_builder, 'store_id': 'subversion'})

The stack scenario is:

  ('subversion', {'get_store': store_builder, 'get_stack': stack_builder})

The matcher scenario (not sure we really want scenarios for matchers, but if
we want):

 ('subversion', {'get_store': store_builder,
                 'store_id': 'subversion',
                 'matcher': matcher_class})

From that which test classes should be used is decided either in bzr or in
bzr-svn.

Deciding from bzr allows to define the required interface.

Deciding from bzr-svn allows to define which interface you agree to
implement.

Asking for more control about the test classes requires knowledege about
them.

I don't know where you stand on that.

You certainly have a great deal of experience with plugins needing to respect
an interface, parametrized or not.

My two proposals for that seems to be at the two extremes, do you have a
preferred sweet spot between them ?

I've started defining:

def multiply_store_tests(tests, result, store_builder):
    scenarios = [('subversion', {'get_store': store_builder,
                                 'store_id': 'subversion'})]
    return tests.multiply_tests(tests, scenarios, tests)

def multiply_matcher_tests(tests, store_builder, matcher_class):
    scenarios = [('subversion', {'get_store': store_builder,
                                 'store_id': 'subversion',
                                 'matcher': matcher_class})]
    return tests.multiply_tests(tests, scenarios, tests)

def multiply_stack_tests(stack_builder, store_builder):
    scenarios = [('subversion', {'get_store': store_builder,
                                 'get_stack': stack_builder})]
    return tests.multiply_tests(tests, scenarios, tests)

in bzr-svn but that still need to know:

- which test classes should be used,

- and derived from that, how the tests should be named.

There are infinite ways to proceed and I don't think iterating them all is
the best use of our time ;) Can we agree on something and make it evolve as
we encounter the issues and better understand the problem ?