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

ci: don't fail fast #838

Merged
merged 1 commit into from
Aug 28, 2024
Merged

ci: don't fail fast #838

merged 1 commit into from
Aug 28, 2024

Conversation

Freax13
Copy link
Contributor

@Freax13 Freax13 commented Aug 28, 2024

The matrix strategy defaults to failing fast: If any one of the test cases fails all other cases get cancelled as well. The problem with this is that sometimes tests fail not because the code is bad, but because the test is flaky. In an ideal world, we'd make the tests less flaky, but until then we shouldn't abort all other tests in the matrix. The advantage of not failing fast is that when we inevitably have to manually push the re-run button, we don't have to wait for all test cases to complete, but only for the one that failed.

The matrix strategy defaults to failing fast: If any one of the
test cases fails all other cases get cancelled as well. The problem
with this is that sometimes tests fail not because the code is bad, but
because the test is flaky. In an ideal world, we'd make the tests less
flaky, but until then we shouldn't abort all other tests in the matrix.
The advantage of not failing fast is that when we inevitably have to
manually push the re-run button, we don't have to wait for all test
cases to complete, but only for the one that failed.
Copy link
Contributor

@msanft msanft left a comment

Choose a reason for hiding this comment

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

The disadvantage is that all tests will take way longer if the code is bad, right?

@Freax13 Freax13 added the no changelog PRs not listed in the release notes label Aug 28, 2024
@Freax13
Copy link
Contributor Author

Freax13 commented Aug 28, 2024

The disadvantage is that all tests will take way longer if the code is bad, right?

Following this change failing tests would take the same amount (or less) time as succeeding tests.

@msanft
Copy link
Contributor

msanft commented Aug 28, 2024

The disadvantage is that all tests will take way longer if the code is bad, right?

Following this change failing tests would take the same amount (or less) time as succeeding tests.

Won't we run all payloads, even if all will fail? So, of course, your statement is correct, but they would take longer than they currently take.

@Freax13 Freax13 merged commit e6af527 into main Aug 28, 2024
21 of 22 checks passed
@Freax13 Freax13 deleted the tom/dont-fail-fast branch August 28, 2024 11:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no changelog PRs not listed in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants