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
28 changes: 19 additions & 9 deletions .github/workflows/pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,17 @@ name: PR
# checks, the WebAssembly, Xpect and conformance gates, and only the clients and
# docs whose own paths changed. A push to a non-draft pull request, marking one
# ready for review, or the label ci:full runs the full tier: the whole suite.
# A merge-queue commit (merge_group, refs/heads/gh-readonly-queue/develop/...) always
# runs the full tier over its base_sha..head_sha; Build and test on it lets the entry merge.
on:
pull_request:
types: [opened, synchronize, reopened, ready_for_review, labeled]
merge_group:
types: [checks_requested]

concurrency:
group: pr-${{ github.event.pull_request.number }}
# A queue commit has no pull request; its ref is unique to the entry.
group: ${{ github.event_name == 'merge_group' && format('merge-group-{0}', github.ref) || format('pr-{0}', github.event.pull_request.number) }}
# Any other label re-runs the tier that applies, but waits for the running one rather than cancelling it.
cancel-in-progress: ${{ github.event.action != 'labeled' || github.event.label.name == 'ci:full' }}

Expand Down Expand Up @@ -83,8 +88,8 @@ jobs:
- name: Decide the tier
id: tier
env:
DRAFT: ${{ github.event.pull_request.draft }}
CI_FULL_LABEL: ${{ contains(github.event.pull_request.labels.*.name, 'ci:full') }}
DRAFT: ${{ github.event_name != 'merge_group' && github.event.pull_request.draft }}
CI_FULL_LABEL: ${{ github.event_name != 'merge_group' && contains(github.event.pull_request.labels.*.name, 'ci:full') }}
run: |
if [ "$DRAFT" != true ] || [ "$CI_FULL_LABEL" = true ]; then
echo "tier=full"
Expand All @@ -94,12 +99,15 @@ jobs:
echo 'go_shards=["runtime","model export graph","rest migrate cli codegen"]'
fi | tee -a "$GITHUB_OUTPUT"

# A pull request is diffed against the branch it merges into; a merge-group
# commit against the queue entry it builds on (the merge group's base_sha).
- name: Decide which areas to run
id: areas
env:
BASE_REF: ${{ github.base_ref }}
DIFF_BASE: ${{ github.event_name == 'merge_group' && github.event.merge_group.base_sha || format('origin/{0}', github.base_ref) }}
DIFF_HEAD: ${{ github.event_name == 'merge_group' && github.event.merge_group.head_sha || 'HEAD' }}
CI_TIER: ${{ steps.tier.outputs.tier }}
run: scripts/ci-changed-areas.sh "origin/$BASE_REF" HEAD | tee -a "$GITHUB_OUTPUT"
run: scripts/ci-changed-areas.sh "$DIFF_BASE" "$DIFF_HEAD" | tee -a "$GITHUB_OUTPUT"

# The vendored copy is checked in this job, not static-and-integrity: the
# changed-areas filter may skip that job for a non-Go change, but drift
Expand Down Expand Up @@ -351,12 +359,13 @@ jobs:
- name: Lint the protobuf schema
run: make proto-lint

# Wire compatibility against the branch this pull request merges into;
# the baseline is a git archive, so a shallow fetch of it is enough.
# Wire compatibility against the branch this pull request (or merge group, as
# refs/heads/<branch>) merges into; the baseline is a git archive, so a shallow fetch does.
- name: Check the protobuf schema for breaking changes
env:
BASE_REF: ${{ github.base_ref }}
BASE_REF: ${{ github.event_name == 'merge_group' && github.event.merge_group.base_ref || github.base_ref }}
run: |
BASE_REF=${BASE_REF#refs/heads/}
git fetch --no-tags --depth=1 origin "$BASE_REF"
make proto-breaking BUF_BREAKING_REF="origin/$BASE_REF"

Expand Down Expand Up @@ -687,10 +696,11 @@ jobs:
- name: Download Go modules
run: go mod download

# A queue commit belongs to no one pull request, so it is stamped merge-queue; COMMIT names it.
- name: Build binaries
run: |
make build \
VERSION="pr-${{ github.event.pull_request.number }}" \
VERSION="${{ github.event_name == 'merge_group' && 'merge-queue' || format('pr-{0}', github.event.pull_request.number) }}" \
COMMIT="$(git rev-parse --short HEAD)" \
BUILD_TIME="$(date -u '+%Y-%m-%d_%H:%M:%S')" \
GO_VERSION="$(go version | awk '{print $3}')"
Expand Down
6 changes: 6 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,12 @@ and solver gates, the PDF toolchain, the coverage merge and the fan-out from a s
to every client wait for the full tier. Draft until the quick tier is green, then mark ready;
the full tier is what a pull request has to pass to merge.

**The merge queue**, when the `develop` ruleset has one, runs the full tier once more on the
commit it builds for each queued pull request (`refs/heads/gh-readonly-queue/develop/...`,
the `merge_group` event): the diff is the merge group's own, `base_sha..head_sha`, the run is
never a draft, and `Build and test` reporting on that commit is what lets the entry merge. A
queue commit therefore gets at least the checks a ready pull request with the same diff gets.

The first job, `Changed areas`, decides the tier from the pull request's draft flag and labels,
then runs `scripts/ci-changed-areas.sh` over its files and the rest of the jobs are gated on
what it reports: a change confined to one client runs that client's job alone, while a change
Expand Down
1 change: 1 addition & 0 deletions changes/unreleased/merge-group-ci.changed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- **The pull-request workflow runs on merge-queue commits.** `.github/workflows/pr.yml` triggers on `merge_group` as well as `pull_request`: a queued pull request's commit on `gh-readonly-queue/develop/...` gets the full tier over the merge group's own diff, keyed to a concurrency group of its own, and `Build and test` reports on it instead of leaving the entry to time out. A hygiene test holds the workflow to the trigger, the merge-group branches of `Changed areas` and the concurrency key.
162 changes: 162 additions & 0 deletions tests/hygiene/merge_group_workflow_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
package hygiene

import (
"os"
"regexp"
"strings"
"testing"

"gopkg.in/yaml.v3"
)

// The develop ruleset merges through a merge queue, which builds a commit on
// refs/heads/gh-readonly-queue/develop/... and asks for the required checks on
// it through the merge_group event. These tests hold .github/workflows/pr.yml,
// the workflow that produces those checks, to answering that event: it has to
// trigger on it, run the full tier over the merge group's own diff, and give
// each queue commit a concurrency group of its own.

type prWorkflow struct {
On yaml.Node `yaml:"on"`
Concurrency struct {
Group string `yaml:"group"`
} `yaml:"concurrency"`
Jobs map[string]actionsJob `yaml:"jobs"`
}

func loadPRWorkflow(t *testing.T) (prWorkflow, string) {
t.Helper()
raw, err := os.ReadFile("../../.github/workflows/pr.yml")
if err != nil {
t.Fatalf("read .github/workflows/pr.yml: %v", err)
}
var workflow prWorkflow
if err := yaml.Unmarshal(raw, &workflow); err != nil {
t.Fatalf("parse .github/workflows/pr.yml: %v", err)
}
return workflow, string(raw)
}

func prWorkflowStep(t *testing.T, workflow prWorkflow, job, name string) actionsStep {
t.Helper()
j, ok := workflow.Jobs[job]
if !ok {
t.Fatalf("pr.yml has no %s job", job)
}
for _, step := range j.Steps {
if step.Name == name {
return step
}
}
t.Fatalf("pr.yml job %s has no step named %q", job, name)
return actionsStep{}
}

func TestPRWorkflowTriggersOnMergeGroup(t *testing.T) {
workflow, _ := loadPRWorkflow(t)
var triggers map[string]struct {
Types []string `yaml:"types"`
}
if err := workflow.On.Decode(&triggers); err != nil {
t.Fatalf("decode pr.yml on: %v", err)
}
if _, ok := triggers["pull_request"]; !ok {
t.Fatal("pr.yml no longer triggers on pull_request")
}
mergeGroup, ok := triggers["merge_group"]
if !ok {
t.Fatal("pr.yml does not trigger on merge_group: the merge queue's commits get no checks and every entry times out")
}
if len(mergeGroup.Types) != 1 || mergeGroup.Types[0] != "checks_requested" {
t.Fatalf("pr.yml merge_group types = %v, want [checks_requested]", mergeGroup.Types)
}
}

// The group expression picks one of two forms by event, and the forms must
// name different things: a queue commit has no pull request number, and a key
// that collapsed every queue commit onto one group would cancel the queue.
func TestPRWorkflowConcurrencyGroupSeparatesMergeGroups(t *testing.T) {
workflow, _ := loadPRWorkflow(t)
group := workflow.Concurrency.Group
conditional := regexp.MustCompile(`^\$\{\{\s*github\.event_name == 'merge_group' && (.+?) \|\| (.+?)\s*\}\}$`)
m := conditional.FindStringSubmatch(group)
if m == nil {
t.Fatalf("pr.yml concurrency.group = %q, want a github.event_name == 'merge_group' && <queue form> || <pull request form> expression", group)
}
mergeGroupForm, pullRequestForm := m[1], m[2]
if mergeGroupForm == pullRequestForm {
t.Fatalf("pr.yml concurrency.group uses %q for both events", mergeGroupForm)
}
if !strings.Contains(mergeGroupForm, "github.ref") && !strings.Contains(mergeGroupForm, "merge_group.head_ref") {
t.Fatalf("pr.yml concurrency.group merge-group form %q is not keyed on the queue ref", mergeGroupForm)
}
if !strings.Contains(pullRequestForm, "pull_request.number") {
t.Fatalf("pr.yml concurrency.group pull-request form %q is not keyed on the pull request number", pullRequestForm)
}
}

// Changed areas decides the tier from the draft flag and the diff from the
// base branch; a merge-group commit has neither a draft flag nor a base_ref,
// so both need a merge_group branch, and the diff is the merge group's own.
func TestPRWorkflowChangedAreasHandlesMergeGroups(t *testing.T) {
workflow, _ := loadPRWorkflow(t)

tier := prWorkflowStep(t, workflow, "changes", "Decide the tier")
for _, name := range []string{"DRAFT", "CI_FULL_LABEL"} {
value, ok := tier.Env[name]
if !ok {
t.Fatalf("pr.yml Decide the tier has no %s env", name)
}
if !strings.Contains(value, "github.event_name != 'merge_group'") {
t.Errorf("pr.yml Decide the tier %s = %q does not turn off for merge_group, so a queue commit could run the quick tier", name, value)
}
}

areas := prWorkflowStep(t, workflow, "changes", "Decide which areas to run")
for env, sha := range map[string]string{"DIFF_BASE": "base_sha", "DIFF_HEAD": "head_sha"} {
value, ok := areas.Env[env]
if !ok {
t.Fatalf("pr.yml Decide which areas to run has no %s env", env)
}
if !strings.Contains(value, "github.event_name == 'merge_group' && github.event.merge_group."+sha) {
t.Errorf("pr.yml Decide which areas to run %s = %q does not use the merge group's %s", env, value, sha)
}
}
if !strings.Contains(areas.Env["DIFF_BASE"], "github.base_ref") {
t.Errorf("pr.yml Decide which areas to run DIFF_BASE = %q no longer diffs a pull request against its base branch", areas.Env["DIFF_BASE"])
}
if !strings.Contains(areas.Run, `"$DIFF_BASE" "$DIFF_HEAD"`) {
t.Errorf("pr.yml Decide which areas to run = %q does not pass the diff range to scripts/ci-changed-areas.sh", areas.Run)
}
}

// Every read of the pull-request payload or the base ref has to be guarded by
// the event, or a queue commit evaluates it to nothing.
func TestPRWorkflowPullRequestReadsHaveMergeGroupBranch(t *testing.T) {
_, raw := loadPRWorkflow(t)
pullRequestOnly := regexp.MustCompile(`github\.event\.pull_request\.|github\.base_ref`)
for i, line := range strings.Split(raw, "\n") {
if !pullRequestOnly.MatchString(line) {
continue
}
if !strings.Contains(line, "github.event_name") || !strings.Contains(line, "merge_group") {
t.Errorf("pr.yml:%d reads the pull request payload with no merge_group branch: %s", i+1, strings.TrimSpace(line))
}
}
}

func TestPRWorkflowAggregateReportsOnEveryEvent(t *testing.T) {
workflow, raw := loadPRWorkflow(t)
aggregate, ok := workflow.Jobs["build-and-test"]
if !ok {
t.Fatal("pr.yml has no build-and-test job")
}
if len(aggregate.Steps) == 0 {
t.Fatal("pr.yml build-and-test has no steps")
}
// The job-level if and name are not in actionsJob; read them where they are written.
jobHeader := regexp.MustCompile(`(?m)^ build-and-test:\n name: Build and test\n if: always\(\)\n`)
if !jobHeader.MatchString(raw) {
t.Fatal("pr.yml build-and-test is not the always-running job named Build and test, the check the develop ruleset requires")
}
}
Loading