pytest-dev / pytest

The pytest framework makes it easy to write small tests, yet scales to support complex functional testing
https://pytest.org
MIT License
11.9k stars 2.65k forks source link

raising an exception pytest_runtest_teardown causes all all further tests to fail #7724

Open torcolvin opened 4 years ago

torcolvin commented 4 years ago

Raising an exception in pytest_runtest_teardown in one test will cause an exception in all subsequent tests. This is a regression from 5.2.4 and earlier releases. I use exceptions in pytest_runtest_teardown to assert resource cleanup for all tests regardless of other fixtures that are used. It's unusual to fail in this manner, but this makes it look like all tests are failing.

Here is the output from pytest 6.0.1, where it marks test_a as a failure in the expected fashion. However it marks test_b as a failure with assert colitem in self.stack. If there are more tests, all tests will be marked with the same failure message as test_b.

============================= test session starts ==============================
platform linux -- Python 3.8.2, pytest-6.0.1, py-1.9.0, pluggy-0.13.1
rootdir: /home/colvin/pytest_repro
collected 2 items

test_exception.py .E.E                                                   [100%]

==================================== ERRORS ====================================
_________________________ ERROR at teardown of test_a __________________________

item = <Function test_a>, nextitem = <Function test_b>

    def pytest_runtest_teardown(item, nextitem):
        global raised
        if not raised:
            raised = True
>           assert False, "fails raising a plain assert"
E           AssertionError: fails raising a plain assert
E           assert False

conftest.py:9: AssertionError
_________________________ ERROR at teardown of test_b __________________________

cls = <class '_pytest.runner.CallInfo'>
func = <function call_runtest_hook.<locals>.<lambda> at 0x7f83ab0d4940>
when = 'teardown'
reraise = (<class '_pytest.outcomes.Exit'>, <class 'KeyboardInterrupt'>)

    @classmethod
    def from_call(
        cls,
        func: "Callable[[], _T]",
        when: "Literal['collect', 'setup', 'call', 'teardown']",
        reraise: "Optional[Union[Type[BaseException], Tuple[Type[BaseException], ...]]]" = None,
    ) -> "CallInfo[_T]":
        excinfo = None
        start = timing.time()
        precise_start = timing.perf_counter()
        try:
>           result = func()  # type: Optional[_T]

../pytest-venv/lib/python3.8/site-packages/_pytest/runner.py:294:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
../pytest-venv/lib/python3.8/site-packages/_pytest/runner.py:247: in <lambda>
    lambda: ihook(item=item, **kwds), when=when, reraise=reraise
../pytest-venv/lib/python3.8/site-packages/pluggy/hooks.py:286: in __call__
    return self._hookexec(self, self.get_hookimpls(), kwargs)
../pytest-venv/lib/python3.8/site-packages/pluggy/manager.py:93: in _hookexec
    return self._inner_hookexec(hook, methods, kwargs)
../pytest-venv/lib/python3.8/site-packages/pluggy/manager.py:84: in <lambda>
    self._inner_hookexec = lambda hook, methods, kwargs: hook.multicall(
../pytest-venv/lib/python3.8/site-packages/_pytest/runner.py:166: in pytest_runtest_teardown
    item.session._setupstate.teardown_exact(item, nextitem)
../pytest-venv/lib/python3.8/site-packages/_pytest/runner.py:402: in teardown_exact
    self._teardown_towards(needed_collectors)
../pytest-venv/lib/python3.8/site-packages/_pytest/runner.py:417: in _teardown_towards
    raise exc
../pytest-venv/lib/python3.8/site-packages/_pytest/runner.py:410: in _teardown_towards
    self._pop_and_teardown()
../pytest-venv/lib/python3.8/site-packages/_pytest/runner.py:370: in _pop_and_teardown
    self._teardown_with_finalization(colitem)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <_pytest.runner.SetupState object at 0x7f83ab11fc70>
colitem = <Function test_b>

    def _teardown_with_finalization(self, colitem) -> None:
        self._callfinalizers(colitem)
        colitem.teardown()
        for colitem in self._finalizers:
>           assert colitem in self.stack
E           AssertionError

../pytest-venv/lib/python3.8/site-packages/_pytest/runner.py:391: AssertionError
=========================== short test summary info ============================
ERROR test_exception.py::test_a - AssertionError: fails raising a plain assert
ERROR test_exception.py::test_b - AssertionError
========================= 2 passed, 2 errors in 0.04s ==========================

Here's is the output from pytest 5.4.3

=============================== test session starts ================================
platform linux -- Python 3.8.2, pytest-5.4.3, py-1.9.0, pluggy-0.13.1
rootdir: /home/colvin/pytest_repro
collected 2 items

test_exception.py .E.                                                        [100%]

====================================== ERRORS ======================================
___________________________ ERROR at teardown of test_a ____________________________

item = <Function test_a>, nextitem = <Function test_b>

    def pytest_runtest_teardown(item, nextitem):
        global raised
        if not raised:
            raised = True
>           assert False, "fails raising a plain assert"
E           AssertionError: fails raising a plain assert
E           assert False

conftest.py:9: AssertionError
============================= short test summary info ==============================
ERROR test_exception.py::test_a - AssertionError: fails raising a plain assert
============================ 2 passed, 1 error in 0.01s ============================

To reproduce, I use two files:

conftest.py

import pytest

raised = False

def pytest_runtest_teardown(item, nextitem):
    global raised
    if not raised:
        raised = True
        assert False, "fails raising a plain assert"

test_exception.py

def test_a():
    assert True

def test_b():
    assert True

pip list from python 3.8.2 on ubuntu 20.04

Package        Version
-------------- -------
attrs          19.3.0
iniconfig      1.0.1
more-itertools 8.4.0
packaging      20.4
pip            20.0.2
pkg-resources  0.0.0
pluggy         0.13.1
py             1.9.0
pyparsing      2.4.7
pytest         6.0.1
setuptools     44.0.0
six            1.15.0
toml           0.10.1
wcwidth        0.2.5
The-Compiler commented 4 years ago

Bisected to bb878a2b13508e88ace7718759065de56a63aac5 (runner: don't try to teardown previous items from pytest_runtest_setup) from #7368 - cc @bluetech

bluetech commented 4 years ago

Thanks for the great report @torcolvin, and @The-Compiler for the bisection.

What happens here is:

pytest itself uses the pytest_runtest_teardown hook for some work, and assumes that it runs. Thus the scenario breaks it.

I consider it more of an issue with the user code than a regression in pytest, but maybe others will disagree.

In any case the simplest fix for you would be

@pytest.hookimpl(trylast=True)
def pytest_runtest_teardown(item, nextitem):
    ....

This makes your hookimpl run after pytest does its thing. This would have been a good idea even in previous pytest versions.

torcolvin commented 4 years ago

i think using pytest.hookimpl(trylast=True) could be a good idea in the use case I have this case, but if more than one plugin registers a teardown hook, we can end up right back where we started. I can see why this situation might be unavoidable, but maybe the exception assert colitem in self.stack could say something about this happening because a previous teardown raised an exception? Or maybe an exception raised here should terminate the test harness, since further results are invalidated?

nicoddemus commented 4 years ago

I can see why this situation might be unavoidable, but maybe the exception assert colitem in self.stack could say something about this happening because a previous teardown raised an exception?

I think we can improve that exception message as you suggest.

FWIW, I used an autouse fixture for the same purpose in the past:

@pytest.fixture(autouse=True)
def ensure_cleanup():
    yield
    cleanup_all_resources()

Fixtures raising an exception are expected and pytest handles them correctly, but with hooks things are a bit more delicate.