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

Recommended fixture yield implementation doesn't execute teardown if setup raises exception #2508

Closed
samjl opened this issue Jun 19, 2017 · 4 comments
Labels
type: docs documentation improvement, missing or needing clarification

Comments

@samjl
Copy link

samjl commented Jun 19, 2017

Using fixtures for setup and teardown:
Difference between older addfinalizer approach and recommended approach with yield only.

Note: The old method described here does still work (as mentioned in the documentation note), but I wish to highlight the different behaviour and the usefulness of the old method before the old method is removed entirely.

Current recommendation as documented in the second note here https://docs.pytest.org/en/latest/fixture.html#fixture-finalization-executing-teardown-code. If this method (do not add finalizer) is used and the setup code raises an exception then the teardown code is not executed.

Old behaviour (no longer recommended), using request.addfinalizer:
If setup code (before yield) raises an exception then the teardown code (after yield) is executed as long as addfinalizer is before the setup code as below,

# Old method - always executes the teardown code, even when setup raises exception
@pytest.fixture(scope='function')
def setupTeardown(request):
    def fin():
        print "Function teardown"
    request.addfinalizer(fin)
    print "Function setup"
    setupPassed = False
    assert setupPassed, "Argh setup failed"
    yield

Example output:

platform linux2 -- Python 2.7.6, pytest-3.1.2, py-1.4.34, pluggy-0.4.0
rootdir: /home/slea1/workspace/sessions, inifile:
collected 1 items 
testcases/testDir2/test_cnet-3_pytest_Fixtures_bug.py 
Function setup
E
Function teardown
========================================================== short test summary info ==========================================================
ERROR testcases/testDir2/test_cnet-3_pytest_Fixtures_bug.py::test_fixtureSetupFail
================================================================== ERRORS ===================================================================
__________________________________________________ ERROR at setup of test_fixtureSetupFail __________________________________________________
request = <SubRequest 'setupTeardown' for <Function 'test_fixtureSetupFail'>>
    @pytest.fixture(scope='function')
    def setupTeardown(request):
        def fin():
            print "\nFunction teardown"
        request.addfinalizer(fin)
        print "\nFunction setup"
        setupPassed = False
>       assert setupPassed, "Argh setup failed"
E       AssertionError: Argh setup failed
E       assert False
testcases/testDir2/test_cnet-3_pytest_Fixtures_bug.py:23: AssertionError

Notice that the teardown code "Function teardown" is executed after the exception is raised.

Recommended method in documentation (do not use addfinalizer)
If setup raises an exception the teardown code is not executed.

# New method - doesn't execute teardown if exception caught in setup
@pytest.fixture(scope='function')
def setupTeardown(request):
    print "\nFunction setup"
    setupPassed = False
    assert setupPassed, "Argh setup failed"
    yield
    # This teardown doesn't execute if setup fails
    print "\nFunction teardown"

Example output:

platform linux2 -- Python 2.7.6, pytest-3.1.2, py-1.4.34, pluggy-0.4.0
rootdir: /home/slea1/workspace/sessions, inifile:
collected 1 items 
testcases/testDir2/test_cnet-3_pytest_Fixtures_bug.py 
Function setup
E
========================================================== short test summary info ==========================================================
ERROR testcases/testDir2/test_cnet-3_pytest_Fixtures_bug.py::test_fixtureSetupFail
================================================================== ERRORS ===================================================================
__________________________________________________ ERROR at setup of test_fixtureSetupFail __________________________________________________
request = <SubRequest 'setupTeardown' for <Function 'test_fixtureSetupFail'>>
    @pytest.fixture(scope='function')
    def setupTeardown(request):
        print "\nFunction setup"
        setupPassed = False
>       assert setupPassed, "Argh setup failed"
E       AssertionError: Argh setup failed
E       assert False
testcases/testDir2/test_cnet-3_pytest_Fixtures_bug.py:8: AssertionError

Notice that the "Function teardown" is not printed.

Test code used in the above examples:

# session_setupPass_teardownPass, module_setupPass_teardownPass, class_setupPass_teardownPass, setupPass_teardownPass
def test_fixtureSetupFail(setupTeardown):
    print "Test function executing..."
    print "Test function complete"

pip list

Package      Version
------------ -------
asn1crypto   0.22.0 
bcrypt       3.1.3  
cffi         1.10.0 
colored      1.3.5  
config       0.3.9  
cryptography 1.9    
enum34       1.1.6  
idna         2.5    
ipaddress    1.0.18 
paramiko     2.2.1  
pip          9.0.1  
py           1.4.34 
pyasn1       0.2.3  
pycparser    2.17   
PyNaCl       1.1.2  
pytest       3.1.2  
scp          0.10.2 
setuptools   36.0.1 
six          1.10.0 
wheel        0.29.0

python version: 2.7.6
Operating system: Kubuntu 14.04

Related to issues: #2195, #2440

@nicoddemus
Copy link
Member

Thanks for the detailed description @samjl.

You are right and both methods have the difference you point out. Unfortunately I think there's no way to actually fix this because of how yield works.

By using yield, if an exception happens there's no way for pytest to handle the error and somehow resume the generator, because the function never even yielded in the first place.

Using addfinalizer, pytest will keep an internal list of finalize functions which can be called later at its own leisure, regardless if setup fails or not.

I think we should update the docs to reflect that difference as suggested in #2195. I still would like to recommend yield in general for single resource management, because it better describes the flow of execution and doesn't make sense to call teardown code for a single resource if it failed to be properly created anyway. But addfinalizer is useful when your fixture deals with multiple resources, which is common when converting from unittest-style setUp methods which traditionally create a lot of resources and make use of addCleanup.

Btw, it was never our intention to eventually remove addfinalizer, we just decided to recommend using yield in general because it is (arguably) simpler to use.

@nicoddemus nicoddemus added the type: docs documentation improvement, missing or needing clarification label Jun 21, 2017
@samjl
Copy link
Author

samjl commented Jun 21, 2017

Great, thanks for the update.

We do a lot of embedded software testing where it is ideal to use a setup fixture(s) to perform a fairly complex setup before a test or set of tests. If the setup subsequently fails it is convenient for us to go through the same teardown process that a successful test would have performed to ensure it is restored to the previous default configuration before the next set of tests.

Rather than using addfinalizer I could create a teardown function that could be called after the yield and if the setup code fails (by placing the setup in a try...except). Something like this perhaps:

@pytest.fixture(scope='function')
def setupConfig(request):
    try:
        print "Function setup"
        setupPassed = False
        assert setupPassed, "Argh setup failed"
    except Exception as e:
        # Teardown the failed configuration and re-raise the exception
        teardownConfig()
        raise e
    yield
    teardownConfig()

def teardownConfig():
    print "Teardown code..."

def test_fixtureSetupFail(setupConfig):
    print "Test function executing..."
    print "Test function complete"

Maybe this describes the flow a little better?

I agree that in most cases simply using yield is preferred as it better describes the flow.

@nicoddemus
Copy link
Member

nicoddemus commented Jun 21, 2017

We do a lot of embedded software testing where it is ideal to use a setup fixture(s) to perform a fairly complex setup before a test or set of tests. If the setup subsequently fails it is convenient for us to go through the same teardown process that a successful test would have performed to ensure it is restored to the previous default configuration before the next set of tests.

I see, thanks. This is a perfect case where multiple addfinalizer calls make sense. 👍

Maybe this describes the flow a little better?

This is a matter of opinion of course, but I think if you already isolated the teardown code into a function you might as well just call request.addfinalizer and avoid the try/except block:

@pytest.fixture(scope='function')
def setupConfig(request):
    print "Function setup"
    request.addfinalizer(teardownConfig)
    setupPassed = False
    assert setupPassed, "Argh setup failed"
    return

@samjl
Copy link
Author

samjl commented Jun 21, 2017

Yes I think that is the neatest solution. Thanks very much for your assistance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: docs documentation improvement, missing or needing clarification
Projects
None yet
Development

No branches or pull requests

2 participants