Harden #301/#212/#295 regression tests + make AttributeDict generic#326
Harden #301/#212/#295 regression tests + make AttributeDict generic#326wolph wants to merge 5 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces generic typing to AttributeDict and adds several regression tests covering stream redirection, logging deduplication, blank line positioning, and context manager usage. The review feedback suggests improving the subprocess test by propagating PYTHONPATH to ensure reliable imports, and refactoring the manual stream unwrapping loop to use the existing reset_wrapped_streams() helper.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| result = subprocess.run( | ||
| [sys.executable, '-c', child], capture_output=True, text=True | ||
| ) |
There was a problem hiding this comment.
To ensure that the subprocess can reliably import the progressbar package across different test runners and environments (e.g., when the package is not installed globally or in editable mode but is present in sys.path), propagate the current sys.path via the PYTHONPATH environment variable.
| result = subprocess.run( | |
| [sys.executable, '-c', child], capture_output=True, text=True | |
| ) | |
| env = os.environ.copy() | |
| env["PYTHONPATH"] = os.path.pathsep.join(sys.path) | |
| result = subprocess.run( | |
| [sys.executable, "-c", child], | |
| capture_output=True, | |
| text=True, | |
| env=env, | |
| ) |
| for _ in range(5): | ||
| progressbar.streams.unwrap(stderr=True, stdout=True) | ||
| progressbar.streams.unwrap_logging() |
There was a problem hiding this comment.
Instead of manually unwrapping the streams with a hardcoded loop, use the existing helper function reset_wrapped_streams() which is defined at the top of this file. This ensures consistent, robust, and clean reset of all wrapped streams across all tests.
reset_wrapped_streams()![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a142850d8d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
|
|
||
| class AttributeDict(dict): | ||
| class AttributeDict(dict[str, T], typing.Generic[T]): |
There was a problem hiding this comment.
When a caller instantiates the new runtime generic form (AttributeDict[int]()), typing.Generic writes __orig_class__ onto the instance. Because this class routes every __setattr__ into self[name], that metadata becomes a real dictionary entry, so an empty typed AttributeDict has 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 👍 / 👎.
Locks the already-merged fixes for three open issues with integration-level tests, and upgrades
AttributeDictto be generic over its value type.Changes
tests/test_stream.py::test_redirect_stdout_unwrapped_after_keyboard_interrupt: subprocess test provingsys.stdoutis unwrapped after aKeyboardInterruptabandons aredirect_stdoutiteration (process-global, so it needs a real child process).tests/test_with.py::test_context_manager_and_iterable_reporter_widgets_no_duplicate: the reporter's exact widget set + a generator used as both context manager and iterable renders the completed bar exactly once.tests/test_terminal.py::test_redirect_blank_line_position: asserts the separating blank line sits between the clear sequence and the bar redraw (not merely "somewhere").progressbar/utils.py:dict→dict[str, T], Generic[T], so homogeneous typed use (AttributeDict[int]) flows the real value type through__getattr__/__setattr__and catches mis-typed assignment (pyright-verified).tests/test_utils.py::test_attribute_dict_generic_value_type.tests/test_stream.py::test_wrap_logging_deduplicates_shared_handler: covers the shared-handler dedupcontinueinwrap_logging, restoring the 100% gate.Verification (clean checkout)
pyright progressbar: 0 errors.ruff check: clean.