Merge lp:~bac/launchpad/bug-1007111 into lp:launchpad

[Brad Crittenden]

Make --load-list account for duplicate tests across layers.

= Summary =

--load-list now accounts for multiple tests with the same id to
account for legacy Launchpad testing infrastructure. It was assumed
incorrectly that those repeated tests were all in the same layer.

== Proposed fix ==

Account for repeated tests appearing in multiple layers.

== Tests ==

bin/test -vv lp.services.testing.tests.test_customresult

== Demo and Q/A ==

% echo "lib/lp/bugs/doc/bugmessage.txt" > /tmp/t0
% bin/test --list-tests --load-list /tmp/t0
Listing lp.testing.layers.LaunchpadZopelessLayer tests:
  lib/lp/bugs/doc/bugmessage.txt
  lib/lp/bugs/doc/bugmessage.txt
  lib/lp/bugs/doc/bugmessage.txt
Listing lp.services.mail.tests.test_doc.ProcessMailLayer tests:
  lib/lp/bugs/doc/bugmessage.txt
Listing lp.testing.layers.LaunchpadFunctionalLayer tests:
  lib/lp/bugs/doc/bugmessage.txt

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/services/testing/customresult.py
  lib/lp/services/testing/tests/test_customresult.py

[Gary Poster (gary)]

Great, thanks!

We talked on IRC about benji's concern with naming Python file objects after file descriptors, and so possibly changing "fd" to "f" or something else. As I said, your call.

Also as we discussed even earlier, I'd like to see bug 682772 addressed as well (and maybe engage with Robert on ideas he has for a good hook point for bug 682771 if that's not clear) but we have that on our board, and this is a good change.

review: Approve

=== modified file 'lib/lp/services/testing/customresult.py'
--- lib/lp/services/testing/customresult.py	2012-05-28 16:17:01 +0000
+++ lib/lp/services/testing/customresult.py	2012-06-01 15:29:18 +0000
@@ -57,16 +57,18 @@
         # must be tracked separately.
         for layer_name, suite in tests_by_layer_name.iteritems():
             for testcase in suite:
-                layer, testlist = test_lookup.setdefault(
-                    testcase.id(), (layer_name, []))
-                testlist.append(testcase)
+                layer_to_tests = test_lookup.setdefault(
+                    testcase.id(), {})
+                testcases = layer_to_tests.setdefault(
+                    layer_name, [])
+                testcases.append(testcase)
 
         result = {}
         for testname in tests:
-            layer, testcases = test_lookup.get(testname, (None, None))
-            if testcases is not None:
-                suite = result.setdefault(layer, TestSuite())
-                for testcase in testcases:
-                    suite.addTest(testcase)
+            layer_to_tests = test_lookup.get(testname, {})
+            for layer_name, testcases in layer_to_tests.items():
+                if testcases is not None:
+                    suite = result.setdefault(layer_name, TestSuite())
+                    suite.addTests(testcases)
         return result
     return do_filter

=== modified file 'lib/lp/services/testing/tests/test_customresult.py'
--- lib/lp/services/testing/tests/test_customresult.py	2012-05-30 11:41:02 +0000
+++ lib/lp/services/testing/tests/test_customresult.py	2012-06-01 15:29:18 +0000
@@ -34,12 +34,13 @@
 
     layer = BaseLayer
 
-    def writeFile(self, fd, contents):
+    def writeFile(self, f, contents):
         for line in contents:
-            fd.write(line + NEWLINE)
-        fd.flush()
+            f.write(line + NEWLINE)
+        f.flush()
 
-    def make_suites(self):
+    @staticmethod
+    def make_suites():
         """Make two suites.
 
         The first has 'a'..'m' and the second 'n'..'z'.
@@ -54,6 +55,15 @@
             suite_nz.addTest(FakeTestCase(letter))
         return suite_am, suite_nz
 
+    @staticmethod
+    def make_repeated_suite(testnames):
+        suite = unittest.TestSuite()
+        for t in testnames:
+            # Each test will be repeated equal to the number represented.
+            for i in range(int(t)):
+                suite.addTest(FakeTestCase(t))
+        return suite
+
     def test_ordering(self):
         # Tests should be returned in the order seen in the testfile.
         layername = 'layer-1'
@@ -61,9 +71,9 @@
         suite = unittest.TestSuite()
         for letter in string.lowercase:
             suite.addTest(FakeTestCase(letter))
-        with tempfile.NamedTemporaryFile() as fd:
-            self.writeFile(fd, testnames)
-            do_filter = filter_tests(fd.name)
+        with tempfile.NamedTemporaryFile() as f:
+            self.writeFile(f, testnames)
+            do_filter = filter_tests(f.name)
             results = do_filter({layername: suite})
         self.assertEqual(1, len(results))
         self.assertIn(layername, results)
@@ -74,9 +84,9 @@
         # Tests must be kept in their layer.
         suite1, suite2 = self.make_suites()
         testnames = ['a', 'b', 'c', 'z', 'y', 'x']
-        with tempfile.NamedTemporaryFile() as fd:
-            self.writeFile(fd, testnames)
-            do_filter = filter_tests(fd.name)
+        with tempfile.NamedTemporaryFile() as f:
+            self.writeFile(f, testnames)
+            do_filter = filter_tests(f.name)
             results = do_filter({'layer1': suite1,
                                  'layer2': suite2})
         self.assertEqual(2, len(results))
@@ -90,14 +100,10 @@
         # collapsed and lost.
         layername = 'layer-1'
         testnames = ['1', '2', '3']
-        suite = unittest.TestSuite()
-        for t in testnames:
-            # Each test will be repeated equal to the number represented.
-            for i in range(int(t)):
-                suite.addTest(FakeTestCase(t))
-        with tempfile.NamedTemporaryFile() as fd:
-            self.writeFile(fd, testnames)
-            do_filter = filter_tests(fd.name)
+        suite = self.make_repeated_suite(testnames)
+        with tempfile.NamedTemporaryFile() as f:
+            self.writeFile(f, testnames)
+            do_filter = filter_tests(f.name)
             results = do_filter({layername: suite})
         self.assertEqual(1, len(results))
         self.assertIn(layername, results)
@@ -105,13 +111,33 @@
         expected = ['1', '2', '2', '3', '3', '3']
         self.assertEqual(expected, [t.id() for t in suite])
 
+    def test_repeated_names_different_layers(self):
+        # Some doctests are run repeatedly with different scenarios, including
+        # being included in different layers.
+        testnames = ['a', 'b', 'c']
+        suite = self.make_suites()[0]
+
+        with tempfile.NamedTemporaryFile() as f:
+            self.writeFile(f, testnames)
+            do_filter = filter_tests(f.name)
+            results = do_filter({'layer1': suite,
+                                 'layer2': suite,
+                                 'layer3': suite})
+
+        self.assertEqual(3, len(results))
+        self.assertEqual(
+            ['layer1', 'layer2', 'layer3'], sorted(results.keys()))
+        self.assertEqual(['a', 'b', 'c'], [t.id() for t in results['layer1']])
+        self.assertEqual(['a', 'b', 'c'], [t.id() for t in results['layer2']])
+        self.assertEqual(['a', 'b', 'c'], [t.id() for t in results['layer3']])
+
     def test_no_layer(self):
         # If tests have no layer (None) work.
         testnames = ['a', 'b', 'y', 'z']
         suite1, suite2 = self.make_suites()
-        with tempfile.NamedTemporaryFile() as fd:
-            self.writeFile(fd, testnames)
-            do_filter = filter_tests(fd.name)
+        with tempfile.NamedTemporaryFile() as f:
+            self.writeFile(f, testnames)
+            do_filter = filter_tests(f.name)
             results = do_filter({'layer1': suite1,
                                  None: suite2})
         self.assertEqual(2, len(results))