-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Adjust repro file message for merged test #102141
Adjust repro file message for merged test #102141
Conversation
For now the script ./src/tests/run.py outputs "Failed to create repro for test" for a failed merged test. But this comment 1288 if assembly_is_merged_tests_run: 1289 # REVIEW: Even if the test is a .dll file or .CMD file and is found, we don't know how to 1290 # build a repro case with it. from ./src/test/run.py script says that for a merged test a repro file should not be created at all. Changing the message "Failed to create repro for test" to "Skipping repro for merged test".
Tagging subscribers to this area: @hoyosjs |
@BruceForstall Could you review this PR? Thank you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@BruceForstall @jkotas Could you take a look? |
RISC-V test results for qemu-prio1-checked: 450 / 1045 (43.06%)detailsGIT: 5a6b372
failed tests
killed tests
skipped tests
|
RISC-V test results for starfive-prio1-checked: 450 / 1045 (43.06%)detailsGIT: 5a6b372
failed tests
killed tests
skipped tests
|
@BruceForstall @jkotas It adds an error message. I think it looks better. What do you think? Could you review and give any advice about error message string and variable name? Thank you. |
I am sorry I do not have a context on this script. |
Adjust message about repro file for a merged test
For now the script
./src/tests/run.py
outputs "Failed to create repro for test" for a failed merged test.But this comment
from
./src/test/run.py
script says that for a merged test a repro file should not be created at all.Changing the message "Failed to create repro for test" to "Skipping repro for merged test".
Part of #84834, cc @dotnet/samsung