[MERGE] Slight test suite refactorings

Vincent Ladeuil v.ladeuil+lp at free.fr
Tue Apr 1 08:09:09 BST 2008


>>>>> "martin" == Martin Pool <mbp at sourcefrog.net> writes:

    martin> On Tue, Apr 1, 2008 at 4:25 PM, Vincent Ladeuil <v.ladeuil+lp at free.fr> wrote:
    >> These are needed for the upcoming FilteredByModuleTestLoader (see
    >> loom https://code.launchpad.net/~v-ladeuil/bzr/faster-selftest
    >> for full context).

    martin> Hm, no bb response yet.

    martin> +        self.assertEqual(set(_test_ids(self.suite)),
    martin> +                         set(_test_ids(randomized_suite)))

    martin> In passing, I would probably write this as comparing sorted(x),
    martin> sorted(y).  I realize it's not your original code.

We are comparing sets, why do you want to sort lists and compare
them ? Easier to read ?

    martin> @@ -2933,8 +2926,13 @@
    >>>> tests[1].param
    martin>      2
    martin>      """
    martin> -    loader = TestLoader()
    martin> -    suite = TestSuite()
    martin> +    # XXX: Isn't load_tests() a better way to provide the same functionality
    martin> +    # without forcing a predefined TestScenarioApplier ? --vila 080215
    martin> +    if loader is None:
    martin> +        loader = TestUtil.TestLoader()
    martin> +
    martin> +    suite = loader.suiteClass()
    martin> +

    martin> I agree with the sentiment

I didn't make the change since I think it needs a deprecation
phase (if we all agree to make the change). It's used in only two
places in our test suite, but since it was defined as 'the
recommended public interface for test parameterization' (which
was true at the time of the writing), it may have been used by
plugins.

    martin> but I don't understand this comment in this location.

It's inside multiply_tests_from_modules, but yes it may be better
placed right at the beginning of the function. I'll do that.

    martin> bb: tweak

Thanks, I'll take your first remark remark into account and merge.

        Vincent



More information about the bazaar mailing list