Fix tests failing in GitHub Actions #420
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
lukpueh
reviewed
Jul 18, 2022
Test `test_run_duplicate_streams_timeout` fails sometimes, and after some investigations it is found that the process used in the test exits before considered for a Timeout. To fix the test, the command is now replaced with another command. Signed-off-by: Pradyumna Krishna <git@onpy.in>
PradyumnaKrishna
force-pushed
the
fix_process_timeout
branch
from
ef112e5
to
1b697fc
Compare
lukpueh
reviewed
Jul 18, 2022
There was a problem hiding this comment.
Thanks for the fix! What do you think should be the expected behavior for timeout <= 0? It seems surprising that a subprocess can successfully run with timeout=-1, just because it finishes before we check for timeout. Would you mind taking a look how the subprocess module handles this?
|
3 tasks
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.
Description of the changes being introduced by the pull request:
Test
test_run_duplicate_streams_timeoutfails sometimes, and aftersome investigations it is found that the process used in the test
exits before considered for a Timeout.
To fix the test, the command is now replaced with another command.
Also, I consider testing this Pull Request multiple time using GitHub Actions
before merging to make sure the problem is now solved.
A failed test: https://github.com/secure-systems-lab/securesystemslib/runs/7280432273
Please verify and check that the pull request fulfils the following requirements: