fix: handle file stdout/stderr #186
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
Allows tests to write to the standard input of a mocked process
Owner
|
@DMedina559 thank you for your contribution, but please fix the tests. |
- ignore `PytestDeprecationWarning` in `pytest.ini`. - suppress `docutils` errors in `tests/test_examples.py` by setting `report_level` to 5.
test: fix failing test
* test: convert to lowercase in whoami test * test: use a more cross platform command or else you'd get FileNotFoundError errors
for more information, see https://pre-commit.ci
Contributor
Author
|
Ok, I've gotten all 143 test passing on Windows (py3.13) and all 140 test passing on Linux (py3.12) |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes several issues with subprocess handling and improves cross-platform compatibility. The main purpose is to resolve a bug where stdout/stderr redirection to files caused TypeErrors and to enhance stdin handling with subprocess.PIPE.
- Fixed TypeError when stdout/stderr is redirected to files and streams are not registered
- Added support for stdin=subprocess.PIPE to provide a writable buffer
- Improved cross-platform compatibility in tests and documentation
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/test_subprocess.py | Added tests for stdin pipe handling and stdout/stderr file redirection fixes, plus Windows compatibility improvements |
| tests/test_examples.py | Suppressed docutils error reporting by setting report_level to 5 |
| pytest_subprocess/fake_popen.py | Implemented stdin pipe support and fixed None data handling in buffer writes |
| pytest.ini | Added asyncio configuration and ignored pytest deprecation warnings |
| README.rst | Updated examples to use more cross-platform commands instead of Unix-specific 'ls' |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
* temp: test on push * fix: increase timeout for mac test * fix: ignore warning in universal newlines test * fix: add another filterwarning tag * remove: temp on push test
Contributor
Author
|
All test on all platforms now pass |
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.
stdin=subprocess.PIPE.vs/to.gitignorefile for visual studioPytestDeprecationWarninginpytest.ini.docutilserrors intests/test_examples.pyby settingreport_levelto 5.whoamitest and using a more cross-platform generic command in README.rst