Terminate the test loop if shouldfail or shouldstop is set

[bbrown1867]

Terminate the test loop if shouldfail or shouldstop is set in a worker's pytest session

Closes #1024.

[bbrown1867]

@bluetech are we good to merge?

[bbrown1867]

@bluetech I found a new solution that does the same thing (stopping the worker when the global max is reached) without needing any changes to dsession.py. Let me know what you think!

[bluetech]

If I understand the new solution, when reaching shouldstop it "drains" the remaining items from the worker by fake-executing them and reporting them to the DSession. I have a feeling that this faking is going to cause some issues since we're basically skipping over the entire interior of the runtest protocol. Perhaps the assumption is that by this point, the DSession has already processed its own shouldstop and is preparing to shutdown, but this is the sort of race we want to avoid.

Reading the code, I'm thinking perhaps we go back to something like the previous solution, but make it safe.
Previously the worker would just stop, and the dsession would interpret it as a crash, which it might as well have been, no way to know.
I think instead we can add the worker's shouldstop to the worker_workerfinished message. If the dsession sees it, it knows it's not a crash and it can set its own shouldstop (if not already set) and shutdown.

I'm not sure it would work out, but if does I think it will fix the issue cleanly. WDYT?

[bbrown1867]

I'm not sure it would work out, but if does I think it will fix the issue cleanly. WDYT?

Yeah I agree that is a cleaner solution and I like that it propagates the worker session shouldstop/shouldfail status to the DSession. I took a stab at this in c927b1a and it seems to work on the example from #1024.

(.venv) % pytest test_parallel.py  -n 2  -s --maxfail=1
======================================================================== test session starts =========================================================================
platform darwin -- Python 3.8.13, pytest-8.0.0, pluggy-1.4.0
configfile: tox.ini
plugins: xdist-0.1.dev1152+ge11ad4d
2 workers [4 items]
### executing fixture! ###
### executing fixture! ###
F..
============================================================================== FAILURES ==============================================================================
_______________________________________________________________________________ test_1 _______________________________________________________________________________
[gw0] darwin -- Python 3.8.13 /Users/bbrown/Development/fun/pytest-xdist/.venv/bin/python

    def test_1():
>       assert False
E       assert False

test_parallel.py:16: AssertionError
====================================================================== short test summary info =======================================================================
FAILED test_parallel.py::test_1 - assert False
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! stopping after 1 failures !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! xdist.dsession.Interrupted: stopping after 1 failures !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
==================================================================== 1 failed, 2 passed in 2.49s =====================================================================

The failure occurs on gw0 and it exits. gw1 runs both tests successfully, finishing before the failure propagates from gw0 to DSession.

[bluetech]

Thanks for the update, I think the approach is good now. I left a couple of comments on the code.

[bluetech]

Thanks! I will merge in a few days, maybe @RonnyPfannschmidt or @nicoddemus want to take a look as well.

[bluetech]

@bbrown1867 Can you add a changelog/1024.bugfix changelog entry? Would do it myself but no permission to push to your fork. I wrote this entry if you want to use it:

Added proper handling of ``shouldstop`` (such as set by ``--max-fail``) and ``shouldfail`` conditions in workers.
Previously, a worker might have continued executing further tests before the controller could terminate the session.

[bbrown1867]

@bbrown1867 Can you add a changelog/1024.bugfix changelog entry? Would do it myself but no permission to push to your fork. I wrote this entry if you want to use it:

Done! Thanks for drafting that