8000 fix(ci): harden stacked PR test scoping and add unit-tests concurrency by VLukiantsev · Pull Request #1008 · google/xmc · GitHub
[go: up one dir, main page]

Skip to content

fix(ci): harden stacked PR test scoping and add unit-tests concurrency - #1008

Merged
VLukiantsev merged 7 commits into
mainfrom
fix/ci-action-scalability-and-latency
Oct 2, 2026
Merged

VLukiantsev merged 7 commits into
mainfrom
fix/ci-action-scalability-and-latency

Conversation

@VLukiantsev
@VLukiantsev VLukiantsev commented Sep 28, 2026 •
Copy link
Copy Markdown
Collaborator

Summary

Eliminates CI runner queue starvation during stacked PR updates and multi-PR bursts by cancelling superseded pull_request unit test runs and fixing stacked-PR diff scoping against synthetic merge commits and stale base SHAs in resolve_test_scope.py.

Motivation & Context

Recent multi-PR bursts and gh stack updates experienced runner pickup delays due to two workflow and test-scoping issues:

  1. .github/workflows/unit-tests.yml lacked a concurrency group, allowing superseded force-pushes on the same PR branch to hold ARC runner pods.
  2. .github/actions/run-backend-tests and .github/actions/run-ui-tests diffed PR_BASE_SHA...GITHUB_SHA (refs/pull/<num>/merge), causing stacked PR updates to include upstream main commits 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

  • Workflow Concurrency (.github/workflows/unit-tests.yml): Added concurrency (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 cancelling main push runs.
  • Composite Actions (.github/actions/run-backend-tests/action.yml, .github/actions/run-ui-tests/action.yml): Fetch origin/main and origin/${PR_BASE_REF}, pass PR_HEAD_SHA (with merge_group fallbacks) as --head instead of GITHUB_SHA, and pass PR_BASE_REF as --base-branch.
  • Test Scope Resolver (tools/benchmark/resolve_test_scope.py, tools/benchmark/resolve_test_scope_test.py):
    • Added _get_merge_base and resolve_effective_base_ref to prioritize live origin/<base_branch> over stale webhook base_ref, resolve the closest ancestor across target and origin/main via git merge-base --independent, and exclude head_ref self-matches.
    • Added fallback retry against HEAD in get_git_modified_files if PR_HEAD_SHA is missing locally after a force-push race.
    • Added fail-open full-suite execution on workflow_dispatch/schedule on main and when files is empty ([]).
    • Added .bazelrc, .bazelversion, .github/workflows/unit-tests.yml, .github/actions/run-backend-tests/action.yml, and tools/benchmark/resolve_test_scope.py to CORE_INFRA_FILES, classified .github/actions/run-ui-tests/ as has_ui = True, and included non-code backend testdata packages in discover_bazel_rdeps_targets.
  • Documentation (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

  • Commands Executed: ./.githooks/pre-commit, bazel test //tools/benchmark/..., bazel coverage --combined_report=lcov //tools/benchmark:resolve_test_scope_test
  • Coverage & Test Status: All 7 test targets in //tools/benchmark/... (including 6 new unit tests in resolve_test_scope_test.py covering stacked PR merge-base selection, commit-rewind down-stack handling, head_ref self-match exclusion, missing PR_HEAD_SHA retry, workflow_dispatch/empty-diff fail-open, and non-code testdata rdeps scoping) and all pre-commit linters pass cleanly.

Related Issues / Bugs

N/A

TAG=agy
CONV=a6d09aaa-980b-48ad-9a26-4b42929c4dab

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
@gemini-code-assist gemini-code-assist Bot left a comment
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

7438

Comment thread .github/actions/run-ui-tests/action.yml Resolved
Comment thread tools/benchmark/resolve_test_scope.py Resolved
Comment thread tools/benchmark/resolve_test_scope.py Resolved
- 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
VLukiantsev marked this pull request as ready for review September 28, 2026 15:40
@github-actions
github-actions Bot requested a review from pachopepe September 28, 2026 15:41
Comment thread tools/benchmark/resolve_test_scope.py Resolved
Comment thread tools/benchmark/resolve_test_scope.py Resolved
Comment thread tools/benchmark/resolve_test_scope.py Resolved
pachopepe
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
@VLukiantsev
VLukiantsev requested a review from pachopepe October 2, 2026 08:04
@VLukiantsev
VLukiantsev added this pull request to the merge queue Oct 2, 2026
Merged via the queue into main with commit b017a89 Oct 2, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

0