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.

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

workflows: Retry failed test by default in kickstart test GH workflow #5941

Merged
merged 1 commit into from
Oct 17, 2024
Merged

workflows: Retry failed test by default in kickstart test GH workflow #5941

merged 1 commit into from
Oct 17, 2024

Conversation

rvykydal
Copy link
Contributor

More and more often we are running the whole kickstart test suite (or big set of tests) on a PR, for example:

/kickstart-test --testtype network

/kickstart-test --skip-testtypes none

There is quite a big chance of hitting a flake, so retry failing tests by default so there is no need to check if the failures are flakes or re-run the failed tests manually.

@github-actions github-actions bot added infrastructure Changes affecting mostly infrastructure f42 Fedora 42 labels Oct 16, 2024
@rvykydal rvykydal force-pushed the kickstart-test-workflow-retry-by-default branch from b40344a to ff2871c Compare October 16, 2024 09:22
@rvykydal rvykydal mentioned this pull request Oct 16, 2024
Merged
M4rtinK
M4rtinK approved these changes Oct 16, 2024
View reviewed changes
Copy link
Contributor

@M4rtinK M4rtinK 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 to me & a really good idea, thanks! :)

@rvykydal
infra: Retry failed test by default in kickstart test GH workflow
b597a48 
More and more often we are running the whole kickstart test suite (or
big set of tests) on a PR, for example:

/kickstart-test --testtype network

/kickstart-test --skip-testtypes none

There is quite a big chance of hitting a flake, so retry failing tests
by default so there is no need to check if the failures are flakes or
re-run the failed tests manually.

The setting corresponds to the '--retry' launcher option.
@rvykydal rvykydal force-pushed the kickstart-test-workflow-retry-by-default branch from ff2871c to b597a48 Compare October 16, 2024 12:14
@rvykydal
Copy link
Contributor Author

/kickstart-test --testtype smoke

KKoukiou
KKoukiou reviewed Oct 17, 2024
View reviewed changes
Copy link
Contributor

@KKoukiou KKoukiou left a comment

Choose a reason for hiding this comment

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

How many times will it retry?

Let's say we have a situation where kickstart tests fail in an expected way, and we just need to wait for a container refresh or something to move on with testing. Will it retry forever eating resources?

@rvykydal
Copy link
Contributor Author

rvykydal commented Oct 17, 2024 •
edited
Loading

How many times will it retry?

Let's say we have a situation where kickstart tests fail in an expected way, and we just need to wait for a container refresh or something to move on with testing. Will it retry forever eating resources?

@KKoukiou it retries a failing test one time. Tests have default timeout 30mins (https://github.com/rhinstaller/kickstart-tests/blob/e53f343ede308bf5893e8cd8f769fabd6886ae58/functions.sh#L464)
Single retry is used also in daily runs where it is usually enough to deal with flakes. If it is not (rate of flakes say ~7/100 tests or higher) causing double-flake of a test in some of daily runs I usually rise the issue and we try to fix it.

So the run should take max ~1.2 h - in case of a test timing out twice.

@rvykydal rvykydal merged commit 7b21249 into rhinstaller:master Oct 17, 2024
17 checks passed
jkonecny12
jkonecny12 reviewed Oct 17, 2024
View reviewed changes
Copy link
Member

@jkonecny12 jkonecny12 left a comment

Choose a reason for hiding this comment

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

Thanks for this!

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

@KKoukiou KKoukiou KKoukiou approved these changes

@jkonecny12 jkonecny12 jkonecny12 left review comments

@jstodola jstodola jstodola approved these changes

@M4rtinK M4rtinK M4rtinK approved these changes

@jikortus jikortus Awaiting requested review from jikortus

Assignees
No one assigned
Labels
f42 Fedora 42 infrastructure Changes affecting mostly infrastructure
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@rvykydal @M4rtinK @jkonecny12 @jstodola @KKoukiou