-
-
Notifications
You must be signed in to change notification settings - Fork 105
Harden #301/#212/#295 regression tests + make AttributeDict generic #326
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
base: develop
Are you sure you want to change the base?
Changes from all commits
4f94b10
44970be
277eb29
469c4b1
a142850
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,7 +1,9 @@ | ||||||||||||||||||||||||
| import io | ||||||||||||||||||||||||
| import logging | ||||||||||||||||||||||||
| import os | ||||||||||||||||||||||||
| import subprocess | ||||||||||||||||||||||||
| import sys | ||||||||||||||||||||||||
| import textwrap | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| import pytest | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
|
|
@@ -485,3 +487,69 @@ def flush(self) -> None: | |||||||||||||||||||||||
| written = wrapper.write('hello\n') | ||||||||||||||||||||||||
| assert written == 6 | ||||||||||||||||||||||||
| assert target.flushes >= 1 | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| def test_redirect_stdout_unwrapped_after_keyboard_interrupt() -> None: | ||||||||||||||||||||||||
| # #212: when redirect_stdout wraps sys.stdout and iteration is abandoned | ||||||||||||||||||||||||
| # by a KeyboardInterrupt, the bar must unwrap stdout again. Runs in a | ||||||||||||||||||||||||
| # subprocess because stream wrapping mutates process-global sys.stdout. | ||||||||||||||||||||||||
| child = textwrap.dedent( | ||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||
| import sys | ||||||||||||||||||||||||
| import progressbar | ||||||||||||||||||||||||
| from progressbar.utils import WrappingIO | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||
| for i in progressbar.progressbar(range(100), redirect_stdout=True): | ||||||||||||||||||||||||
| print('text', i) | ||||||||||||||||||||||||
| if i == 50: | ||||||||||||||||||||||||
| raise KeyboardInterrupt | ||||||||||||||||||||||||
| except KeyboardInterrupt: | ||||||||||||||||||||||||
| pass | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| sys.exit(2 if isinstance(sys.stdout, WrappingIO) else 0) | ||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||
| result = subprocess.run( | ||||||||||||||||||||||||
| [sys.executable, '-c', child], capture_output=True, text=True | ||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||
|
Comment on lines
+513
to
+515
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To ensure that the subprocess can reliably import the
Suggested change
|
||||||||||||||||||||||||
| assert result.returncode == 0, ( | ||||||||||||||||||||||||
| f'stdout left wrapped after interrupt; rc={result.returncode}\n' | ||||||||||||||||||||||||
| f'stderr={result.stderr[-500:]}' | ||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| def test_wrap_logging_deduplicates_shared_handler(monkeypatch) -> None: | ||||||||||||||||||||||||
| # A handler attached to more than one logger must be wrapped only once. | ||||||||||||||||||||||||
| # _iter_loggers yields the root logger then named loggers, so a handler | ||||||||||||||||||||||||
| # shared by both is seen twice and the second visit is skipped. | ||||||||||||||||||||||||
| for _ in range(5): | ||||||||||||||||||||||||
| progressbar.streams.unwrap(stderr=True, stdout=True) | ||||||||||||||||||||||||
| progressbar.streams.unwrap_logging() | ||||||||||||||||||||||||
|
Comment on lines
+526
to
+528
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| stream = io.StringIO() | ||||||||||||||||||||||||
| monkeypatch.setattr(sys, 'stderr', stream) | ||||||||||||||||||||||||
| monkeypatch.setattr(progressbar.streams, 'original_stderr', stream) | ||||||||||||||||||||||||
| monkeypatch.setattr(progressbar.streams, 'stderr', stream) | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| root = logging.getLogger() | ||||||||||||||||||||||||
| named = logging.getLogger('progressbar-test-shared-handler') | ||||||||||||||||||||||||
| named.handlers = [] | ||||||||||||||||||||||||
| named.propagate = False | ||||||||||||||||||||||||
| handler = logging.StreamHandler(sys.stderr) | ||||||||||||||||||||||||
| named.addHandler(handler) | ||||||||||||||||||||||||
| root.addHandler(handler) # same handler object on two loggers | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| progressbar.streams.wrap_stderr() | ||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||
| progressbar.streams.wrap_logging() | ||||||||||||||||||||||||
| # Recorded exactly once despite being reachable via two loggers. | ||||||||||||||||||||||||
| shared = [ | ||||||||||||||||||||||||
| h for h, _ in progressbar.streams.logging_handlers if h is handler | ||||||||||||||||||||||||
| ] | ||||||||||||||||||||||||
| assert len(shared) == 1 | ||||||||||||||||||||||||
| finally: | ||||||||||||||||||||||||
| progressbar.streams.unwrap_logging() | ||||||||||||||||||||||||
| progressbar.streams.unwrap(stderr=True) | ||||||||||||||||||||||||
| root.removeHandler(handler) | ||||||||||||||||||||||||
| named.handlers = [] | ||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a caller instantiates the new runtime generic form (
AttributeDict[int]()),typing.Genericwrites__orig_class__onto the instance. Because this class routes every__setattr__intoself[name], that metadata becomes a real dictionary entry, so an empty typedAttributeDicthas length 1 and iterates/serializes with a spurious__orig_class__key. The added test only annotates an unparameterized constructor, so it misses the advertised usage; special-case this internal attribute or avoid storing all real attributes as dict items.Useful? React with 👍 / 👎.