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

Identify correct test crashed during teardown and support multiple test logs from plugins #218

Merged
merged 2 commits into from
Aug 11, 2017

Conversation

nicoddemus
Copy link
Member

While trying to fix #124, I realized the root of the problem was that xdist was trying to identify when a test was "done" on a worker by looking at the report from the pytest_runtest_logreport, but this is a problem because it is not entirely obvious when a test is actually "done". pytest_runtest_logreport is called for all testing stages, setup, call and teardown, and those stages are called or not depending on specific outcomes:

  • a failed setup prevent's call and teardown stages;
  • a failed call will follow with a teardown stage;

And plugins might even affect that by implementing pytest_runtest_protocol themselves.

So I figured the problem was trying to implicitly figure out if a test was done based on that information.

To solve this I created an explicit event "runtest_protocol_complete" which is sent by workers after runtestprotocol, this way we ensure that a worker is indeed "done" with an item.


I also realized that this fixes #206 because we no longer use pytest_runtest_logreport to determine if a test is done, allowing plugins to issue multiple pytest_runtest_logreport hooks if needed.

I modified pytest-rerunfailures locally to skip its check if xdist is running, allowing it to log multiple times and it works well:

============================= test session starts =============================
platform win32 -- Python 3.5.2, pytest-3.0.7, py-1.4.34, pluggy-0.4.0
rootdir: C:\Users\bruno\pytest-xdist, inifile: tox.ini
plugins: xdist-1.19.1.dev6+gf8e138b, rerunfailures-2.2, forked-0.2, hypothesis-3.16.0
gw0 [1] / gw1 [1]
scheduling tests via LoadScheduling
RRRRRF
=========================== rerun test summary info ===========================
RERUN .tmp/test_reruns.py::test_example
RERUN .tmp/test_reruns.py::test_example
RERUN .tmp/test_reruns.py::test_example
RERUN .tmp/test_reruns.py::test_example
RERUN .tmp/test_reruns.py::test_example
=========================== short test summary info ===========================
FAIL .tmp/test_reruns.py::test_example
================================== FAILURES ===================================
________________________________ test_example _________________________________
[gw0] win32 -- Python 3.5.2 c:\users\bruno\pytest-xdist\.env35\scripts\python.exe

    def test_example():
>       assert False
E       assert False

.tmp\test_reruns.py:3: AssertionError
====================== 1 failed, 5 rerun in 1.04 seconds ======================

I also executed pytest's test suite using this branch and it worked with no problems.

cc @davehunt

Copy link
Member

@RonnyPfannschmidt RonnyPfannschmidt left a comment

Choose a reason for hiding this comment

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

looks good

@nicoddemus nicoddemus merged commit 7b9546b into pytest-dev:master Aug 11, 2017
@nicoddemus nicoddemus deleted the teardown-crash branch August 11, 2017 00:33
@davehunt
Copy link

@nicoddemus I owe you a 🍺!

@nicoddemus
Copy link
Member Author

@davehunt I will take you up on that! 😁

@davehunt
Copy link

@nicoddemus is there a way to determine the plugin version? I'd like to write a patch for pytest-rerunfailures that enables this only when pytest-xdist 1.20.0 or later is in use.

@nicoddemus
Copy link
Member Author

@davehunt I think you can use:

import pkg_resources
d = pkg_resources.get_distribution('pytest-xdist')
if d.parsed_version > pkg_resources.parse_version('1.19'):
    # do something

@RonnyPfannschmidt please correct me if I'm wrong.

@RonnyPfannschmidt
Copy link
Member

@nicoddemus off hand i'd say its preferable to test for >= the known minimal target, but otherwise it looks correct

additionally the code should handle the missing distribution case

@davehunt
Copy link

@nicoddemus @RonnyPfannschmidt see pytest-dev/pytest-rerunfailures#53 for how I got this working, and let me know if you think using pkg_resources would be preferable.

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

Successfully merging this pull request may close these issues.

Allow tests to be logged multiple times with pytest-xdist xdist misidentifies failing test on teardown crash
3 participants