Repository navigation
fix(ci): harden stacked PR test scoping and add unit-tests concurrency - #1008
Merged
Merged
Conversation
Add workflow concurrency cancellation to .github/workflows/unit-tests.yml for superseded pull_request runs while preserving post-submit runs on main. Update run-backend-tests and run-ui-tests composite actions to fetch origin/main and origin/<base_ref>, pass PR_HEAD_SHA (with merge_group fallbacks) instead of diffing against GitHub's synthetic merge commit, and pass PR_BASE_REF (--base-branch) to resolve_test_scope.py. Harden tools/benchmark/resolve_test_scope.py to: - Prioritize live origin/<base_branch> over stale webhook base_ref and select the closest ancestor across target and origin/main via git merge-base --independent while excluding head_ref self-matches. - Retry git diff against HEAD when PR_HEAD_SHA is missing locally due to rapid force-push races during checkout. - Fail open to the full test suite on workflow_dispatch/schedule runs on main and when the modified file list is empty. - Include .bazelrc, .bazelversion, unit-tests.yml, run-backend-tests, and resolve_test_scope.py in CORE_INFRA_FILES, classify run-ui-tests action modifications as UI changes, and include non-code backend testdata packages in Bazel rdeps queries. Document the scaled-up ARC runner capacity (minRunners: 24, maxRunners: 48 across 12-24 n2-standard-16 GKE nodes) in docs/ci_runner_infrastructure.md. TAG=agy CONV=a6d09aaa-980b-48ad-9a26-4b42929c4dab
There was a problem hiding this comment.
Code Review
This pull request enhances the CI test scope resolution to support stacked PR branches by determining the closest merge-base across the target branch, base reference, and main branch. It updates the GitHub Actions workflows, documents the changes, and adds comprehensive unit tests. The review feedback suggests improving shell safety in the workflows by replacing unquoted expansions with explicit conditional blocks, handling potential command failures when running git merge-base --independent, and using the more idiomatic any() generator expression instead of a list comprehension to check for non-empty files.
- Replace unquoted ${PR_BASE_REF:+...} expansion with explicit if-else
blocks and double-quoted "${PR_BASE_REF}" in run-backend-tests and
run-ui-tests composite actions.
- Guard indep.stdout parsing with indep.returncode == 0 in
resolve_effective_base_ref.
- Use short-circuiting any(f.strip() for f in files) in resolve_test_scope.
Comment link: #1008 (comment)
Comment link: #1008 (comment)
Comment link: #1008 (comment)
Comment link: #1008 (comment)
TAG=agy
CONV=a6d09aaa-980b-48ad-9a26-4b42929c4dab
Keep ARC AutoscalingRunnerSet documentation generic without exposing specific GKE node machine types, node counts, or pod resource bounds. TAG=agy CONV=a6d09aaa-980b-48ad-9a26-4b42929c4dab
VLukiantsev
marked this pull request as ready for review
September 28, 2026 15:40
pachopepe
reviewed
Sep 29, 2026
…nd git fetch Comment link: #1008 (comment) Comment link: #1008 (comment) Comment link: #1008 (comment) Comment link: #1008 (comment)
pachopepe
previously approved these changes
Sep 30, 2026
resolve_test_scope.py now falls back to GITHUB_EVENT_NAME, GITHUB_REF, GITHUB_BASE_REF, GITHUB_OUTPUT and the GITHUB_EVENT_PATH payload (pull_request/merge_group base and head SHAs, preferred over the synthetic GITHUB_SHA merge commit) when CLI flags are omitted. Explicit flags win. Drops the duplicated env blocks and flag plumbing from the three composite action steps. Comment link: #1008 (comment) TAG=agy CONV=a6d09aaa-980b-48ad-9a26-4b42929c4dab
Run the real-repo gh-stack test with GIT_CONFIG_GLOBAL=/dev/null, GIT_CONFIG_NOSYSTEM=1, fixed author/committer identity, and gpgsign disabled so host signing/config cannot break local runs. TAG=agy CONV=a6d09aaa-980b-48ad-9a26-4b42929c4dab
pachopepe
approved these changes
Oct 2, 2026
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.
Summary
Eliminates CI runner queue starvation during stacked PR updates and multi-PR bursts by cancelling superseded
pull_requestunit test runs and fixing stacked-PR diff scoping against synthetic merge commits and stale base SHAs inresolve_test_scope.py.Motivation & Context
Recent multi-PR bursts and
gh stackupdates experienced runner pickup delays due to two workflow and test-scoping issues:.github/workflows/unit-tests.ymllacked aconcurrencygroup, allowing superseded force-pushes on the same PR branch to hold ARC runner pods..github/actions/run-backend-testsand.github/actions/run-ui-testsdiffedPR_BASE_SHA...GITHUB_SHA(refs/pull/<num>/merge), causing stacked PR updates to include upstreammaincommits from the previous synthetic merge commit (or stale pre-rebase parent SHAs) and unnecessarily run full UI and Backend test suites on isolated backend PRs.Key Changes
.github/workflows/unit-tests.yml): Addedconcurrency(group: ${{ github.workflow }}-v2-${{ github.event.pull_request.number || github.head_ref || github.run_id }},cancel-in-progress: ${{ github.event_name == 'pull_request' }}) to cancel superseded PR runs while never cancellingmainpush runs..github/actions/run-backend-tests/action.yml,.github/actions/run-ui-tests/action.yml): Fetchorigin/mainandorigin/${PR_BASE_REF}, passPR_HEAD_SHA(withmerge_groupfallbacks) as--headinstead ofGITHUB_SHA, and passPR_BASE_REFas--base-branch.tools/benchmark/resolve_test_scope.py,tools/benchmark/resolve_test_scope_test.py):_get_merge_baseandresolve_effective_base_refto prioritize liveorigin/<base_branch>over stale webhookbase_ref, resolve the closest ancestor across target andorigin/mainviagit merge-base --independent, and excludehead_refself-matches.HEADinget_git_modified_filesifPR_HEAD_SHAis missing locally after a force-push race.workflow_dispatch/scheduleonmainand whenfilesis empty ([])..bazelrc,.bazelversion,.github/workflows/unit-tests.yml,.github/actions/run-backend-tests/action.yml, andtools/benchmark/resolve_test_scope.pytoCORE_INFRA_FILES, classified.github/actions/run-ui-tests/ashas_ui = True, and included non-code backend testdata packages indiscover_bazel_rdeps_targets.docs/ci_runner_infrastructure.md,tools/benchmark/README.md): Documented workflow concurrency and stacked-PR merge-base resolution.Assumptions
None — requirements were fully specified.Verification & Testing
./.githooks/pre-commit,bazel test //tools/benchmark/...,bazel coverage --combined_report=lcov //tools/benchmark:resolve_test_scope_test//tools/benchmark/...(including 6 new unit tests inresolve_test_scope_test.pycovering stacked PR merge-base selection, commit-rewind down-stack handling,head_refself-match exclusion, missingPR_HEAD_SHAretry,workflow_dispatch/empty-diff fail-open, and non-code testdatardepsscoping) and all pre-commit linters pass cleanly.Related Issues / Bugs
N/A
TAG=agy
CONV=a6d09aaa-980b-48ad-9a26-4b42929c4dab