-
-
Notifications
You must be signed in to change notification settings - Fork 58
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
GH Actions/test: allow concurrency for code coverage builds - take two #735
Merged
jrfnl
merged 1 commit into
master
from
feature/ghactions-dont-cancel-in-progress-take-two
Nov 27, 2024
Merged
GH Actions/test: allow concurrency for code coverage builds - take two #735
jrfnl
merged 1 commit into
master
from
feature/ghactions-dont-cancel-in-progress-take-two
Nov 27, 2024
+16
−10
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jrfnl
commented
Nov 27, 2024
Comment on lines
-259
to
+266
if [[ ${{ matrix.custom_ini }} == true && "${{ matrix.php }}" == '7.2' ]]; then | ||
echo 'PHP_INI=, date.timezone=Australia/Sydney, short_open_tag=On' >> "$GITHUB_OUTPUT" | ||
if [[ ${{ matrix.custom_ini }} == true && "${{ matrix.php }}" == '7.2' ]]; then | ||
echo 'PHP_INI=, date.timezone=Australia/Sydney, short_open_tag=On' >> "$GITHUB_OUTPUT" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No changes here. This may be a line ending thingie or something.
Looks like I need to make the group name more specific still. |
Follow up on 710, which was added with the following reasoning: > The `concurrency` setting will cancel running workflows if a new push to the same branch is seen. This is useful to prevent unnecessary workflow runs (as the previous push was superseded, so the outcome is no longer relevant). > > However, for the "main" branches (`master` and `4.0`), this workflow cancelling means that if multiple PRs are merged in succession, only the code coverage for the _last_ merge is recorded in Coveralls as the workflow runs on `master` for the previous merges will have been cancelled. > > While in practice, it's not a biggie, it does make it more difficult to identify which commit/merge added or decreased code coverage. Let's try this again. The previous attempt to prevent cancelling code coverage builds for merges to the "main" branches is not working as intended and is still cancelling builds. This is likely due to the way GH looks at concurrency groups: > By default, GitHub Actions allows multiple jobs within the same workflow, multiple workflow runs within the same repository, and multiple workflow runs across a repository owner's account to run concurrently. This means that multiple workflow runs, jobs, or steps can run at the same time. > > You can use `*.concurrency` to ensure that only a single job or workflow using the same concurrency group will run at a time. > > This means that there can be at most one running and one pending job in a concurrency group at any time. When a concurrent job or workflow is queued, if another job or workflow using the same concurrency group in the repository is in progress, the queued job or workflow will be `pending`. Any existing `pending` job or workflow in the same concurrency group, if it exists, will be canceled and the new queued job or workflow will take its place. Interpreting this strictly, this appears to mean that, as soon as a `concurrency` `group` name is defined, there can now only be - at most - two builds for that group - one running, one queued -, while without the `concurrency` `group` name being associated with a build, there can be unlimited concurrent builds. This means that code coverage builds for merges in quick succession are still being killed off. This new attempt now moves the `concurrency` setting from the workflow level to the "job" level and adds the `job` name to the `group` key. This should hopefully still allow for cancelling in progress builds for the `build` and `test` jobs, while leaving the `coverage` (and `coveralls-finish`) jobs alone. The down-side of this change (providing it works) is that it can't be limited to the "main" branches, which means that the `coverage` jobs will now continue running for _every_ push to an open pull request as well. While a little wasteful, that appears to be the price to pay. Refs: * https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/control-the-concurrency-of-workflows-and-jobs
jrfnl
force-pushed
the
feature/ghactions-dont-cancel-in-progress-take-two
branch
from
November 27, 2024 07:22
c2340c6
to
f8ed72b
Compare
This appears to work. Merging once the build has finished. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Follow up on #710, which was added with the following reasoning:
Let's try this again.
The previous attempt to prevent cancelling code coverage builds for merges to the "main" branches is not working as intended and is still cancelling builds.
This is likely due to the way GH looks at concurrency groups:
Interpreting this strictly, this appears to mean that, as soon as a
concurrency
group
name is defined, there can now only be - at most - two builds for that group - one running, one queued -, while without theconcurrency
group
name being associated with a build, there can be unlimited concurrent builds.This means that code coverage builds for merges in quick succession are still being killed off.
This new attempt now moves the
concurrency
setting from the workflow level to the "job" level and adds thejob
name to thegroup
key. This should hopefully still allow for cancelling in progress builds for thebuild
andtest
jobs, while leaving thecoverage
(andcoveralls-finish
) jobs alone.The down-side of this change (providing it works) is that it can't be limited to the "main" branches, which means that the
coverage
jobs will now continue running for every push to an open pull request as well. While a little wasteful, that appears to be the price to pay.Refs:
Suggested changelog entry
N/A