fix issue 594: xdist is not executing tests parametrized with random values

Now xdist properly reports the collection errors instead of silently failing to execute
the test suite.

testing/acceptance_test.py

@@ -485,6 +485,28 @@ def test_fixture_scope_caching_issue503(testdir):
     ])
 
 
+def test_issue_594_random_parametrize(testdir):
+    """
+    Make sure that tests that are randomly parametrized display an appropriate
+    error message, instead of silently skipping the entire test run.
+    """
+    p1 = testdir.makepyfile("""
+        import pytest
+        import random
+
+        xs = list(range(10))
+        random.shuffle(xs)
+        @pytest.mark.parametrize('x', xs)
+        def test_foo(x):
+            assert 1
+    """)
+    result = testdir.runpytest(p1, '-v', '-n4')
+    assert result.ret == 1
+    result.stdout.fnmatch_lines([
+        "Different tests were collected between gw* and gw*",
+    ])
+
+
 class TestNodeFailure:
 
     def test_load_single(self, testdir):

testing/test_dsession.py

@@ -144,24 +144,36 @@ class TestLoadScheduling:
         crashitem = sched.remove_node(node)
         assert crashitem == collection[0]
 
-    def test_schedule_different_tests_collected(self):
+    def test_different_tests_collected(self, testdir):
         """
-        Test that LoadScheduling is logging different tests were
+        Test that LoadScheduling is reporting collection errors when
-        collected by slaves when that happens.
+        different test ids are collected by slaves.
         """
+        class CollectHook(object):
+            """
+            Dummy hook that stores collection reports.
+            """
+
+            def __init__(self):
+                self.reports = []
+
+            def pytest_collectreport(self, report):
+                self.reports.append(report)
+
+        collect_hook = CollectHook()
+        config = testdir.parseconfig()
+        config.pluginmanager.register(collect_hook, "collect_hook")
         node1 = MockNode()
         node2 = MockNode()
-        sched = LoadScheduling(2)
+        sched = LoadScheduling(2, config=config)
-        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 len(collect_hook.reports) == 1
-        assert 'Different tests were collected between' in logged_content
+        rep = collect_hook.reports[0]
-        assert 'Different tests collected, aborting run' in logged_content
+        assert 'Different tests were collected between' in rep.longrepr
 
 
 class TestDistReporter:

xdist/dsession.py

@@ -1,4 +1,5 @@
 import difflib
+from _pytest.runner import CollectReport
 
 import pytest
 import py
@@ -88,8 +89,9 @@ class EachScheduling:
         elif self._removed2pending:
             for deadnode in self._removed2pending:
                 if deadnode.gateway.spec == node.gateway.spec:
-                    if collection != self.node2collection[deadnode]:
+                    dead_collection = self.node2collection[deadnode]
-                        msg = report_collection_diff(self.collection,
+                    if collection != dead_collection:
+                        msg = report_collection_diff(dead_collection,
                                                      collection,
                                                      deadnode.gateway.id,
                                                      node.gateway.id)
@@ -175,9 +177,10 @@ class LoadScheduling:
 
     :log: A py.log.Producer instance.
 
+    :config: Config object, used for handling hooks.
     """
 
-    def __init__(self, numnodes, log=None):
+    def __init__(self, numnodes, log=None, config=None):
         self.numnodes = numnodes
         self.node2collection = {}
         self.node2pending = {}
@@ -187,6 +190,7 @@ class LoadScheduling:
             self.log = py.log.Producer("loadsched")
         else:
             self.log = log.loadsched
+        self.config = config
 
     @property
     def nodes(self):
@@ -376,8 +380,9 @@ class LoadScheduling:
     def _check_nodes_have_same_collection(self):
         """Return True if all nodes have collected the same items.
 
-        If collections differ this returns False and logs the
+        If collections differ, this method returns False while logging
-        collection differences as they are found.
+        the collection differences and posting collection errors to
+        pytest_collectreport hook.
         """
         node_collection_items = list(self.node2collection.items())
         first_node, col = node_collection_items[0]
@@ -390,8 +395,12 @@ class LoadScheduling:
                 node.gateway.id,
             )
             if msg:
-                self.log(msg)
                 same_collection = False
+                self.log(msg)
+                if self.config is not None:
+                    rep = CollectReport(node.gateway.id, 'failed', longrepr=msg,
+                                        result=[])
+                    self.config.hook.pytest_collectreport(report=rep)
 
         return same_collection
 
@@ -494,7 +503,8 @@ class DSession:
         numnodes = len(self.nodemanager.specs)
         dist = self.config.getvalue("dist")
         if dist == "load":
-            self.sched = LoadScheduling(numnodes, log=self.log)
+            self.sched = LoadScheduling(numnodes, log=self.log,
+                                        config=self.config)
         elif dist == "each":
             self.sched = EachScheduling(numnodes, log=self.log)
         else: