Skip to content

Pytest aborts when fixture errors during teardown and --maxfail=1 #11706

Description

@bbrown1867
  • a detailed description of the bug or problem you are having
  • output of pip list from the virtual environment you are using
  • pytest and operating system versions
  • minimal example if possible

Description

Pytest aborts if a session-scoped fixture errors during teardown when (a) one or more test cases fail and (b) --maxfail=1 is used.

When this happens, test-report.xml does not contain the teardown fixture error. This is problematic as users may want to also record teardown fixture errors when their test cases fail.

Minimal Example

Code

import pytest


def some_condition() -> bool:
    return True


@pytest.fixture(scope="session", autouse=True)
def my_end_of_session_check():
    yield

    print("Running end of session check!")
    if some_condition():
        pytest.fail("Oh noes")


def test_case_1():
    pytest.fail("This is a failing test")


def test_case_2():
     pytest.fail("This is also a failing test")

Args

pytest example.py --maxfail=1 --junitxml=test-report.xml

Output

Console:

============================================================================== test session starts ===============================================================================
platform darwin -- Python 3.8.13, pytest-7.4.3, pluggy-1.3.0
rootdir: /Users/bbrown/Downloads
collected 2 items

example.py FRunning end of session check!
Traceback (most recent call last):
  File "/Users/bbrown/Downloads/.venv/bin/pytest", line 8, in <module>
    sys.exit(console_main())
  <clipped>
  File "/Users/bbrown/Downloads/.venv/lib/python3.8/site-packages/_pytest/fixtures.py", line 911, in _teardown_yield_fixture
    next(it)
  File "/Users/bbrown/Downloads/example.py", line 14, in my_end_of_session_check
    pytest.fail("Oh noes")
  File "/Users/bbrown/Downloads/.venv/lib/python3.8/site-packages/_pytest/outcomes.py", line 198, in fail
    raise Failed(msg=reason, pytrace=pytrace)
Failed: Oh noes

test-report.xml:

<?xml version="1.0" encoding="utf-8"?><testsuites><testsuite name="pytest" errors="0" failures="1" skipped="0" tests="1" time="0.035" timestamp="2023-12-15T08:56:19.980572" hostname="macbookpro"><testcase classname="example" name="test_case_1" time="0.001"><failure message="Failed: This is a failing test">def test_case_1():
&gt;       pytest.fail("This is a failing test")
E       Failed: This is a failing test

example.py:18: Failed</failure></testcase></testsuite></testsuites>

Expected Output

I expect pytest to not abort, and report "1 failed, 1 error, 1 skipped" in both the console and test-report.xml when this happens.

Environment

OS:

macOS Sonoma 14.1.2

Virtual environment:

(.venv) % pip list
Package        Version
-------------- -------
exceptiongroup 1.2.0
iniconfig      2.0.0
packaging      23.2
pip            23.3.1
pluggy         1.3.0
pytest         7.4.3
setuptools     56.0.0
tomli          2.0.1

Activity

  1. changed the title [-]Test case failure + session-scoped fixture teardown error with `--maxfail=1` leads to pytest aborting[/-] [+]Pytest aborts when session-scoped fixture errors during teardown and `--maxfail=1`[/+] on Dec 15, 2023
  2. bbrown1867 commented on Dec 15, 2023

    @bbrown1867
    ContributorAuthor

    Note that this also happens when the fixture is module scoped, but does not happen if the fixture is function scoped. Will update the issue name to indicate this.

  3. changed the title [-]Pytest aborts when session-scoped fixture errors during teardown and `--maxfail=1`[/-] [+]Pytest aborts when fixture errors during teardown and `--maxfail=1`[/+] on Dec 15, 2023
  4. bluetech commented on Dec 16, 2023

    @bluetech
    Member

    This is an interesting edge case you've stumbled upon.

    Background

    pytest's setups/teardowns are managed by the SetupState class.

    You can see the docstring for how it works, but the important part here is that we're tearing down some item (test), we look at the next item (nextitem), see which fixtures the nextitem won't be needing, and tear them down in the context of item. In particular, the last item gets nextitem = None which tears down everything.

    When --maxfail=N is used (or -x which is alias for --maxfail=1), if the N failures are reached, the runner sets session.shouldfail = ..., then the runner loop checks it and exits the loop.

    Problem

    Let's call the Nth failing test "test N" and assume there is at least one test after it, "test N + 1", which shares some higher-scoped fixtures.

    "test N" is torn down with nextitem = test N + 1, then shouldfail is set and the loop exits. But, all of the remaining fixtures weren't torn down because nothing told them to.

    To fix this, pytest in a last ditch effort runs the remaining teardowns in a pytest_sessionfinish hook.

    But this pytest_sessionfinish hook currently is not written to handle exceptions, except for pytest.exit, even though it executes user code. So if one of the remaining teardowns raises, the exception just propagates.

    Possible solution 1

    Catch the exceptions from the pytest_sessionfinish and issue some warning.

    This would work, however then the teardown failures aren't reported in the context of any test. In particular they won't show in any xml reports and such.

    Possible solution 2

    Detect when an item had set shouldfail = True, then instead of tearing it down with nextitem = item N+1, tear it down with nextitem = None.

    This would work and run the teardowns in the context of the test, but is a bit dangerous - doesn't handle the case where plugins reset shouldfail. Also, I think there are other cases where we bail early, those would still need pytest_sessionfinish and the problem would still be possible there.

  5. bbrown1867 commented on Dec 18, 2023

    @bbrown1867
    ContributorAuthor

    Thanks for the detailed reply @bluetech!

    The real pain point here is the lack of reporting - my team's observability tools consume test-report.xml and want all errors/failures to be reported such that we can triage them. Because of that, I'll try to investigate Possible solution 2 you've outlined above.

  6. bbrown1867 commented on Dec 18, 2023

    @bbrown1867
    ContributorAuthor

    Detect when an item had set shouldfail = True, then instead of tearing it down with nextitem = item N+1, tear it down with nextitem = None.

    This was pretty easy to implement, basically a one-line change in pytest_runtest_teardown:

    def pytest_runtest_teardown(item: Item, nextitem: Optional[Item]) -> None:
        _update_current_test_var(item, "teardown")
        nextitem = None if item.session.shouldfail else nextitem
        item.session._setupstate.teardown_exact(nextitem)
        _update_current_test_var(item, None)

    I also added a unittest in test_runner.py mimicking the sample code above. I can open a PR with this change, however I'm not sure what to do about these comments:

    This would work and run the teardowns in the context of the test, but is a bit dangerous - doesn't handle the case where plugins reset shouldfail.

    A plugin resetting shouldfail feels like even more of an edge-case, not sure if we should be that concerned about it?

    Also, I think there are other cases where we bail early, those would still need pytest_sessionfinish and the problem would still be possible there.

    I'm not really following this comment, likely just due to my lack of context about the codebase.

  7. bluetech commented on Feb 23, 2024

    @bluetech
    Member

    Reopening since the fix was reverted.

  8. bbrown1867 commented on Apr 2, 2024

    @bbrown1867
    ContributorAuthor

    @bluetech do you think we can "revert the revert" (93cd7ba) now that we have a fix in pytest-xdist? Or will we need to wait for a release of pytest-xdist?

  9. bluetech commented on Apr 6, 2024

    @bluetech
    Member

    Let's wait for a pytest-xdist release as there is no rush on the pytest side (there's probably some time until we release 8.2).

    I think we will do a pytest-xdist release soon (once we tie up so some typing changes there). I will notify you when we can reapply the fix here (unless I forget...).

    I do foresee we'll get some complaints from users who will be using new pytest with old pytest-xdist, since we can't use dependency constraints to prevent this scenario, but that's OK.

  10. bluetech commented on Apr 28, 2024

    @bluetech
    Member

    I think we will do a pytest-xdist release soon (once we tie up so some typing changes there). I will notify you when we can reapply the fix here (unless I forget...).

    @bbrown1867 We has some issues with the 3.6.0 xdist release which had to be yanked, but I released 3.6.1 now and hopefully it'd be fine. So we can proceed on the pytest side.

  11. bbrown1867 commented on May 1, 2024

    @bbrown1867
    ContributorAuthor

    Okay sounds good, I'll open a PR to revert 4cc87ad

  12. added a commit that references this issue on May 2, 2024
    4c5298c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    type: bugproblem that needs to be addressed

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions