From fe06f27b8bdc1e3d38d0003da5673b603b316aad Mon Sep 17 00:00:00 2001 From: Jan Kasiak Date: Sun, 30 Sep 2018 13:59:35 -0400 Subject: [PATCH] Issue node shutdown on empty workload Remote pytest_runtestloop requires at least 2 tests, or a test and a shutdown command. In case of inter test locking, for example using a file lock, the tests could deadlock, since the shutdown command would not be enqueued, and the last test of a worker would never finish, and would not allow another worker to make progress. --- changelog/351.bugfix | 1 + testing/acceptance_test.py | 50 ++++++++++++++++++++++++++++++++++++ tox.ini | 1 + xdist/scheduler/each.py | 1 + xdist/scheduler/load.py | 3 +++ xdist/scheduler/loadscope.py | 1 + 6 files changed, 57 insertions(+) create mode 100644 changelog/351.bugfix diff --git a/changelog/351.bugfix b/changelog/351.bugfix new file mode 100644 index 0000000..b887d1b --- /dev/null +++ b/changelog/351.bugfix @@ -0,0 +1 @@ +Fix scheduling deadlock in case of inter-test locking. diff --git a/testing/acceptance_test.py b/testing/acceptance_test.py index 987dbe6..93d8dd6 100644 --- a/testing/acceptance_test.py +++ b/testing/acceptance_test.py @@ -1132,6 +1132,56 @@ class TestFileScope: assert c1 == c2 +class TestLocking: + _test_content = """ + class TestClassName%s(object): + + @classmethod + def setup_class(cls): + FILE_LOCK.acquire() + + @classmethod + def teardown_class(cls): + FILE_LOCK.release() + + def test_a(self): + pass + + def test_b(self): + pass + + def test_c(self): + pass + + """ + + test_file1 = """ + import filelock + + FILE_LOCK = filelock.FileLock("test.lock") + + """ + ( + (_test_content * 4) % ("A", "B", "C", "D") + ) + + @pytest.mark.parametrize("scope", ["each", "load", "loadscope", "loadfile", "no"]) + def test_single_file(self, testdir, scope): + testdir.makepyfile(test_a=self.test_file1) + result = testdir.runpytest("-n2", "--dist=%s" % scope, "-v") + result.assert_outcomes(passed=(12 if scope != "each" else 12 * 2)) + + @pytest.mark.parametrize("scope", ["each", "load", "loadscope", "loadfile", "no"]) + def test_multi_file(self, testdir, scope): + testdir.makepyfile( + test_a=self.test_file1, + test_b=self.test_file1, + test_c=self.test_file1, + test_d=self.test_file1, + ) + result = testdir.runpytest("-n2", "--dist=%s" % scope, "-v") + result.assert_outcomes(passed=(48 if scope != "each" else 48 * 2)) + + def parse_tests_and_workers_from_output(lines): result = [] for line in lines: diff --git a/tox.ini b/tox.ini index ffe7424..ebafc13 100644 --- a/tox.ini +++ b/tox.ini @@ -23,6 +23,7 @@ deps = pytestmaster: git+https://github.com/pytest-dev/pytest.git@master pytestfeatures: git+https://github.com/pytest-dev/pytest.git@features pexpect: pexpect + filelock platform= pexpect: linux|darwin commands= diff --git a/xdist/scheduler/each.py b/xdist/scheduler/each.py index 250421f..486d8b0 100644 --- a/xdist/scheduler/each.py +++ b/xdist/scheduler/each.py @@ -126,6 +126,7 @@ class EachScheduling(object): if not pending: pending[:] = range(len(self.node2collection[node])) node.send_runtest_all() + node.shutdown() else: node.send_runtest_some(pending) self._started.append(node) diff --git a/xdist/scheduler/load.py b/xdist/scheduler/load.py index dae43ee..4116984 100644 --- a/xdist/scheduler/load.py +++ b/xdist/scheduler/load.py @@ -178,6 +178,9 @@ class LoadScheduling(object): return num_send = items_per_node_max - len(node_pending) self._send_tests(node, num_send) + else: + node.shutdown() + self.log("num items waiting for node:", len(self.pending)) def remove_node(self, node): diff --git a/xdist/scheduler/loadscope.py b/xdist/scheduler/loadscope.py index eb3b23b..2917fc0 100644 --- a/xdist/scheduler/loadscope.py +++ b/xdist/scheduler/loadscope.py @@ -306,6 +306,7 @@ class LoadScopeScheduling(object): # Check that more work is available if not self.workqueue: + node.shutdown() return self.log("Number of units waiting for node:", len(self.workqueue))