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

TestCase.skipTest() skips _post_teardown #772

Closed
lanzz opened this issue Oct 18, 2019 · 8 comments · Fixed by #782
Closed

TestCase.skipTest() skips _post_teardown #772

lanzz opened this issue Oct 18, 2019 · 8 comments · Fixed by #782
Labels

Comments

@lanzz
Copy link

lanzz commented Oct 18, 2019

TestCase.skipTest() will raise a SkipTest exception, which is not caught in _cleaning_debug, so the _post_teardown() call is skipped. Also apparently cleanups registered with TestCase.addCleanup() will not be run. If setup done in _pre_setup needs to be undone in _post_teardown (or in cleanups scheduled with addCleanup()), that will not happen for tests skipped with skipTest().

This does not apply to tests skipped with a @unittest.skip decorator.

For comparison, nose runs _post_teardown methods for tests skipped with skipTest().

@blueyed
Copy link
Contributor

blueyed commented Oct 18, 2019

Thanks for the report.
I assume this works better with Django master (3.1), where this gets handled by Django now? (#773)

Can you provide a failing test, similar to https://github.com/blueyed/pytest-django/blob/c023564e708b5c55b57df43776d21643d1f1f2ce/tests/test_unittest.py#L461-L489 I guess?

@blueyed blueyed added the bug label Oct 18, 2019
@blueyed
Copy link
Contributor

blueyed commented Oct 18, 2019

So it's only a problem when using --pdb after all?
In general the finalizer gets used:

test_case._pre_setup()
request.addfinalizer(test_case._post_teardown)

But is skipped explicitly in _cleaning_debug:

super(cls, self).debug()
if not skipped:
self._post_teardown()

@blueyed
Copy link
Contributor

blueyed commented Oct 18, 2019

Django does the same: https://github.com/django/django/pull/7436/files#diff-5d7d8ead1a907fe91ffc121f830f2a49R272-R279

This does not apply to tests skipped with a @unittest.skip decorator.

Interesting. Likely because the test is not being invoked in the first place though.

@lanzz
Copy link
Author

lanzz commented Oct 18, 2019

This might be a bigger issue with pytest itself, it does not run tearDown at all for skipped tests when running with --pdb: pytest-dev/pytest#5991

@blueyed
Copy link
Contributor

blueyed commented Oct 18, 2019

pytest-dev/pytest#5996 should fix the issue in pytest.

But pytest-django would still not call self._post_teardown when a test is skipped:

if not skipped:
self._pre_setup()
super(cls, self).debug()
if not skipped:
self._post_teardown()

However, with pytest not calling .debug anymore it would only affect you when calling .debug() yourself.

AFAICS _cleaning_debug was only necessary due to the misuse in pytest itself (#406, /cc @asfaltboy).

@blueyed
Copy link
Contributor

blueyed commented Oct 18, 2019

Not calling _post_teardown (the Django method) is in line with Django, but I've created django/django#11938 to fix/address this.

blueyed added a commit to blueyed/pytest-django that referenced this issue Oct 18, 2019
blueyed added a commit to blueyed/pytest-django that referenced this issue Oct 18, 2019
blueyed added a commit to blueyed/pytest that referenced this issue Nov 9, 2019
Fixes pytest-dev#5991
Fixes pytest-dev#3823

Ref: pytest-dev/pytest-django#772
Ref: pytest-dev#1890
Ref: pytest-dev/pytest-django#782

- inject wrapped testMethod

- adjust test_trial_error

- add test for `--trace` with unittests
blueyed added a commit to blueyed/pytest that referenced this issue Nov 9, 2019
Fixes pytest-dev#5991
Fixes pytest-dev#3823

Ref: pytest-dev/pytest-django#772
Ref: pytest-dev#1890
Ref: pytest-dev/pytest-django#782

- inject wrapped testMethod

- adjust test_trial_error

- add test for `--trace` with unittests
@blueyed
Copy link
Contributor

blueyed commented Nov 9, 2019

I believe #782 fixes it.
Please test it, I will release it then shortly.

@blueyed
Copy link
Contributor

blueyed commented Nov 9, 2019

Releasing as v3.7.0 already..

blueyed added a commit to blueyed/pytest that referenced this issue Nov 9, 2019
Fixes pytest-dev#5991
Fixes pytest-dev#3823

Ref: pytest-dev/pytest-django#772
Ref: pytest-dev#1890
Ref: pytest-dev/pytest-django#782

- inject wrapped testMethod

- adjust test_trial_error

- add test for `--trace` with unittests
blueyed added a commit to blueyed/pytest that referenced this issue Nov 9, 2019
Fixes pytest-dev#5991
Fixes pytest-dev#3823

Ref: pytest-dev/pytest-django#772
Ref: pytest-dev#1890
Ref: pytest-dev/pytest-django#782

- inject wrapped testMethod

- adjust test_trial_error

- add test for `--trace` with unittests
blueyed added a commit to blueyed/pytest that referenced this issue Nov 9, 2019
Fixes pytest-dev#5991
Fixes pytest-dev#3823

Ref: pytest-dev/pytest-django#772
Ref: pytest-dev#1890
Ref: pytest-dev/pytest-django#782

- inject wrapped testMethod

- adjust test_trial_error

- add test for `--trace` with unittests
blueyed added a commit to blueyed/pytest that referenced this issue Nov 9, 2019
Fixes pytest-dev#5991
Fixes pytest-dev#3823

Ref: pytest-dev/pytest-django#772
Ref: pytest-dev#1890
Ref: pytest-dev/pytest-django#782

- inject wrapped testMethod

- adjust test_trial_error

- add test for `--trace` with unittests
blueyed added a commit to blueyed/pytest that referenced this issue Nov 9, 2019
Fixes pytest-dev#5991
Fixes pytest-dev#3823

Ref: pytest-dev/pytest-django#772
Ref: pytest-dev#1890
Ref: pytest-dev/pytest-django#782

- inject wrapped testMethod

- adjust test_trial_error

- add test for `--trace` with unittests
blueyed added a commit to blueyed/pytest that referenced this issue Nov 9, 2019
Fixes pytest-dev#5991
Fixes pytest-dev#3823

Ref: pytest-dev/pytest-django#772
Ref: pytest-dev#1890
Ref: pytest-dev/pytest-django#782

- inject wrapped testMethod

- adjust test_trial_error

- add test for `--trace` with unittests
blueyed added a commit to blueyed/pytest that referenced this issue Nov 9, 2019
Fixes pytest-dev#5991
Fixes pytest-dev#3823

Ref: pytest-dev/pytest-django#772
Ref: pytest-dev#1890
Ref: pytest-dev/pytest-django#782

- inject wrapped testMethod

- adjust test_trial_error

- add test for `--trace` with unittests
vinaycalastry pushed a commit to vinaycalastry/pytest that referenced this issue Dec 11, 2019
Fixes pytest-dev#5991
Fixes pytest-dev#3823

Ref: pytest-dev/pytest-django#772
Ref: pytest-dev#1890
Ref: pytest-dev/pytest-django#782

- inject wrapped testMethod

- adjust test_trial_error

- add test for `--trace` with unittests
blueyed added a commit to blueyed/pytest-django that referenced this issue Mar 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants