fix pytest issue503: avoid random re-setup of broad scoped fixtures

(anything above function).

CHANGELOG

@@ -1,9 +1,14 @@
-XXX
+1.11
 -------------------------
 
-- fix pytest/xdist issue485 (also depends on py-1.4.21.dev1): 
+- fix pytest/xdist issue485 (also depends on py-1.4.22):
   attach stdout/stderr on --boxed processes that die.
 
+- fix pytest/xdist issue503: make sure that a node has usually
+  two items to execute to avoid scoped fixtures to be torn down
+  pre-maturely (fixture teardown/setup is "nextitem" sensitive).
+  Thanks to Andreas Pelme for bug analysis and failing test.
+
 1.10
 -------------------------
 

testing/acceptance_test.py

@@ -1,3 +1,4 @@
+import pytest
 import py
 import sys
 
@@ -459,3 +460,29 @@ def test_issue34_pluginloading_in_subprocess(testdir):
     result.stdout.fnmatch_lines([
         "*1 passed*",
     ])
+
+
+def test_fixture_scope_caching_issue503(testdir):
+    p1 = testdir.makepyfile("""
+            import pytest
+
+            @pytest.fixture(scope='session')
+            def fix():
+                assert fix.counter == 0, 'session fixture was invoked multiple times'
+                fix.counter += 1
+            fix.counter = 0
+
+            def test_a(fix):
+                pass
+
+            def test_b(fix):
+                pass
+    """)
+    result = testdir.runpytest(p1, '-v', '-n1')
+    assert result.ret == 0
+    result.stdout.fnmatch_lines([
+    "*2 passed*",
+    ])
+
+
+

testing/test_dsession.py

@@ -98,15 +98,15 @@ class TestLoadScheduling:
         assert sched.node2collection[node1] == collection
         assert sched.node2collection[node2] == collection
         sched.init_distribute()
-        assert sched.tests_finished()
-        assert len(node1.sent) == 1
-        assert len(node2.sent) == 1
-        x = sorted(node1.sent + node2.sent)
-        assert x == [0, 1]
-        sched.remove_item(node1, node1.sent[0])
-        sched.remove_item(node2, node2.sent[0])
-        assert sched.tests_finished()
         assert not sched.pending
+        assert not sched.tests_finished()
+        assert len(node1.sent) == 2
+        assert len(node2.sent) == 0
+        assert node1.sent == [0, 1]
+        sched.remove_item(node1, node1.sent[0])
+        assert sched.tests_finished()
+        sched.remove_item(node1, node1.sent[1])
+        assert sched.tests_finished()
 
     def test_init_distribute_chunksize(self):
         sched = LoadScheduling(2)
@@ -114,22 +114,25 @@ class TestLoadScheduling:
         node2 = MockNode()
         sched.addnode(node1)
         sched.addnode(node2)
-        col = ["xyz"] * (3)
+        col = ["xyz"] * (6)
         sched.addnode_collection(node1, col)
         sched.addnode_collection(node2, col)
         sched.init_distribute()
         #assert not sched.tests_finished()
         sent1 = node1.sent
         sent2 = node2.sent
-        chunkitems = col[:1]
+        assert sent1 == [0, 1]
-        assert (sent1 == [0] and sent2 == [1]) or (
+        assert sent2 == [2, 3]
-                sent1 == [1] and sent2 == [0])
+        assert sched.pending == [4, 5]
         assert sched.node2pending[node1] == sent1
         assert sched.node2pending[node2] == sent2
-        assert len(sched.pending) == 1
+        assert len(sched.pending) == 2
-        for node in (node1, node2):
+        sched.remove_item(node1, 0)
-            for i in sched.node2pending[node]:
+        assert node1.sent == [0, 1, 4]
-                sched.remove_item(node, i)
+        assert sched.pending == [5]
+        assert node2.sent == [2, 3]
+        sched.remove_item(node1, 1)
+        assert node1.sent == [0, 1, 4, 5]
         assert not sched.pending
 
     def test_add_remove_node(self):

xdist/dsession.py

@@ -49,7 +49,7 @@ class EachScheduling:
         if not pending:
             return
         crashitem = self.node2collection[node][pending.pop(0)]
-        # XXX what about the rest of pending?
+        # XXX do or report something wrt the remaining per-node pending items?
         return crashitem
 
     def init_distribute(self):
@@ -58,11 +58,13 @@ class EachScheduling:
             node.send_runtest_all()
             pending[:] = range(len(self.node2collection[node]))
 
+
 class LoadScheduling:
     def __init__(self, numnodes, log=None):
         self.numnodes = numnodes
         self.node2pending = {}
         self.node2collection = {}
+        self.nodes = []
         self.pending = []
         if log is None:
             self.log = py.log.Producer("loadsched")
@@ -75,13 +77,14 @@ class LoadScheduling:
 
     def addnode(self, node):
         self.node2pending[node] = []
+        self.nodes.append(node)
 
     def tests_finished(self):
-        if not self.collection_is_completed or self.pending:
+        if not self.collection_is_completed:
+            return False
+        for pending in self.node2pending.values():
+            if len(pending) >= 2:
                 return False
-        #for items in self.node2pending.values():
-        #    if items:
-        #        return False
         return True
 
     def addnode_collection(self, node, collection):
@@ -92,37 +95,46 @@ class LoadScheduling:
             self.collection_is_completed = True
 
     def remove_item(self, node, item_index, duration=0):
-        node_pending = self.node2pending[node]
+        self.node2pending[node].remove(item_index)
-        node_pending.remove(item_index)
+        self.check_schedule(node, duration=duration)
-        # pre-load items-to-test if the node may become ready
 
+    def check_schedule(self, node, duration=0):
         if self.pending:
-            if duration >= 0.1 and node_pending:
+            # how many nodes do we have?
-                # seems the node is doing long-running tests
-                # so let's rather wait with sending new items
-                return
-            # how many nodes do we have remaining per node roughly?
             num_nodes = len(self.node2pending)
             # if our node goes below a heuristic minimum, fill it out to
             # heuristic maximum
             items_per_node_min = max(
-                    1, len(self.pending) // num_nodes // 4)
+                    2, len(self.pending) // num_nodes // 4)
             items_per_node_max = max(
-                    1, len(self.pending) // num_nodes // 2)
+                    2, len(self.pending) // num_nodes // 2)
-            if len(node_pending) <= items_per_node_min:
+            node_pending = self.node2pending[node]
-                num_send = items_per_node_max - len(node_pending) + 1
+            if len(node_pending) < items_per_node_min:
+                if duration >= 0.1 and len(node_pending) >= 2:
+                    # seems the node is doing long-running tests
+                    # and has enough items to continue
+                    # so let's rather wait with sending new items
+                    return
+                num_send = items_per_node_max - len(node_pending)
                 self._send_tests(node, num_send)
 
         self.log("num items waiting for node:", len(self.pending))
         #self.log("node2pending:", self.node2pending)
 
     def remove_node(self, node):
+        self.nodes.remove(node)
         pending = self.node2pending.pop(node)
         if not pending:
             return
-        # the node must have crashed on the item if there are pending ones
+        # the node has crashed on the item if there are pending ones
+        # and we are told to remove the node
         crashitem = self.collection[pending.pop(0)]
+
+        # put the remaining items back to the general pending list
         self.pending.extend(pending)
+        # see if some nodes can pick the remaining tests up already
+        for node in self.node2pending:
+            self.check_schedule(node)
         return crashitem
 
     def init_distribute(self):
@@ -147,9 +159,9 @@ class LoadScheduling:
         # 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
-        node_chunksize = max(items_per_node // 4, 1)
+        node_chunksize = max(items_per_node // 4, 2)
         # and initialize each node with a chunk of tests
-        for node in self.node2pending:
+        for node in self.nodes:
             self._send_tests(node, node_chunksize)
 
     #f = open("/tmp/sent", "w")

xdist/remote.py

@@ -51,9 +51,12 @@ class SlaveInteractor:
             elif name == "runtests_all":
                 torun.extend(range(len(session.items)))
             self.log("items to run:", torun)
-            while torun:
+            # only run if we have an item and a next item
+            while len(torun) >= 2:
                 self.run_tests(torun)
             if name == "shutdown":
+                if torun:
+                    self.run_tests(torun)
                 break
         return True