Skip to content

Store summary of previous runs when deflaking #360

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Oct 13, 2022
Merged

Store summary of previous runs when deflaking #360

merged 6 commits into from
Oct 13, 2022

Conversation

@imcdo
Copy link
Member

@imcdo imcdo commented Aug 23, 2022 •
edited
Loading

Motivation:
when getting a flaky test, the summaries are clear due to the last run passing, loosing all data of failed runs. This PR intends to remedy this by changing the summary behavior of deflake runs. In a deflake run, all summaries are included, and demarked by the run number and a ~~~~~:

image

Testing:
ran unit tests and ran all system tests

@imcdo imcdo requested a review from a team August 23, 2022 20:18
nagpavan-chilakam
nagpavan-chilakam approved these changes Aug 24, 2022
View reviewed changes
Copy link

@nagpavan-chilakam nagpavan-chilakam left a comment

Choose a reason for hiding this comment

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

Just a thought, if the error summary is same for all the runs, it would be good if we can just display that 1 error and number of times it failed with the same reason. If a test failed all 5 times with same reason, displaying the same error summary 5 times may be cumbersome to read, especially if we are running a bunch of tests.
Otherwise looks good

@imcdo
Copy link
Member Author

imcdo commented Aug 24, 2022

Just a thought, if the error summary is same for all the runs, it would be good if we can just display that 1 error and number of times it failed with the same reason. If a test failed all 5 times with same reason, displaying the same error summary 5 times may be cumbersome to read, especially if we are running a bunch of tests. Otherwise looks good

This is a good idea but Im not sure a good way to display this,
would something like this work:

summary:
run 1, 3, 5 summary:
....
run 2, 4 summary:
...
run 6: PASSED

? Thoughts @nagpavan-chilakam @stan-confluent

@stan-is-hate
Copy link
Member

stan-is-hate commented Aug 25, 2022

Looks good overall, thanks Ian!

  • your way of printing similar results makes sense (aggregate them using a dict)
  • worth adding a unit test

@imcdo
Copy link
Member Author

imcdo commented Oct 12, 2022

example fail case:
image

example flaky case:
image

@imcdo imcdo merged commit 2048d79 into confluentinc:master Oct 13, 2022
gousteris pushed a commit to gousteris/ducktape that referenced this pull request Aug 30, 2023
@imcdo @gousteris
Store summary of previous runs when deflaking (confluentinc#360)
6a8149e 
* Store summary of previous runs when deflaking

* combine same stack traces

* update comments
gousteris pushed a commit to gousteris/ducktape that referenced this pull request Aug 30, 2023
@imcdo @gousteris
Store summary of previous runs when deflaking (confluentinc#360)
fdbcc09 
* Store summary of previous runs when deflaking

* combine same stack traces

* update comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@stan-is-hate stan-is-hate stan-is-hate approved these changes

@nagpavan-chilakam nagpavan-chilakam nagpavan-chilakam approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@imcdo @stan-is-hate @nagpavan-chilakam