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

Update summarise-ct-results #5

Merged
merged 7 commits into from
Aug 28, 2024

Conversation

arcusfelis
Copy link
Collaborator

After analyzing history, it looks like the script also belongs to the repo and could be tested with tests.

@arcusfelis arcusfelis marked this pull request as ready for review August 26, 2024 13:04
@arcusfelis arcusfelis changed the title Summarise ct results with end per testcase failed Update summarise-ct-results Aug 27, 2024
Copy link
Member

@chrzaszcz chrzaszcz left a comment

Choose a reason for hiding this comment

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

It looks good in general. I added some comments. I am not completely sure how it fits into MIM - does it require any change in the MIM repo?

"No test groups were executed successfully",
"Failing the test due to failed rerun groups",
"Failing the test due to failed test cases",
"CODE=53"] = summarise_results(),
Copy link
Member

Choose a reason for hiding this comment

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

Why is the code 53 even though CT results above show lower numbers?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh, it is erlang:halt(100405), but error code overflows.

1> erlang:halt(100405).
exits
echo $?
53

" 0 tests eventually passed",
" 0 tests with end_per_testcase failed",
" 6 tests passed",
" 3 tests failed"," 0 tests skipped by user",
Copy link
Member

Choose a reason for hiding this comment

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

Add line break

" 3 tests eventually passed",
" 0 tests with end_per_testcase failed",
" 9 tests passed",
" 3 tests failed"," 0 tests skipped by user",
Copy link
Member

Choose a reason for hiding this comment

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

Add line break

@arcusfelis
Copy link
Collaborator Author

It looks good in general. I added some comments. I am not completely sure how it fits into MIM - does it require any change in the MIM repo?

MIM repo would have to execute big_tests/_build/test/lib/ct_groups_summary_hook/priv/summarise-ct-results instead of tools/summarise-ct-results

@arcusfelis arcusfelis force-pushed the summarise-ct-results-with-end-per-testcase-failed branch from d694eb1 to 8e39acd Compare August 27, 2024 16:36
Copy link
Member

@chrzaszcz chrzaszcz left a comment

Choose a reason for hiding this comment

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

Looks good. I am not a fan of such reformatting of a whole repo, but since the history is lost anyway, we can do it.

@chrzaszcz chrzaszcz merged commit 619ad53 into main Aug 28, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants