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.
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
fix finding of failed tests in output of PyTorch test step #2859
base: develop
Are you sure you want to change the base?
fix finding of failed tests in output of PyTorch test step #2859
Changes from all commits
0f6a9b1
7187b15
63abd5d
b296c04
b8770f7
eaf4056
be911fc
4952598
8e0ccbf
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Nit: To avoid long distracting lines like
sorted(set(failed_tests_info.failed_test_suites))
(2 times "failed") maybe either unwrap the named tuple here:failure_report, failed_test_suites = failed_tests_info.failure_report, failed_tests_info.failed_test_suites
etc. or rename totest_info
astest_info.failed_test_suites
is easier to read.And maybe the
test_cnt
extraction can be done in there too which would fit the name even better (then drop the "failed" from the function name).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.
This or the SIGSEGV test needs to be another one or
failed_test_suites
could pass when the signal was not detected as in the case of a forced termination due to a signla there won't be a test summary, will there?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.
Looks like we have a bug here. For the output of
it must NOT match the RegExp. The original, correct output is:
So the RegExp must be made to only add the counting of "FAILED" if the next line is the failed test, see https://gist.github.com/Flamefire/dc1403ccefdebfc3412c6fbb2d5cbabd#file-pytorch-1-9-0-foss-2020b_partial-log-L480
I would actually add this wrong output as a test against regressions. With that snipped it should only find
test_fx
as failed (with an unknown number of tests)