Skip to content
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 build log parser failed test list #2077

Merged
merged 4 commits into from
Jun 14, 2021

Conversation

hakonsbm
Copy link
Contributor

Resolves #1945 by updating the summarize_tests.py script for junitparser version 2.0. With version 2.0 of junitparser case.result is changed to a list. This caused skipped tests to not be detected as skipped, and therefore added to the list of failing tests which is printed if any other tests fail.

@hakonsbm hakonsbm added T: Bug Wrong statements in the code or documentation S: Normal Handle this with default priority I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) labels Jun 14, 2021
Copy link
Contributor

@heplesser heplesser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hakonsbm Thanks for this! A few questions:

  • Should we add a version requirement to junitparser in the right places?
  • Have you tested this by breaking a few tests?
  • Are we sure we will only ever need element [0] from the list?

@hakonsbm
Copy link
Contributor Author

@heplesser

Should we add a version requirement to junitparser in the right places?

Done!

Have you tested this by breaking a few tests?

Yes, to find out what caused this I had to break a few eggs tests. See for example this output with a failing test.

Are we sure we will only ever need element [0] from the list?

Right now it seems like each testcase always give at most one result and skipped tests give exactly one result.

Copy link
Contributor

@heplesser heplesser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one suggestion for an additional protection against additional result elements.

@heplesser
Copy link
Contributor

Minimal but important fix to test reporting, therefore merging based on single review.

@heplesser heplesser merged commit 8dc5500 into nest:master Jun 14, 2021
@hakonsbm hakonsbm deleted the testsuite_parser_table branch August 1, 2022 07:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) S: Normal Handle this with default priority T: Bug Wrong statements in the code or documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improved robustness of build log parser
2 participants