launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

The proposal to merge lp:~jtv/launchpad/importqueue-test-cleanup into lp:launchpad/devel has been updated.

Description changed to:

= Translations Import Queue Test Cleanup =

This cleans up something I just found in a Translations test.

The test uses a popular and valid technique for building multiple variant of very similar test scenarios: it creates a base class containing the re-usable tests, and derives from that multiple test cases that define the different circumstances under which the same tests from the base class are to be run.

However it does this in an awkward way.  The base class is derived from TestCaseWithFactory, so the test runner treats it as a test case and tries to run it.  This is undesirable, and probably won't even work because the base class lacks some of the specifics needed to set up and run a useful test.

To work around that, the test defines a test_suite function (which we have otherwise more or less abolished) to define exactly which tests are to be run.  As far as I can see, every test case that is added to the test will have to be added to test_suite in order for it to become part of the test suite.

There is a better pattern to deal with this particular problem.  Because of Python's dynamic nature, the base class does not need to be derived from TestCase (or TestCaseWithFactory).  In this branch I remove that derivation, treating the base test class as a mix-in instead and giving its children a TestCaseWithFactory as a second base class.

I verified manually (well, using diff) that this runs the exact same tests as before.  Only the order is different.  Running "wc -l" on the test output confirms this.

To test, try this before and after:
{{{
./bin/test -vv lp.translations.tests.test_translationimportqueue
}}}

The output shows the same tests passing, though not necessarily in the same order.

Jeroen


-- 
https://code.launchpad.net/~jtv/launchpad/importqueue-test-cleanup/+merge/39931
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/importqueue-test-cleanup into lp:launchpad/devel.