Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Properly wait for workers when test run terminates early. #963

Merged
merged 5 commits into from
Nov 11, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions src/xdist/dsession.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,15 @@ def pytest_runtestloop(self):
assert self.sched is not None

self.shouldstop = False
pending_exception = None
while not self.session_finished:
self.loop_once()
if self.shouldstop:
self.triggershutdown()
raise Interrupted(str(self.shouldstop))
if not self.shuttingdown:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps it would be safer/less error prone to move this if not self.shuttingdown to triggershutdown(), so it becomes a no-op if self.shuttingdown is already True?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that. I'll make the change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bit puzzled about the change log entry. Is 'improvement' supposed to be the $type in the name of the changelog file? I ask because it is not one of the defined types, if my understanding is correct,

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, something like changelog/963.improvement.rst . 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. Done.

self.triggershutdown()
pending_exception = Interrupted(str(self.shouldstop))
if pending_exception:
raise pending_exception
return True

def loop_once(self):
Expand Down Expand Up @@ -351,7 +355,11 @@ def _failed_worker_collectreport(self, node, rep):
def _handlefailures(self, rep):
if rep.failed:
self.countfailures += 1
if self.maxfail and self.countfailures >= self.maxfail:
if (
self.maxfail
and self.countfailures >= self.maxfail
and not self.shouldstop
):
self.shouldstop = f"stopping after {self.countfailures} failures"

def triggershutdown(self):
Expand Down
34 changes: 30 additions & 4 deletions testing/acceptance_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,18 +109,44 @@ def test_skip():
)
assert result.ret == 1

def test_n1_fail_minus_x(self, pytester: pytest.Pytester) -> None:
def test_exitfail_waits_for_workers_to_finish(
self, pytester: pytest.Pytester
) -> None:
"""The DSession waits for workers before exiting early on failure.

When -x/--exitfail is set, the DSession wait for the workers to finish
before raising an Interrupt exception. This prevents reports from the
faiing test and other tests from being discarded.
"""
p1 = pytester.makepyfile(
"""
import time

def test_fail1():
time.sleep(0.1)
assert 0
def test_fail2():
time.sleep(0.2)
def test_fail3():
time.sleep(0.3)
assert 0
def test_fail4():
time.sleep(0.3)
def test_fail5():
time.sleep(0.3)
def test_fail6():
time.sleep(0.3)
"""
)
result = pytester.runpytest(p1, "-x", "-v", "-n1")
result = pytester.runpytest(p1, "-x", "-rA", "-v", "-n2")
assert result.ret == 2
result.stdout.fnmatch_lines(["*Interrupted: stopping*1*", "*1 failed*"])
result.stdout.re_match_lines([".*Interrupted: stopping.*[12].*"])
m = re.search(r"== (\d+) failed, (\d+) passed in ", str(result.stdout))
assert m
n_failed, n_passed = (int(s) for s in m.groups())
assert 1 <= n_failed <= 2
assert 1 <= n_passed <= 3
assert (n_passed + n_failed) < 6

def test_basetemp_in_subprocesses(self, pytester: pytest.Pytester) -> None:
p1 = pytester.makepyfile(
Expand Down Expand Up @@ -1150,7 +1176,7 @@ def test_aaa1(crasher):
"""
)
result = pytester.runpytest_subprocess("--maxfail=1", "-n1")
result.stdout.fnmatch_lines(["* 1 error in *"])
result.stdout.re_match_lines([".* [12] errors? in .*"])
assert "INTERNALERROR" not in result.stderr.str()


Expand Down
Loading