Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ func runReleasePleasePRChecksWithDeps(ctx context.Context, stdout io.Writer, std
writeReleasePRCheckLine(stdout, "No pending release-please PR found for "+config.targetBranch+".")
return 0
}
releasePRs = orderReleasePRsForChecks(releasePRs, config.targetBranch)

// One component's failing checks must not strand the other components'
// release PRs in draft, so every PR is processed and the failures are
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -890,6 +890,49 @@ func TestReleasePRChecksLeaveUnityPackagePullRequestDraft(t *testing.T) {
assertReleasePRCheckLogDoesNotContainLine(t, commandLogText, "gh pr ready 2002 --repo owner/repository")
}

// Verifies that the Unity package release PR's checks are dispatched before another component's even when the listing puts the package PR last, so post-publish's bounded wait for that PR never sits behind another component's checks.
func TestReleasePRChecksDispatchUnityPackagePullRequestFirst(t *testing.T) {
prListJSON := `[` +
`{"number":2001,"headRefName":"release-please--branches--main--components--dispatcher","headRefOid":"dispatcher123","title":"chore(main): release dispatcher 3.6.0","url":"https://example.test/pr/2001"},` +
`{"number":2002,"headRefName":"release-please--branches--main--components--unity-package","headRefOid":"package123","title":"chore(main): release 3.6.0","url":"https://example.test/pr/2002"}` +
`]`

exitCode, _, stderr, stub := runMultiReleasePRCheck(t, prListJSON, nil)

if exitCode != 0 {
t.Fatalf("expected exit code 0, got %d\nstderr: %s", exitCode, stderr)
}
dispatchedBranches := []string{}
for _, commandLine := range stub.commandLog {
if strings.HasPrefix(commandLine, "gh workflow run ") {
dispatchedBranches = append(dispatchedBranches, commandLine)
}
}
if len(dispatchedBranches) == 0 || !strings.Contains(dispatchedBranches[0], "--components--unity-package") {
t.Fatalf("expected the first dispatch to target the Unity package release branch, got %v", dispatchedBranches)
}
if !strings.Contains(dispatchedBranches[len(dispatchedBranches)-1], "--components--dispatcher") {
t.Fatalf("expected the dispatcher release branch to be dispatched after the package one, got %v", dispatchedBranches)
}
}

// Verifies that failing Unity package checks, which now run first, still let the next component's release PR be marked ready, and the command reports only the package PR as failed.
func TestReleasePRChecksContinueAfterUnityPackageChecksFail(t *testing.T) {
prListJSON := `[` +
`{"number":2001,"headRefName":"release-please--branches--main--components--dispatcher","headRefOid":"dispatcher123","title":"chore(main): release dispatcher 3.6.0","url":"https://example.test/pr/2001"},` +
`{"number":2002,"headRefName":"release-please--branches--main--components--unity-package","headRefOid":"package123","title":"chore(main): release 3.6.0","url":"https://example.test/pr/2002"}` +
`]`

exitCode, _, stderr, stub := runMultiReleasePRCheck(t, prListJSON, map[string]bool{"21": true})

if exitCode != 1 {
t.Fatalf("expected exit code 1, got %d\nstderr: %s", exitCode, stderr)
}
commandLogText := strings.Join(stub.commandLog, "\n")
assertReleasePRCheckLogContainsLine(t, commandLogText, "gh pr ready 2001 --repo owner/repository")
assertReleasePRCheckLogContains(t, stderr, "Release PR checks failed for #2002; the errors above say why.")
}

// Verifies that one component's failing checks still let the other component's release PR be finalized, and the command reports the failure.
func TestReleasePRChecksContinueAfterOneComponentFails(t *testing.T) {
prListJSON := `[` +
Expand Down Expand Up @@ -922,7 +965,7 @@ func TestReleasePRChecksNameEveryFailedPullRequestLast(t *testing.T) {
}
stderrLines := strings.Split(strings.TrimRight(stderr, "\n"), "\n")
lastLine := stderrLines[len(stderrLines)-1]
expected := "Release PR checks failed for #2001, #2002; the errors above say why."
expected := "Release PR checks failed for #2002, #2001; the errors above say why."
if lastLine != expected {
t.Fatalf("expected last stderr line %q, got %q\nstderr: %s", expected, lastLine, stderr)
}
Expand Down
17 changes: 17 additions & 0 deletions cli/release-automation/internal/automation/release_pr_lookup.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"encoding/json"
"fmt"
"sort"
"strings"
"time"
)
Expand Down Expand Up @@ -85,6 +86,22 @@ func releasePRCheckComponentFromHeadRef(headRefName string, targetBranch string)
return strings.TrimPrefix(headRefName, componentPrefix)
}

// orderReleasePRsForChecks puts the Unity package release pull request first and
// keeps every other pull request in listed order. Checks run one pull request at a
// time, and dispatcher-publish's post-publish waits a bounded time for the package
// pull request's checks before merging it; a package pull request listed after
// another component's waited behind that component's checks and timed out.
func orderReleasePRsForChecks(releasePRs []releasePullRequest, targetBranch string) []releasePullRequest {
ordered := append([]releasePullRequest(nil), releasePRs...)
isPackage := func(releasePR releasePullRequest) bool {
return releasePRCheckComponentFromHeadRef(releasePR.HeadRefName, targetBranch) == mergePackageReleasePRComponent
}
sort.SliceStable(ordered, func(i, j int) bool {
return isPackage(ordered[i]) && !isPackage(ordered[j])
})
return ordered
}

// releasePleaseReleaseTitleMatches says whether a title is a release-please release title. A
// release pull request carries one, and so does the commit that squash-merges it, with the pull
// request number appended.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package automation
import (
"context"
"errors"
"slices"
"strings"
"testing"
"time"
Expand Down Expand Up @@ -58,3 +59,33 @@ func TestFindReleasePRCheckPullRequestsWithRetryStopsOnInterruptedWait(t *testin
t.Fatalf("expected the wait error after one lookup, got %v after %d lookups", err, calls)
}
}

// Verifies the Unity package release pull request is moved to the front while every other pull request keeps its listed order, including the combined branch and a listing without a package pull request.
func TestOrderReleasePRsForChecks(t *testing.T) {
const prefix = "release-please--branches--main--components--"
runner := releasePullRequest{Number: 1, HeadRefName: prefix + "uloop-project-runner"}
dispatcher := releasePullRequest{Number: 2, HeadRefName: prefix + "dispatcher"}
unityPackage := releasePullRequest{Number: 3, HeadRefName: prefix + "unity-package"}
combined := releasePullRequest{Number: 4, HeadRefName: "release-please--branches--main"}
cases := []struct {
name string
listed []releasePullRequest
want []int
}{
{name: "package listed last", listed: []releasePullRequest{runner, dispatcher, unityPackage}, want: []int{3, 1, 2}},
{name: "package already first", listed: []releasePullRequest{unityPackage, dispatcher}, want: []int{3, 2}},
{name: "package only", listed: []releasePullRequest{unityPackage}, want: []int{3}},
{name: "no package", listed: []releasePullRequest{runner, dispatcher}, want: []int{1, 2}},
{name: "combined branch is not the package", listed: []releasePullRequest{combined, unityPackage}, want: []int{3, 4}},
}
for _, testCase := range cases {
ordered := orderReleasePRsForChecks(testCase.listed, "main")
got := []int{}
for _, releasePR := range ordered {
got = append(got, releasePR.Number)
}
if !slices.Equal(got, testCase.want) {
t.Fatalf("%s: order = %v, want %v", testCase.name, got, testCase.want)
}
}
}
4 changes: 3 additions & 1 deletion docs/dispatcher-pin-release-order.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@ and is merged only by automation; the other two are the ones people merge.
2. `post-publish` stamps the pin on `main`, then marks the **unity-package**
release pull request ready and merges it, once that pull request's head
records the dispatcher tag just published and has a successful run of every
required workflow for that exact head commit.
required workflow for that exact head commit. The release PR checks run one
pull request at a time, so the unity-package one always goes first; behind
another component's checks it would outlast the time `post-publish` waits.
3. A package release that carries no dispatcher bump never reaches step 2, so
`release-please.yml` merges it instead: after the release PR checks, it
marks the package pull request ready and merges it, but only while no
Expand Down
Loading