Merged in nicoddemus/pytest-xdist/log-collection-diff (pull request #9)

Log different tests collected by slaves instead of an error

testing/acceptance_test.py

@@ -310,9 +310,9 @@ class TestTerminalReporting:
         """)
         result = testdir.runpytest("-n1", "-v")
         result.stdout.fnmatch_lines_random([
-            "*PASS*test_pass_skip_fail.py?2*test_ok*",
+            "*PASS*test_pass_skip_fail.py*test_ok*",
-            "*SKIP*test_pass_skip_fail.py?4*test_skip*",
+            "*SKIP*test_pass_skip_fail.py*test_skip*",
-            "*FAIL*test_pass_skip_fail.py?6*test_func*",
+            "*FAIL*test_pass_skip_fail.py*test_func*",
         ])
         result.stdout.fnmatch_lines([
             "*def test_func():",
@@ -327,7 +327,7 @@ class TestTerminalReporting:
         """)
         result = testdir.runpytest("-n1", "-v")
         result.stdout.fnmatch_lines([
-            "*FAIL*test_fail_platinfo.py*1*test_func*",
+            "*FAIL*test_fail_platinfo.py*test_func*",
             "*0*Python*",
             "*def test_func():",
             ">       assert 0",

testing/test_dsession.py

@@ -146,6 +146,25 @@ class TestLoadScheduling:
         crashitem = sched.remove_node(node)
         assert crashitem == collection[0]
 
+    def test_schedule_different_tests_collected(self):
+        """
+        Test that LoadScheduling is logging different tests were
+        collected by slaves when that happens.
+        """
+        node1 = MockNode()
+        node2 = MockNode()
+        sched = LoadScheduling(2)
+        logged_messages = []
+        py.log.setconsumer('loadsched', logged_messages.append)
+        sched.addnode(node1)
+        sched.addnode(node2)
+        sched.addnode_collection(node1, ["a.py::test_1"])
+        sched.addnode_collection(node2, ["a.py::test_2"])
+        sched.init_distribute()
+        logged_content = ''.join(x.content() for x in logged_messages)
+        assert 'Different tests were collected between' in logged_content
+        assert 'Different tests collected, aborting run' in logged_content
+
 
 class TestDistReporter:
 
@@ -181,7 +200,7 @@ class TestDistReporter:
 def test_report_collection_diff_equal():
     """Test reporting of equal collections."""
     from_collection = to_collection = ['aaa', 'bbb', 'ccc']
-    assert report_collection_diff(from_collection, to_collection, 1, 2)
+    assert report_collection_diff(from_collection, to_collection, 1, 2) is None
 
 
 def test_report_collection_diff_different():
@@ -204,10 +223,8 @@ def test_report_collection_diff_different():
         '-YYY'
     )
 
-    try:
+    msg = report_collection_diff(from_collection, to_collection, 1, 2)
-        report_collection_diff(from_collection, to_collection, 1, 2)
+    assert msg == error_message
-    except AssertionError as e:
-        assert py.builtin._totext(e) == error_message
 
 @pytest.mark.xfail(reason="duplicate test ids not supported yet")
 def test_pytest_issue419(testdir):

xdist/dsession.py

@@ -17,7 +17,7 @@ class EachScheduling:
         if log is None:
             self.log = py.log.Producer("eachsched")
         else:
-            self.log = log.loadsched
+            self.log = log.eachsched
         self.collection_is_completed = False
 
     def hasnodes(self):
@@ -139,22 +139,17 @@ class LoadScheduling:
     def init_distribute(self):
         assert self.collection_is_completed
         # XXX allow nodes to have different collections
-        node_collection_items = list(self.node2collection.items())
+        if not self._check_nodes_have_same_collection():
-        first_node, col = node_collection_items[0]
+            self.log('**Different tests collected, aborting run**')
-        for node, collection in node_collection_items[1:]:
+            return
-            report_collection_diff(
-                col,
-                collection,
-                first_node.gateway.id,
-                node.gateway.id,
-            )
 
         # all collections are the same, good.
         # we now create an index
-        self.collection = col
+        self.collection = list(self.node2collection.values())[0]
-        self.pending[:] = range(len(col))
+        self.pending[:] = range(len(self.collection))
-        if not col:
+        if not self.collection:
             return
+
         # how many items per node do we have about?
         items_per_node = len(self.collection) // len(self.node2pending)
         # take a fraction of tests for initial distribution
@@ -172,17 +167,36 @@ class LoadScheduling:
             self.node2pending[node].extend(tests_per_node)
             node.send_runtest_some(tests_per_node)
 
+    def _check_nodes_have_same_collection(self):
+        """
+        Return True if all nodes have collected the same items, False otherwise.
+        This method also logs the collection differences as they are found.
+        """
+        node_collection_items = list(self.node2collection.items())
+        first_node, col = node_collection_items[0]
+        same_collection = True
+        for node, collection in node_collection_items[1:]:
+            msg = report_collection_diff(
+                col,
+                collection,
+                first_node.gateway.id,
+                node.gateway.id,
+            )
+            if msg:
+                self.log(msg)
+                same_collection = False
+
+        return same_collection
+
+
 def report_collection_diff(from_collection, to_collection, from_id, to_id):
     """Report the collected test difference between two nodes.
 
-    :returns: True if collections are equal.
+    :returns: detailed message describing the difference between the given
-
+    collections, or None if they are equal.
-    :raises: AssertionError with a detailed error message describing the
-             difference between the collections.
-
     """
     if from_collection == to_collection:
-        return True
+        return None
 
     diff = difflib.unified_diff(
         from_collection,
@@ -196,7 +210,7 @@ def report_collection_diff(from_collection, to_collection, from_id, to_id):
         '{diff}'
     ).format(from_id=from_id, to_id=to_id, diff='\n'.join(diff))
     msg = "\n".join([x.rstrip() for x in error_message.split("\n")])
-    raise AssertionError(msg)
+    return msg
 
 
 class Interrupted(KeyboardInterrupt):