launchpad-reviewers team mailing list archive

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

Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/importqueue-test-cleanup into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers): code


= 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 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.

=== modified file 'lib/lp/translations/tests/test_translationimportqueue.py'
--- lib/lp/translations/tests/test_translationimportqueue.py	2010-10-29 05:58:51 +0000
+++ lib/lp/translations/tests/test_translationimportqueue.py	2010-11-03 06:27:51 +0000
@@ -3,8 +3,6 @@
 
 __metaclass__ = type
 
-import unittest
-
 import transaction
 from zope.component import getUtility
 
@@ -25,7 +23,7 @@
     )
 
 
-class TestCanSetStatusBase(TestCaseWithFactory):
+class TestCanSetStatusBase:
     """Base for tests that check that canSetStatus works ."""
 
     layer = LaunchpadZopelessLayer
@@ -166,7 +164,7 @@
             [False, False, False, False, False, False, False])
 
 
-class TestCanSetStatusPOTemplate(TestCanSetStatusBase):
+class TestCanSetStatusPOTemplate(TestCanSetStatusBase, TestCaseWithFactory):
     """Test canStatus applied to an entry with a POTemplate."""
 
     def setUp(self):
@@ -180,7 +178,7 @@
             productseries=self.productseries, potemplate=self.potemplate)
 
 
-class TestCanSetStatusPOFile(TestCanSetStatusBase):
+class TestCanSetStatusPOFile(TestCanSetStatusBase, TestCaseWithFactory):
     """Test canStatus applied to an entry with a POFile."""
 
     def setUp(self):
@@ -332,16 +330,3 @@
                 productseries=self.product.series[0])
             self.product.owner = self.new_owner
         self.assertEqual(self.old_owner, old_entry.importer)
-
-
-def test_suite():
-    """Add only specific test cases and leave out the base case."""
-    suite = unittest.TestSuite()
-    suite.addTest(unittest.makeSuite(TestCanSetStatusPOTemplate))
-    suite.addTest(unittest.makeSuite(TestCanSetStatusPOFile))
-    suite.addTest(
-        unittest.makeSuite(TestCanSetStatusPOTemplateWithQueuedUser))
-    suite.addTest(unittest.makeSuite(TestCanSetStatusPOFileWithQueuedUser))
-    suite.addTest(unittest.makeSuite(TestGetGuessedPOFile))
-    suite.addTest(unittest.makeSuite(TestProductOwnerEntryImporter))
-    return suite