Ensure logging tests always cleanup after themselves #11531
Merged
Conversation
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
nicoddemus
added
the
needs backport
|
2nd commits adds type annotations for all tests in |
bluetech
approved these changes
Oct 23, 2023
testing/logging/test_fixture.py
Outdated
| @@ -290,25 +294,47 @@ def test_caplog_captures_for_all_stages(caplog, logging_during_setup_and_teardow | |||
| assert [x.message for x in caplog.get_records("setup")] == ["a_setup_log"] | |||
|
|
|||
| # This reaches into private API, don't use this type of thing in real tests! | |||
| assert set(caplog._item.stash[caplog_records_key]) == {"setup", "call"} | |||
| # error: Invalid index type "StashKey[dict[str, list[LogRecord]]]" for "Stash"; | |||
There was a problem hiding this comment.
I tried playing a bit with the StashKey's TypeVar variance to fix this, but I think it needs to be invariant because it's used in both setters and getters. Maybe the new Python 3.12 type parameter syntax fixes this, didn't check.
Anyway I think it can be worked around by doing it in two lines:
caplog_records = caplog._item.stash[caplog_records_key]
assert set(caplog_records) == {"setup", "call"}There was a problem hiding this comment.
Indeed that fixes it, however I do not get why... could you explain it, please?
Logging has many global states, and we did foresee this by creating a ``cleanup_disabled_logging`` fixture, however one might still forget to use it and failures leak later -- sometimes not even in the same PR, because the order of the tests might change in the future, specially when running under xdist. This problem surfaced during pytest-dev#11530, where tests unrelated to the change started to fail.
nicoddemus
force-pushed
the
ensure-logging-cleanup
branch
from
29fe9c4 to
7e69ce7
Compare
nicoddemus
added
backport 7.4.x
and removed
needs backport
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Logging has many global states, and we did foresee this by creating a
cleanup_disabled_loggingfixture, however one might still forget to use it and failures leak later -- sometimes not even in the same PR, because the order of the tests might change in the future, specially when running under xdist.This problem surfaced during #11530, where tests unrelated to the change started to fail.