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

xdist bails out too soon when using --exitfirst #54

Open
campbellr opened this issue Apr 12, 2016 · 11 comments
Open

xdist bails out too soon when using --exitfirst #54

campbellr opened this issue Apr 12, 2016 · 11 comments

Comments

@campbellr
Copy link

While investigating pytest-dev/pytest#442 I ran into an issue where xdist behaves differently than regular pytest.

Basically, it seems that when using --exitfirst or --maxfail=X, in combination with -n X, pytest (DSession?) bails out before collecting the teardown report, so the pytest_runtest_logreport doesn't get run.

At first glance, it seems like the the plugin should flush any unread entries from the queue after telling the nodes to stop.

I'm willing to work on this if someone can verify that this makes sense and/or point me in the right direction :)

@RonnyPfannschmidt
Copy link
Member

it makes sense, but the direction is not clear at first glance

we do have a problem of spaghetti state machines in general (state machines that exist via code that randomly interacts)

and this issue seems to be yet another manifestation of exiting one at a non-exitable state

@campbellr
Copy link
Author

I experimented a bit and this change seems to work (whether it is appropriate, I'm not sure):

diff --git a/xdist/dsession.py b/xdist/dsession.py
index fa66f1d..cfc0335 100644
--- a/xdist/dsession.py
+++ b/xdist/dsession.py
@@ -515,8 +515,23 @@ class DSession:
         nm = getattr(self, 'nodemanager', None)  # if not fully initialized
         if nm is not None:
             nm.teardown_nodes()
+            self._drain_queue()
+
         self._session = None

+    def _drain_queue(self):
+        while 1:
+            try:
+                eventcall = self.queue.get_nowait()
+            except queue.Empty:
+                return
+            callname, kwargs = eventcall
+            assert callname, kwargs
+            method = "slave_" + callname
+            call = getattr(self, method)
+            self.log("calling method", method, kwargs)
+            call(**kwargs)
+
     def pytest_collection(self):
         # prohibit collection of test items in master process
         return True
@@ -535,6 +550,7 @@ class DSession:
         while not self.session_finished:
             self.loop_once()
             if self.shouldstop:
+                self.triggershutdown()
                 raise Interrupted(str(self.shouldstop))
         return True

The interaction with --exitfirst is still a bit weird because even with -n 1, I actually end up running/reporting 3 tests before exiting, but i think that's just due to how LoadScheduling distributes tests. I believe they were run before but just not reported.

@RonnyPfannschmidt
Copy link
Member

looks like a correct solution, but also points to the problem, there should be a state-machine that cannot exit until all event sources quit and the queue is drained

@xmo-odoo
Copy link

xmo-odoo commented Jan 23, 2020

An other possible effect of this is it also looks like xdist does not run all session-scoped teardowns properly when terminated early (due to either -x or to a C-c interruption).

I've got fairly long but largely IO-bound tests for the integration of various services. To cut down on the wallclock runtime I'm trying to make it work with xdist and that's largely working, however I've got some set up and teardown of external services using session-scoped fixtures[0] and I regularly find out that some of the service teardowns did not work (and I assume not run) which often breaks the next run if I don't notice.

[0] it's slightly less efficient but fine that these create separate instances of the external services so each xdist worker having its own version of the fixture is not an issue

@rbierbasz
Copy link

It doesn't run any teardowns, not only session-scoped:

import pytest

@pytest.fixture()
def a():
    print("a - setup")
    yield
    print("a - teardown")

def test(a):
    print("test")
    assert False

Executed with xdist:

(core3) ~\repos> pytest -x .\test_teardown.py -n1
========================================================================================================================================================================== test session starts ===========================================================================================================================================================================
platform win32 -- Python 3.8.1, pytest-5.3.5, py-1.8.1, pluggy-0.13.1
rootdir: C:\Users\rbierbasz\repos
plugins: forked-1.1.3, xdist-1.31.0
gw0 [1]
F
================================================================================================================================================================================ FAILURES ================================================================================================================================================================================
__________________________________________________________________________________________________________________________________________________________________________________ test __________________________________________________________________________________________________________________________________________________________________________________
[gw0] win32 -- Python 3.8.1 c:\users\rbierbasz\envs\core3\scripts\python.exe

a = None

    def test(a):
        print("test")
>       assert False
E       assert False

test_teardown.py:11: AssertionError
------------------------------------------------------------------------------------------------------------------------------------------------------------------------- Captured stdout setup --------------------------------------------------------------------------------------------------------------------------------------------------------------------------
a - setup
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------- Captured stdout call --------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! stopping after 1 failures !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! xdist.dsession.Interrupted: stopping after 1 failures !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
=========================================================================================================================================================================== 1 failed in 0.44s ============================================================================================================================================================================

Expected behavior (no -n1 flag):

(core3) ~\repos> pytest -x .\test_teardown.py
========================================================================================================================================================================== test session starts ===========================================================================================================================================================================
platform win32 -- Python 3.8.1, pytest-5.3.5, py-1.8.1, pluggy-0.13.1
rootdir: C:\Users\rbierbasz\repos
plugins: forked-1.1.3, xdist-1.31.0
collected 1 item

test_teardown.py F                                                                                                                                                                                                                                                                                                                                                  [100%]

================================================================================================================================================================================ FAILURES ================================================================================================================================================================================
__________________________________________________________________________________________________________________________________________________________________________________ test __________________________________________________________________________________________________________________________________________________________________________________

a = None

    def test(a):
        print("test")
>       assert False
E       assert False

test_teardown.py:11: AssertionError
------------------------------------------------------------------------------------------------------------------------------------------------------------------------- Captured stdout setup --------------------------------------------------------------------------------------------------------------------------------------------------------------------------
a - setup
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------- Captured stdout call --------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test
------------------------------------------------------------------------------------------------------------------------------------------------------------------------ Captured stdout teardown ------------------------------------------------------------------------------------------------------------------------------------------------------------------------
a - teardown
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! stopping after 1 failures !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
=========================================================================================================================================================================== 1 failed in 0.07s ============================================================================================================================================================================

@RonnyPfannschmidt
Copy link
Member

@rbierbasz i believe this is a reporting mistake due to the teardown running file and the error happening in the call

this is possibly related to the report transfer ptorocol used in xdist and how we disconnect from items/serialize there

@rbierbasz
Copy link

@RonnyPfannschmidt OK, I tested setup and teardown with side effect (touching files) and the files were touched. Thanks.

@RonnyPfannschmidt
Copy link
Member

Thanks for verifying,

@AndreiPashkin
Copy link

Can confirm, seems like xdist doesn't like teardowns at all with -x.

@nicoddemus
Copy link
Member

FTR pytest-xdist needs to implement -x support explicitly.

The master worker sends tests in batches for workers to execute, which report back each test status (success, failure, etc).

With -x active, a worker will stop itself when it encounters a test failure, however the master worker needs to notice that as well, and send a "stop" signal to the other workers so they will stop too.

@qarlosalberto
Copy link

I have the same problem. Any solution? Thanks!

nicoddemus pushed a commit that referenced this issue Nov 11, 2023
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants