[MERGE] Make the test suite use the new test loader

[Ian Clatworthy]

Vincent Ladeuil wrote:

> This patch makes the test suite use the new test loader, mostly
> by defining load_tests() in modules that were using test_suite().
> 
> This allowed further simplifications of bzrlib.tests.test_suite()
> function itself.

I honestly don't have strong opinions about the right/best way to design
and implement test loaders so I'm not going to vote on this patch. I can
provide some initial feedback on it though in case that helps ...


> -      function. (a forthcoming patch will provide many examples on how to
> -      implement this).
> +      function. All tests modules have been updated and provides many examples
> +      (grep for load_tests).


You need a separate item in NEWS now given that the first set of changes
made it into 1.4 and these ones won't.

> +def load_tests(basic_tests, module, loader):
> +    suite = loader.suiteClass()
> +    # add the tests for this module (obviously none so far)
> +    suite.addTests(basic_tests)

The "obviously none so far" comment looks like a hangover from a template?

> === modified file 'bzrlib/tests/commands/__init__.py'
> --- bzrlib/tests/commands/__init__.py	2007-10-04 15:45:21 +0000
> +++ bzrlib/tests/commands/__init__.py	2008-03-30 21:57:16 +0000
> @@ -24,10 +24,11 @@
>  # FIXME: If the separation described above from the blackbox tests is not worth
>  # it, all the tests defined below should be moved to blackbox instead. 
>  
> -from bzrlib.tests import TestLoader
> -
> -
> -def test_suite():
> +def load_tests(basic_tests, module, loader):
> +    suite = loader.suiteClass()
> +    # add the tests for this module
> +    suite.addTests(basic_tests)
> +
>      testmod_names = [
>          'bzrlib.tests.commands.test_branch',
>          'bzrlib.tests.commands.test_cat',
> @@ -41,7 +42,6 @@
>          'bzrlib.tests.commands.test_push',
>          'bzrlib.tests.commands.test_update',
>          ]
> -    loader = TestLoader()
>      suite = loader.loadTestsFromModuleNames(testmod_names)

This looks wrong. Shouldn't you be calling suite.addTests(loader...) on
the last line now?

> @@ -38,6 +39,7 @@
>                             FileExists,
>                             InvalidURL,
>                             LockError,
> +                           NoSmartServer,

This import of NoSmartServer looks redundant.

Most of the rest looks OK at first glance (but I must confess I haven't
dug too deeply as I'm not planning to officially review it).

Ian C.