launchpad-reviewers team mailing list archive

[Brad Crittenden <bac@xxxxxxxxxxxxx>]

Brad Crittenden has proposed merging lp:~bac/launchpad/bug-1007111 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1007111 in Launchpad itself: "Tests of same id but run in multiple layers fail"
  https://bugs.launchpad.net/launchpad/+bug/1007111

For more details, see:
https://code.launchpad.net/~bac/launchpad/bug-1007111/+merge/108359

= 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
-- 
https://code.launchpad.net/~bac/launchpad/bug-1007111/+merge/108359
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~bac/launchpad/bug-1007111 into lp:launchpad.

=== 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:11:34 +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:11:34 +0000
@@ -39,7 +39,8 @@
             fd.write(line + NEWLINE)
         fd.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'
@@ -90,11 +100,7 @@
         # 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))
+        suite = self.make_repeated_suite(testnames)
         with tempfile.NamedTemporaryFile() as fd:
             self.writeFile(fd, testnames)
             do_filter = filter_tests(fd.name)
@@ -105,6 +111,26 @@
         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 fd:
+            self.writeFile(fd, testnames)
+            do_filter = filter_tests(fd.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']