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

unittest: fix the _GetOutOf_testPartExecutor exception kind. Fixes … #6972

Closed
wants to merge 1 commit into from
Closed

unittest: fix the _GetOutOf_testPartExecutor exception kind. Fixes … #6972

wants to merge 1 commit into from

Conversation

qbey
Copy link

@qbey qbey commented Mar 26, 2020

#6947

_GetOutOf_testPartExecutor was introduced in
04f27d4
and allow to catch Exception from a unittest TestCase, but as it
inheritate from KeyboardInterrupt it is managed differently by
unittest (see.
https://github.com/python/cpython/blob/032de7324e30c6b44ef272cea3be205a3d768759/Lib/unittest/case.py#L61-L62)
as the testPartExecutor reraise it directly. This prevent unittest to
run the cleanup functions properly.

Since this change enable the teardown methods to be ran properly bu
unittest, the explicit teardown is not required anymore.

Checklist:

  • Include documentation when adding new features.
  • Include new tests or update existing tests when applicable.
  • Allow maintainers to push and squash when merging my commits. Please uncheck this if you prefer to squash the commits yourself.
  • Create a new changelog file in the changelog folder, with a name like <ISSUE NUMBER>.<TYPE>.rst. See changelog/README.rst for details.
  • Add yourself to AUTHORS in alphabetical order.

…6947

`_GetOutOf_testPartExecutor` was introduced in
04f27d4
and allow to catch Exception from a unittest TestCase, but as it
inheritate from `KeyboardInterrupt` it is managed differently by
unittest (see.
https://github.com/python/cpython/blob/032de7324e30c6b44ef272cea3be205a3d768759/Lib/unittest/case.py#L61-L62)
as the `testPartExecutor` reraise it directly. This prevent unittest to
run the cleanup functions properly.

Since this change enable the teardown methods to be ran properly by
unittest, the explicit teardown is not required anymore.
@blueyed
Copy link
Contributor

blueyed commented Mar 26, 2020

The point of _GetOutOf_testPartExecutor is to get out of there, using KeyboardInterrupt.

I still think that it would be better to revert all that stuff, and require to use pdb.set_trace() for now, FWIW.

See also #6924 #6947.

@qbey
Copy link
Author

qbey commented Mar 27, 2020

Thank you Daniel.
Alright, this fix does not preserve the --pdb so it can't be accepted. I close it.
Anyway, the fixup for the cleanup part can't be solved without also considering the async issue, so as said it might be better to revert the original commit for now.

@qbey qbey closed this Mar 27, 2020
@qbey qbey deleted the qbey/fix_issue_6947 branch March 27, 2020 09:46
@qbey
Copy link
Author

qbey commented Mar 27, 2020

@blueyed You may still want to add the tests (about cleanup and teardown counter), test_cleanup_called_issue6947 fails with pytest 5.4.1 and should not fail with version <5.4.0. So feel free to copy/paste/rewrite them if you rollback your commit :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants