Skip to content

Commit 230ba6a

Browse files
authored
Properly wait for workers when test run terminates early (#963)
Currently, a reason to terminate early (e.g. test failure with --exitfail option set) causes DSession to immediately raise an Interrupt exception. Subsequent reports generated by the workers, during the shutdown phase, are discarded. One consequence is that, for the failing test, teardown and testfinish are ignored, which prevents corresponding hooks pytest_runtest_logreport and pytest_runtest_logfinish being executed for the failing test (this problem covered in #54). The reporting of tests executing in other workers is also left in an indeterminate state. This can affect other plugin code. This is a relatively simple fix, which appears to have minimal and, I think, acceptable impact on text execution behaviour. The observable differences are differences in what is reported about a test run. For example, when running, for example with the '-x/--exitfail' option, it is possible that more than a single test failure is reported. This is because more than one test did fail before the test run was completely stopped. The reporting is absolutely correct; and complete. Prior to this change, only a single failure would have been reported, but because of incomplete and arguably incorrect reporting.
1 parent 93ca202 commit 230ba6a

File tree

3 files changed

+49
-10
lines changed

3 files changed

+49
-10
lines changed

changelog/963.improvement.rst

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
Wait for workers to finish reporting when test run stops early.
2+
3+
This makes sure that the results of in-progress tests are displayed.
4+
Previously these reports were being discarded, losing information about the
5+
test run.

src/xdist/dsession.py

+14-6
Original file line numberDiff line numberDiff line change
@@ -118,11 +118,14 @@ def pytest_runtestloop(self):
118118
assert self.sched is not None
119119

120120
self.shouldstop = False
121+
pending_exception = None
121122
while not self.session_finished:
122123
self.loop_once()
123124
if self.shouldstop:
124125
self.triggershutdown()
125-
raise Interrupted(str(self.shouldstop))
126+
pending_exception = Interrupted(str(self.shouldstop))
127+
if pending_exception:
128+
raise pending_exception
126129
return True
127130

128131
def loop_once(self):
@@ -351,14 +354,19 @@ def _failed_worker_collectreport(self, node, rep):
351354
def _handlefailures(self, rep):
352355
if rep.failed:
353356
self.countfailures += 1
354-
if self.maxfail and self.countfailures >= self.maxfail:
357+
if (
358+
self.maxfail
359+
and self.countfailures >= self.maxfail
360+
and not self.shouldstop
361+
):
355362
self.shouldstop = f"stopping after {self.countfailures} failures"
356363

357364
def triggershutdown(self):
358-
self.log("triggering shutdown")
359-
self.shuttingdown = True
360-
for node in self.sched.nodes:
361-
node.shutdown()
365+
if not self.shuttingdown:
366+
self.log("triggering shutdown")
367+
self.shuttingdown = True
368+
for node in self.sched.nodes:
369+
node.shutdown()
362370

363371
def handle_crashitem(self, nodeid, worker):
364372
# XXX get more reporting info by recording pytest_runtest_logstart?

testing/acceptance_test.py

+30-4
Original file line numberDiff line numberDiff line change
@@ -109,18 +109,44 @@ def test_skip():
109109
)
110110
assert result.ret == 1
111111

112-
def test_n1_fail_minus_x(self, pytester: pytest.Pytester) -> None:
112+
def test_exitfail_waits_for_workers_to_finish(
113+
self, pytester: pytest.Pytester
114+
) -> None:
115+
"""The DSession waits for workers before exiting early on failure.
116+
117+
When -x/--exitfail is set, the DSession wait for the workers to finish
118+
before raising an Interrupt exception. This prevents reports from the
119+
faiing test and other tests from being discarded.
120+
"""
113121
p1 = pytester.makepyfile(
114122
"""
123+
import time
124+
115125
def test_fail1():
126+
time.sleep(0.1)
116127
assert 0
117128
def test_fail2():
129+
time.sleep(0.2)
130+
def test_fail3():
131+
time.sleep(0.3)
118132
assert 0
133+
def test_fail4():
134+
time.sleep(0.3)
135+
def test_fail5():
136+
time.sleep(0.3)
137+
def test_fail6():
138+
time.sleep(0.3)
119139
"""
120140
)
121-
result = pytester.runpytest(p1, "-x", "-v", "-n1")
141+
result = pytester.runpytest(p1, "-x", "-rA", "-v", "-n2")
122142
assert result.ret == 2
123-
result.stdout.fnmatch_lines(["*Interrupted: stopping*1*", "*1 failed*"])
143+
result.stdout.re_match_lines([".*Interrupted: stopping.*[12].*"])
144+
m = re.search(r"== (\d+) failed, (\d+) passed in ", str(result.stdout))
145+
assert m
146+
n_failed, n_passed = (int(s) for s in m.groups())
147+
assert 1 <= n_failed <= 2
148+
assert 1 <= n_passed <= 3
149+
assert (n_passed + n_failed) < 6
124150

125151
def test_basetemp_in_subprocesses(self, pytester: pytest.Pytester) -> None:
126152
p1 = pytester.makepyfile(
@@ -1150,7 +1176,7 @@ def test_aaa1(crasher):
11501176
"""
11511177
)
11521178
result = pytester.runpytest_subprocess("--maxfail=1", "-n1")
1153-
result.stdout.fnmatch_lines(["* 1 error in *"])
1179+
result.stdout.re_match_lines([".* [12] errors? in .*"])
11541180
assert "INTERNALERROR" not in result.stderr.str()
11551181

11561182

0 commit comments

Comments
 (0)