-
-
Notifications
You must be signed in to change notification settings - Fork 521
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
implement pseudo tty on stdout/stderr #2711
Merged
Merged
Conversation
This file contains 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
during SyncWrite.handler (post-lock): * cancel the timer thread * if a newline was found, _write up to the newline * unconditionally start the timer to sweep up any remaining output after some delay This facilitates interactive use of tox commands.
The purpose of allocating a pty is to allow better interactive use and enable commands to detect that they are running interactively. Based on starter code on issue tox-dev#1773. Fixes issue tox-dev#1773.
gaborbernat
approved these changes
Dec 16, 2022
descope bot
referenced
this pull request
in descope/django-descope
Dec 30, 2022
This PR contains the following updates: | Package | Type | Update | Change | Pending | |---|---|---|---|---| | [tox](https://github.com/tox-dev/tox) | dev | patch | `4.0.11` -> `4.0.12` | `4.1.1` (+8) | --- ### Release Notes <details> <summary>tox-dev/tox</summary> ### [`v4.0.12`](https://github.com/tox-dev/tox/releases/tag/4.0.12) [Compare Source](https://github.com/tox-dev/tox/compare/4.0.11...4.0.12) #### What's Changed - Fix legacy flags, deprecate them and remove --index-url by [@​gaborbernat](https://github.com/gaborbernat) in [https://github.com/tox-dev/tox/pull/2731](https://github.com/tox-dev/tox/pull/2731) - syntax fix for tox 4 - CLI args section by [@​evgeni](https://github.com/evgeni) in [https://github.com/tox-dev/tox/pull/2734](https://github.com/tox-dev/tox/pull/2734) - implement pseudo tty on stdout/stderr by [@​masenf](https://github.com/masenf) in [https://github.com/tox-dev/tox/pull/2711](https://github.com/tox-dev/tox/pull/2711) - Document user level config by [@​ziima](https://github.com/ziima) in [https://github.com/tox-dev/tox/pull/2736](https://github.com/tox-dev/tox/pull/2736) - Fix python hash seed not being set by [@​gaborbernat](https://github.com/gaborbernat) in [https://github.com/tox-dev/tox/pull/2739](https://github.com/tox-dev/tox/pull/2739) #### New Contributors - [@​evgeni](https://github.com/evgeni) made their first contribution in [https://github.com/tox-dev/tox/pull/2734](https://github.com/tox-dev/tox/pull/2734) **Full Changelog**: tox-dev/tox@4.0.11...4.0.12 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC43NC4wIiwidXBkYXRlZEluVmVyIjoiMzQuNzQuMCJ9--> Co-authored-by: descope[bot] <descope[bot]@users.noreply.github.com>
This was referenced Jan 4, 2023
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.
Thanks for contribution
Please, make sure you address all the checklists (for details on how see
development documentation)!
tox -e fix
)docs/changelog
folderpty
This is my first attempt at fixing #1773, starting with the example code posted on that issue, and adapting it for some changes that have occured since then.
For the most part the code worked really nice already.
I did encounter the one problem mentioned after setting the Popen
stdout
andstderr
handles to pty:After lots of poking around, I determine that the code in
SyncWrite
, which is supposed to write the content to the target after some minimum delay, was not restarting its timer appropriately, causing the output to "hang" untilSyncWrite
discovered a newline\n
.With that fix in 9b06df5, everything seems to be working fine, EXCEPT, the log files now have ANSI escape sequences in them... which may or may not be desirable. After all, that is what the program output.
After this patch, colored output and interactive use (
pdb
,ipython
) appears to be working out of the box!Testing
Here's my "human" test file with 4 cases, to exemplify the differences between tox 3, tox 4, and this patch.
I've made these sort of non-scientific comparisons on a macOS 10.15 machine.
tox 3 ("current behavior" expected by users)
py
pytest
F
, and redFailed:
texttextual
vim
: fully interactive, working as expectedtox 3 (Using
tee
to pipe the output to a log file)py
<enter>
)pytest
textual
vim
: fully interactive, working as expected (also surprising)tox 4 ("new behavior" shows some regression in expected functionality)
py
<enter>
shows previous output)pytest
textual
vim
gt
andgT
can sometimes trigger a refresh).tox 4 (Using
tee
to pipe the output to a log file)py
: no change usingtee
pytest
: no change usingtee
textual
: behaves like tox 3 withtee
vim
: no change usingtee
tox 4 with this patch!
py
pytest
F
, and redFailed:
texttextual
vim
: fully interactive, working as expectedtox 4 with this patch! (Using
tee
to pipe the output to a log file)py
SyncWrite
fix at work)pytest
textual
: behaves like tox 3 withtee
vim
: fully interactive, working as expected (again, not sure how, but i'll take it)TODO
Failing test to investigateturned out to be a macOS issue with homebrew-installed python interpreters. The test was expecting the executable to be/usr/local/opt/python@3.11/bin/python3.11
, but when it runs throught the virtualenv, the path name comes out as/usr/local/Cellar/python@3.11/3.11.0/Frameworks/Python.framework/Versions/3.11/bin/python3.11
... These are both hardlinks to the same inode. Not reproducing on linux.Additional test coverage for this featureadded to existingtest_local_subprocess_tty
Testing on other platforms: