ci: factor plumbing calls into a composite actions#2412
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR reduces duplication across GitHub Actions workflows by introducing a local composite action that centralizes FrankenPHP’s Go toolchain setup (pinned Go version, standard module-cache dependency paths, and check-latest), then updating workflows to use that action.
Changes:
- Added a composite action at
.github/actions/setup-go/to wrapactions/setup-go@v6with the repo’s pinned Go version and shared cache config. - Updated 5 workflows to call the composite action instead of duplicating
actions/setup-goconfiguration. - Added inputs to handle subdirectory checkouts (
working-directory) and allow disabling the cache (cache) for release/tag contexts.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/windows.yaml | Switches to the composite setup-go action and configures it for a subdirectory checkout (working-directory: frankenphp) and tag-aware caching. |
| .github/workflows/tests.yaml | Replaces duplicated actions/setup-go blocks with the composite action across test jobs. |
| .github/workflows/static.yaml | Uses the composite action and disables cache for release events as intended. |
| .github/workflows/sanitizers.yaml | Uses the composite action for consistent Go setup in sanitizer runs. |
| .github/workflows/pgo-profile.yaml | Uses the composite action to standardize Go setup for the PGO refresh workflow. |
| .github/actions/setup-go/action.yaml | Introduces the composite action that centralizes Go version, cache dependency paths, check-latest, and the zizmor pragma. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
dunglas
added a commit
that referenced
this pull request
May 13, 2026
`TestAutoScaleRegularThreadsOnAutomaticThreadLimit` (and its sibling `TestAutoScaleWorkerThreads`) in `caddy/admin_test.go` would burn the package-wide 10 minute Go test timeout whenever the PHP server returned a connection error during the load loop. This was observed on the PHP 8.5 / Linux CI matrix on #2412 (and on #2381 for the related `TestAddModuleWorkerViaAdminApi`). Root cause: `caddytest.AssertGetResponse` calls `t.Fatalf` on connection errors. From a non-test goroutine that triggers `runtime.Goexit`, which skips the plain `wg.Done()` after it, so `wg.Wait()` blocks forever and the retry loop never gets to check `amountOfThreads` or break out. Two surgical changes: - `defer wg.Done()` so a `Fatalf`'d goroutine still decrements the WaitGroup. - Break out of the retry loop once `t.Failed()` is true so we don't keep hammering an already-broken server for another 9 rounds. Net diff: +6 / -2 lines per test, no new imports.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
.github/actions/setup-go/action.yaml:18
- This action sets
cache:but the previous workflows carried azizmor: ignore[cache-poisoning]pragma andzizmor.yamldoesn’t appear to allowlist this rule. If zizmor is still enabled in CI, add the pragma here (or updatezizmor.yaml) to avoid new lint failures after centralizing the setup-go call.
- uses: actions/setup-go@v6
with:
go-version-file: ${{ inputs.working-directory }}/go.mod
cache-dependency-path: |
${{ inputs.working-directory }}/go.sum
${{ inputs.working-directory }}/caddy/go.sum
cache: ${{ !startsWith(github.ref, 'refs/tags/') }}
check-latest: true
dunglas
changed the title
This file contains hidden or 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
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.
Factor repeated CI plumbing into three composite actions under
.github/actions/:setup-go/— readsgo-version-file: go.modwithcheck-latest: trueso future Go upgrades only require bumpinggo.mod. Disables the Go module cache on tag-triggered runs (!startsWith(github.ref, 'refs/tags/')) to prevent cache poisoning into release binaries. Replaces 7actions/setup-go@v6call sites.setup-php/— wrapsshivammathur/setup-php@v2with the ZTS + debug +developmentini-file flavor used by tests and PGO. Defaultsphp-versionto"8.5". Replaces 4 nearly-identical call sites intests.yamlandpgo-profile.yaml. (translate.yamlleft alone — it uses plain non-debug PHP.)version/— resolves the FrankenPHP version from the current ref (stripped tag name, fallback to commit SHA), exposed as a step output. Unifies the bash logic instatic.yamlwith the duplicated PowerShell logic inwindows.yaml. Output is consumed via per-stepenv: FRANKENPHP_VERSION:injection in consuming steps, so the action does no$GITHUB_ENVwrite.Side benefits:
static.yaml'scache: ${{ github.event_name != 'release' }}was effectively always-true (the workflow never fires onreleaseevents). The new rule actually disables cache on tag pushes as intended.github-actionsecosystem already tracks.github/actions/, so pins inside the composite actions get auto-bumped.cache-poisoningandgithub-envwere both eliminated by structural changes rather than silencing).Net: +96 / -107 lines.