Exclude test_suite() from pytest collection (fixes #481) - #505
Open
agu2347 wants to merge 2 commits into
Open
Conversation
tests/test_regression.py defines a module-level test_suite() function
returning a doctest.DocTestSuite, following the classic unittest/
zope.testrunner convention for external test discovery. Because its
name matches pytest's default python_functions pattern ("test_*"),
pytest also collects and runs it directly as an ordinary test, then
emits PytestReturnNotNoneWarning since it returns a DocTestSuite
instead of None -- a warning slated to become a hard error in a
future pytest version.
Set test_suite.__test__ = False, the standard pytest idiom for
excluding a specifically named function from collection, so pytest
leaves it alone while it remains available for any external runner
that still calls test_suite() by the classic convention.
Added a regression test asserting the attribute stays set.
Member
|
It shouldn't be excluded, not really. It really needs to be converted from doctests to regular unit tests. |
… on Pylons#505) Per @kgaughan on Pylons#505: excluding test_suite() from pytest collection papered over the underlying issue rather than fixing it. The doctest it wrapped was already dead code -- it used Python 2-only syntax (tuple-unpacking `def bind(self, (ip, port))`, print statements) that cannot execute under Python 3 at all, and referenced a server/channel API (`channel.next_channel_cleanup[0]`, `HTTPChannel.connected` flipping synchronously) that predates the current will_close/maintenance() design. Removed test_suite()/doctest.DocTestSuite() and the now-unrunnable docstring doctest entirely, and replaced them with real pytest functions against the current API, covering the same regression the doctest was meant to guard: - a channel idle past channel_timeout gets marked will_close by server.maintenance() - a channel with recent write activity (via handle_write()) does not - a channel with recent read activity (via handle_read()) does not - a channel with requests in flight is left alone regardless of last_activity (the original doctest's "main loop window" case) Verified these are not vacuous: temporarily removed the `self.last_activity = time.time()` updates from HTTPChannel's handle_read()/handle_write() (reintroducing the original bug) and confirmed the two activity tests fail while the other two still pass, then restored the fix and reconfirmed all four pass. Ran the full suite before and after: 798 passed / 8 skipped, no failures, and the PytestReturnNotNoneWarning from Pylons#481 is gone (there is no longer a test_suite() for pytest to collect and warn about).
Author
|
You're right, that was the better fix. Removed the exclusion hack and the dead Python 2-only doctest entirely (it couldn't run under Python 3 at all -- tuple-unpacking bind() params, print statements), and replaced it with real pytest functions covering the same regression against the current API: a channel idle past channel_timeout gets marked will_close, one with recent read or write activity doesn't, and one with requests in flight is left alone. Verified they're not vacuous by temporarily reintroducing the original bug (removing the last_activity updates from handle_read/handle_write) and confirming the activity tests fail. Full suite: 798 passed, 8 skipped. Ready for re-review. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #481.
tests/test_regression.pydefines a module-leveltest_suite()function that returns adoctest.DocTestSuite, following the classic unittest/zope.testrunner convention for external test discovery. Its name matches pytest's defaultpython_functionspattern (test_*), so pytest also collects and runs it directly as an ordinary test, then emits:This fix sets
test_suite.__test__ = False, the standard pytest idiom for excluding a specifically named function from collection.test_suite()remains available and unchanged for any external runner that still calls it by the classic module-level convention -- only pytest's own collection of it as a test is suppressed.Testing
test_test_suite_excluded_from_pytest_collection, assertingtest_suite.__test__ is False.main(pytest tests/test_regression.py -W always), confirmed it disappears with this change, and confirmed the new test fails withAttributeErrorif the fix is reverted (with the test kept).rufflint failures in both runs, with the only change being one fewer warning (thePytestReturnNotNoneWarning) in the after run.