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

Disable fail-fast for the PHPUnit testing matrix. #47799

Closed
wants to merge 1 commit into from

Conversation

desrosj
Copy link
Contributor

@desrosj desrosj commented Feb 6, 2023

What?

This PR is proposing that the fail-fast option be disabled in the PHPUnit test matrix.

Why?

At the surface, automatically cancelling all jobs within a matrix when any one encounters a problem seems like the right option (and sometimes it is), there are benefits to allowing each individual job to finish in this job strategy.

It's not uncommon for one or more jobs to encounter a connection issue, or something else that results in a failure. When this happens, it doesn't necessarily mean all of them will fail. GitHub Actions allows only failed jobs to be rerun within a workflow. By cancelling all of the PHP jobs when one encounters a problem, the benefit of only re-running the failures is eliminated entirely, and the time spent by any of the jobs that may have succeeded is wasted as they all will be rerun (cancelled is considered failed).

Another instance where it's valuable to know which jobs fail and which pass is when code is incompatible with a specific version of PHP. For example, code may work on PHP 7.x but trigger a fatal error on 8.x. Allowing all jobs to finish will make this scenario more clear and more discoverable, especially since contributors do not commonly test against multiple versions of PHP locally.

It looks like fail-fast was enabled in #46510.

@github-actions
Copy link

github-actions bot commented Feb 6, 2023

Flaky tests detected in 210318e.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4107283625
📝 Reported issues:

@anton-vlasenko
Copy link
Contributor

Thanks for the ping, @desrosj!

It looks like fail-fast was enabled in #46510.

Yes, that's correct.
There was a discussion about enabling the fail-fast option during the review phase of #46510.

Speaking of me, I don't have a strong preference here as each approach has its benefits.
Enabling the fail-fast option allows for saving some GH resources.
At the same time, disabling the fail-fast allows seeing which jobs fail and which pass when code is incompatible with a specific version of PHP. BTW that was my exact argument when we discussed the fail-fast option.

Pinging @jrfnl for visibility.

@desrosj
Copy link
Contributor Author

desrosj commented Feb 7, 2023

Thanks @anton-vlasenko. I was sure it had been discussed, but I missed it when looking back at that one.

fail-fast does save resources during the first run, but if the jobs made it all the way to the end, it's likely that more resources are used when failures occur on select jobs because every one will rerun.

@jrfnl
Copy link
Member

jrfnl commented Feb 8, 2023

@desrosj I suggest reading the previous discussion if you have not done so yet. Kind of feels redundant to repeat myself here....

@github-actions github-actions bot added the Stale label Aug 8, 2023
@jordesign
Copy link
Contributor

Hi @desrosj - just looping back to this stale PR to see if it is something you're still pursuing or if it can be closed?

@desrosj
Copy link
Contributor Author

desrosj commented Nov 6, 2023

Fine with just closing out. Thanks!

@desrosj desrosj closed this Nov 6, 2023
@desrosj desrosj deleted the actions/disable-fail-fast branch November 6, 2023 16:23
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.

4 participants