launchpad-reviewers team mailing list archive

[Brad Crittenden <bac@xxxxxxxxxxxxx>]

Brad Crittenden has proposed merging lp:~bac/launchpad/ordered-load-list into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #989208 in Launchpad itself: "bin/test --load-list does not preserve test order"
  https://bugs.launchpad.net/launchpad/+bug/989208

For more details, see:
https://code.launchpad.net/~bac/launchpad/ordered-load-list/+merge/104287

= Summary =

"bin/test --load-list <fn>" loads the tests but then does not respect
their ordering due to the use of set() and the way the tests are
assigned to layers.

The approach presently taken makes sense from an efficiency standpoint
but at the cost of ordering.  One of the great uses of --load-list is
to repeat a test run to find test isolation problems and that
utility is defeated if the ordering is not maintained.

== Proposed fix ==

Change 'filter_tests' to maintain order within a layer.

The order in which the layers are run may change, though that has not
been seen in practice.  Should the layer ordering change, the tests
are still run in order per layer, which is probably sufficient for
isolating test failures.

== Pre-implementation notes ==

Chat with Gary.

== Implementation details ==

As above.

== Tests ==

bin/test --load-list in.txt --list-tests > out.txt

Then verify in.txt and out.txt are the same modulo the "Listing layer"
messages and any layer rearrangement issues as described above.

== Demo and Q/A ==


= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/services/testing/customresult.py
-- 
https://code.launchpad.net/~bac/launchpad/ordered-load-list/+merge/104287
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~bac/launchpad/ordered-load-list into lp:launchpad.

=== modified file 'lib/lp/services/testing/customresult.py'
--- lib/lp/services/testing/customresult.py	2012-01-01 02:58:52 +0000
+++ lib/lp/services/testing/customresult.py	2012-05-01 19:52:20 +0000
@@ -11,7 +11,6 @@
 
 from unittest import TestSuite
 
-from testtools import iterate_tests
 from zope.testing.testrunner import find
 
 
@@ -39,14 +38,29 @@
         returns only those tests with ids in the file 'list_name'.
     """
     def do_filter(tests_by_layer_name):
-        tests = sorted(set(line.strip() for line in open(list_name, 'rb')))
+        # Read the tests, filtering out any blank lines.
+        tests = filter(None, [line.strip() for line in open(list_name, 'rb')])
+        suites_by_layer = {}
+        for layer_name, suite in tests_by_layer_name.iteritems():
+            testnames = {}
+            for t in suite:
+                testnames[t.id()] = t
+            suites_by_layer[layer_name] = testnames
+        # There are ~30 layers.
+        def find_layer(t):
+            for layer, names in suites_by_layer.items():
+                if t in names:
+                    return layer, names[t]
+            return None, None
+        ordered_layers = []
         result = {}
-        for layer_name, suite in tests_by_layer_name.iteritems():
-            new_suite = TestSuite()
-            for test in iterate_tests(suite):
-                if test.id() in tests:
-                    new_suite.addTest(test)
-            if new_suite.countTestCases():
-                result[layer_name] = new_suite
+        for testname in tests:
+            layer, test = find_layer(testname)
+            if not layer:
+                raise Exception("Test not found: %s" % testname)
+            if not layer in ordered_layers:
+                ordered_layers.append(layer)
+            suite = result.setdefault(layer, TestSuite())
+            suite.addTest(test)
         return result
     return do_filter