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

Revision history for this message
Vincent Ladeuil (vila) wrote :

> This indeed looks a lot saner than what bzr-svn was doing previously, and is
> hopefully also less prone to breakage.
>
> 23 +# Inject our config stuff to be tested against the bzr test suite
> 24 +_mod_bzr_config.test_store_builder_registry.register_lazy(
> 25 + 'subversion', "bzrlib.plugins.svn.tests",
> "build_SubversionStore_for_test")
> 26 +_mod_bzr_config.test_stack_builder_registry.register_lazy(
> 27 + 'subversion', "bzrlib.plugins.svn.tests",
> "build_SvnBranchStack_for_test")
>
> This seems a bit unnecessary to me. I'd much rather just define a
> SvnBranchStackTests class in bzrlib.plugins.svn.tests.test_config that derives
> from some base StackTests test class in bzrlib.tests.test_config.

I tried that. This allowed me to fix the bzrlib tests a bit (I'll submit
that later, some tweaks and 2 bogus tests there).

So the try was worth it but I'm not fully convinced by the result.

Unfortunately the intermediate diff is not crystal clear as I removed some
duplication from my first submission.

In summary, the 2 registrations are replaced by 10 test classes each with
their own setUp() to replace the parametrization. As mentioned on IRC, this
open the door to divergence if more classes are added.

This means the tests are now part of -s bp.svn but all the debug still
happen in the base class, not super sexy to use.

« Back to merge proposal