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.

Sign up for GitHub

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

Jump to bottom

Fix cleanup functions not being invoked on test failures #7151

Merged
merged 2 commits into from
May 2, 2020
Merged

Fix cleanup functions not being invoked on test failures #7151

merged 2 commits into from
May 2, 2020

Conversation

nicoddemus
Copy link
Member

Fix #6947

RonnyPfannschmidt
RonnyPfannschmidt approved these changes May 1, 2020
View reviewed changes
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.

its generally a good idea to unify the wrapping

its unfortunate the async cant be integrated easily/sanely

src/_pytest/unittest.py Show resolved Hide resolved
nicoddemus
nicoddemus commented May 1, 2020
View reviewed changes
testing/test_unittest.py
@@ -537,28 +537,24 @@ def f(_):
)
result.stdout.fnmatch_lines(
[
"test_trial_error.py::TC::test_four SKIPPED",
"test_trial_error.py::TC::test_four FAILED",
Copy link
Member Author

Choose a reason for hiding this comment

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

This reverts the test to the state before 04f27d4, which introduced the breaking change about addCleanup not being called properly for failed tests.

@nicoddemus nicoddemus force-pushed the unittest-cleanup-6947 branch from a8e13e1 to a50fe4a Compare May 1, 2020 21:20
@nicoddemus nicoddemus added the needs backport applied to PRs, indicates that it should be ported to the current bug-fix branch label May 1, 2020
@nicoddemus nicoddemus force-pushed the unittest-cleanup-6947 branch 2 times, most recently from 20ac646 to e244e32 Compare May 2, 2020 14:22
nicoddemus
nicoddemus commented May 2, 2020
View reviewed changes
src/_pytest/unittest.py

# let the unittest framework handle async functions
if is_async_function(self.obj):
self._testcase(self)
else:
setattr(self._testcase, self._testcase._testMethodName, wrapped_testMethod)
# when --pdb is given, we want to postpone calling tearDown() otherwise
Copy link
Member Author

Choose a reason for hiding this comment

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

Can you please review this bit @RonnyPfannschmidt? I noticed this was needed after one of the tests failed.

Copy link
Member

Choose a reason for hiding this comment

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

i'm actually YOLO on that one, my insight to the implementation of unittest is limited

RonnyPfannschmidt
RonnyPfannschmidt approved these changes May 2, 2020
View reviewed changes
src/_pytest/unittest.py Outdated Show resolved Hide resolved
src/_pytest/unittest.py

# let the unittest framework handle async functions
if is_async_function(self.obj):
self._testcase(self)
else:
setattr(self._testcase, self._testcase._testMethodName, wrapped_testMethod)
# when --pdb is given, we want to postpone calling tearDown() otherwise
Copy link
Member

Choose a reason for hiding this comment

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

i'm actually YOLO on that one, my insight to the implementation of unittest is limited

nicoddemus added 2 commits May 2, 2020 15:26
@nicoddemus
Fix cleanup functions not being invoked on test failures
5c2e96c 
Also delay calling tearDown() when --pdb is given, so users still have
access to the instance variables (which are usually cleaned up during tearDown())
when debugging.

Fix pytest-dev#6947
@nicoddemus
Fix test_trial_error in test_unittest
82f584b 
This reverts the test to the state before 04f27d4, which introduced the breaking
change about addCleanup not being called properly for failed tests.
@nicoddemus nicoddemus force-pushed the unittest-cleanup-6947 branch from e244e32 to 82f584b Compare May 2, 2020 18:27
@nicoddemus
Copy link
Member Author

Thanks @RonnyPfannschmidt!

@nicoddemus nicoddemus merged commit 3312820 into pytest-dev:master May 2, 2020
@nicoddemus nicoddemus deleted the unittest-cleanup-6947 branch May 2, 2020 18:43
nicoddemus added a commit to nicoddemus/pytest that referenced this pull request May 2, 2020
@nicoddemus
Merge pull request pytest-dev#7151 from nicoddemus/unittest-cleanup-6947
5ca08e9
@nicoddemus nicoddemus added backported and removed needs backport applied to PRs, indicates that it should be ported to the current bug-fix branch labels May 2, 2020
This was referenced May 2, 2020
Merged
Closed
@perrinjerome perrinjerome mentioned this pull request May 14, 2020
Closed
@sallner sallner mentioned this pull request May 15, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RonnyPfannschmidt RonnyPfannschmidt RonnyPfannschmidt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

unittest.TestCase cleanup functions not invoked on test failure
2 participants
@nicoddemus @RonnyPfannschmidt