gh-112334: Regression test that vfork is used when expected. #112734
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
gpshead
added
tests
gpshead
added
the
🔨 test-with-buildbots
This comment was marked as outdated.
This comment was marked as outdated.
bedevere-bot
removed
the
🔨 test-with-buildbots
This way they only get run once instead of twice via NoPoll.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
gpshead
added
the
🔨 test-with-buildbots
This comment was marked as outdated.
This comment was marked as outdated.
bedevere-bot
removed
the
🔨 test-with-buildbots
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
gpshead
added
the
🔨 test-with-buildbots
bedevere-bot
removed
the
🔨 test-with-buildbots
This makes output easier to understand if debugging a failure. (removes execve which shows up for the initial process launch by strace on _some_ platforms, adds clone2 which claims to only be used on an obsolete architecture but if it cropped up again for some future reason would be annoying to have break the test)
aisk
pushed a commit
to aisk/cpython
that referenced
this pull request
Feb 11, 2024
…ython#112734) Regression test that vfork is used when expected by subprocess. This is written integration test style, it uses strace if it is present and appears to work to find out what system call actually gets used in different scenarios. Test coverage is added for the default behavior and that of each of the specific arguments that must disable the use of vfork. obviously not an entire test matrix, but it covers the most important aspects. If there are ever issues with this test being flaky or failing on new platforms, rather than try and adapt it for all possible platforms, feel free to narrow the range it gets tested on when appropriate. That is not likely to reduce coverage.
Glyphack
pushed a commit
to Glyphack/cpython
that referenced
this pull request
Sep 2, 2024
…ython#112734) Regression test that vfork is used when expected by subprocess. This is written integration test style, it uses strace if it is present and appears to work to find out what system call actually gets used in different scenarios. Test coverage is added for the default behavior and that of each of the specific arguments that must disable the use of vfork. obviously not an entire test matrix, but it covers the most important aspects. If there are ever issues with this test being flaky or failing on new platforms, rather than try and adapt it for all possible platforms, feel free to narrow the range it gets tested on when appropriate. That is not likely to reduce coverage.
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.
Adds a regression test #112334. This is the regression test I was pondering in #112617.
This also ensures strace is present in our CI GH (It may have already been there, ubuntu-standard includes it, now we're explicit about the need). I also made sure to install it on some Linux buildbots that I maintain. From my testing, some clearly do not have it.
If this test ever proves a challenge to maintain, further restricting it to a specific platform is fine. The way it works depends on details that can vary between different Linux platforms. ex: aarch64 uses more modern syscalls than x86_64 for the same task.