gh-127747: Resolve BytesWarning in test.support.strace_helper #127849
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
The strace_helper code has a _make_error function to simplify making StraceResult objects in error cases. That takes a details parameter which is either a caught OSError or `bytes`. If it's bytes, _make_error would implicitly coerce that to a str inside of a f-string, resulting in a BytesWarning. It's useful to see if it's an OSError or bytes when debugging, resolve by changing to format with repr(). This is an error message on an internal helper.
A non-zero exit code occurs if the strace binary isn't found, and no events will be parsed in that case (there is no output). Handle that case by checking exit code before checking for events. Still asserting around events rather than returning false, so that hopefully if there's some change to `strace` that breaks the parsing, will see that as a test failure rather than silently loosing strace tests because they are auto-disabled.
srinivasreddy
pushed a commit
to srinivasreddy/cpython
that referenced
this pull request
Jan 8, 2025
…ython#127849) The strace_helper code has a _make_error function to simplify making StraceResult objects in error cases. That takes a details parameter which is either a caught OSError or `bytes`. If it's bytes, _make_error would implicitly coerce that to a str inside of a f-string, resulting in a BytesWarning. It's useful to see if it's an OSError or bytes when debugging, resolve by changing to format with repr(). This is an error message on an internal helper. A non-zero exit code occurs if the strace binary isn't found, and no events will be parsed in that case (there is no output). Handle that case by checking exit code before checking for events. Still asserting around events rather than returning false, so that hopefully if there's some change to `strace` that breaks the parsing, will see that as a test failure rather than silently loosing strace tests because they are auto-disabled.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
Validation Steps:
Lib/test/support/strace_helper.pyto set_strace_binaryto/usr/bin/false(or don't have strace installed)./python -Werror -I -bb -m test.test_subprocess -vv POSIXProcessTestCase.test_vfork_used_when_expectedDescription
The strace_helper code has a _make_error function to simplify making StraceResult objects in error cases. That takes a details parameter which is either a caught OSError or bytes. If it's bytes, _make_error would implicitly coerce that to a str inside of a f-string, resulting in a BytesWarning when the python test is invoked with -b.
It is useful to distinguish between an OSError or bytes when debugging the test, so eliminate the BytesWarning and show both by changing to format with repr().
After fixing the implicing bytes-> str conversion, if
/usr/bin/straceis uninstalled/doesn't exist, get a new error running the test:Resolved by checking the exit code of the subprocess before asserting there should be events. Reason for asserting there are events, rather than returning False from
_can_strace, is to try and ensure that if strace runs successfully and produces an output that can't be parsed that is a test error rather than silently disabling all strace tests (See: gh-121381 where a feature working was tested with strace but the strace tests stopped running, leading to this infrastructure in #123413).skip news: This is a fix to an error message on a python test suite internal helper.
BytesWarningwhentest.support.strace_helper.strace_pythonfails #127747