Skip to content

Commit cb0a2fc

Browse files
ci(ui-merge): Test UI runs only the ui tests a pull request or merge group affects — the root ci.yml wires ui/scripts/select-affected-tests.mjs (#25882)
<!-- claude-projects-attribution: {"github_login":"os-tesla"} --> _Requested by **Tesla** · [project thread](https://claude.ai/code/project/chan_01AXsQkLFKRdQCqQ3RTr6Jao?thread=cmsg_01AXsQkLFKRdQCqQ3RTr6Jao47d4XTXUAPA2geTEJykbxK)_ Fixes #25868 Part of #25677 Clause-②: no Metadata-protocol: none ## 维护者速读(草稿) - **改了什么:** 根 `ci.yml` 的 Test UI(16 个分片)在 pull request 和 merge queue 上不再每次跑全量 ui 测试,而是先用 objectui 原有的 `ui/scripts/select-affected-tests.mjs` 选出本次改动影响到的测试文件,只跑这些。push 到 main、每小时的 schedule 和手动触发仍然跑全量。 - **为什么改:** 合并后每个 ui PR 都跑 16 个全量分片(每片 7–14 分钟,约 190 runner-min)。在 objectui 仓库时,小 PR 只跑受影响的测试,CI 3–8 分钟。 - **风险与代价(含回滚):** 选择器漏掉的测试,要等 main 上的全量运行(每小时一次)才会发现,这是 objectui 原来就接受的取舍。选择器出错、拿不到 diff 基点时一律回退到全量,不会跳过。ui/ 之外影响所有 ui 测试的改动(根目录的 lockfile、workspace、turbo.json 等非文档文件,`patches/`,以及 ui 测试读取其构建产物的 7 个 @objectstack 包)一律全量。回滚:revert 本 PR 即恢复每次全量。 - **席位意见:** - **你要做的:** 这是 Tier H(改了 `.github/workflows/ci.yml`),需要你批准后由席位落地。 ## What changed 1. **`.github/workflows/ci.yml`, job `test-ui`** — objectui's two-step wiring, ported: - `Select the tests this change affects` (id `select`): `pull_request` and `merge_group` run the selector; every other event writes `mode=full`. Fails open: no base, a non-zero exit or no verdict line means `mode=full`, never a skip. The exit status is captured beside the output, not through a command substitution. - The PR diff base is `merge-base(origin/BASE_REF, HEAD)`, the same base Test Core uses, not the payload's frozen `base.sha`. With the frozen sha, every commit `main` gained while the PR was open (a lockfile bump among them) would be charged to the PR and turn it into a full run. A merge group diffs from `merge_group.base_sha`. - `Run the ui tests (shard N/16)` reads the output's VALUE (`affected` / `full`). On `affected` it runs `pnpm --dir ui exec vitest run --shard=N/16 ... FILTERS` with the filters relative to `ui/` (the vitest root), as positionals with no separator, which is the only form `ui/scripts/vitest-invocation-guard.mjs` accepts. On `full` it runs the command it ran before. - The checkout takes `fetch-depth: 0` so the base's history is there (Test Core and Type Check UI do the same). 2. **`ui/scripts/select-affected-tests.mjs`** — the git paths already went through `--relative` (#25728). Its own diff dropped every changed file outside `ui/`, though, so a lockfile bump, a root `tsconfig.json` or a `packages/spec` change selected only the always-on set (measured below). New: `changedOutsideTree()` lists the monorepo's own changes (git at the workspace root, `--relative` again, the `ui/` prefix dropped), and `outsideTreeReasons()` turns them into full-run reasons: - a workspace-root file that is not documentation, or anything under `patches/` (the rule the selector already applied to objectui's own root, now applied to the root this tree installs from); - a file in a monorepo package that a ui package depends on (any field), or that such a package depends on at run time (`dependencies` / `peerDependencies` / `optionalDependencies`). On this tree that set is exactly the seven packages the `ui_consumers` filter names: spec, client, core, types, lint, formula, sdui-parser. - Everything else outside `ui/` (other packages, docs, `.changeset/`, `.github/`, `scripts/`) reaches ui tests only through the always-on set, as before. 3. **`ui/scripts/__tests__/select-affected-tests-wiring.test.ts`** now reads the root `ci.yml` (located through `workspaceRoot()`) and runs both steps there at the monorepo root. A `git` stand-in answers `merge-base` as HEAD and records the call, so the legs assert the step asks for `refs/remotes/origin/main` and `HEAD`. A new leg covers a failing merge-base: full run. The controls are unchanged. 4. **`ui/scripts/__tests__/select-affected-tests.test.ts`**: the `ci.yml` wiring block now reads the root job (including `fetch-depth: 0` and the 16-wide matrix). A new nested-layout fixture (`ui/` under a parent workspace) covers each outside-tree rule in both directions. One case runs real git in a scratch repo to show that `changedFiles` and `changedOutsideTree` split one diff between them. The turbo parity case (`agrees with turbo on who depends on whom`) was already green in the merged layout on `main`. It is unchanged and still green. `ui/.github/workflows/ci.yml` (objectui's copy, which runs nowhere) is left as it is; the governance-workflow audit owns it. ## Measured Selector verdicts on this tree (`c475b45b` + this diff), from scratch commits in a detached worktree: | change | verdict | selected test files | |---|---|---| | nothing under `ui/` (this PR's own shape: workflow + `ui/scripts`) | affected | 686 of 5810 | | one leaf, `ui/packages/plugin-gantt/src/index.ts` | affected | 1036 of 5810 | | `ui/packages/plugin-kanban/src/KanbanColumnSummary.ts` (reaches app-shell) | affected | 2647 of 5810 | | `pnpm-lock.yaml` + `packages/spec/src/index.ts`, before this diff | affected | 686 of 5810 (the hole) | | the same, after | full | — | Weighted by `ui/scripts/test-shard-timings.json` (the sequencer's per-file dataset): always-on alone is 8.8% of the summed file time, plugin-gantt 20.2%, plugin-kanban 54.2%, `ui/packages/fields` 88.9%. Each shard still pays its fixed cost (checkout, install, building the seven packages). The before/after shard wall times are read from this PR's own Test UI run, which is the always-on shape, and posted on the card. ## Verification - `pnpm exec vitest run scripts/__tests__/select-affected-tests.test.ts scripts/__tests__/select-affected-tests-wiring.test.ts` (in `ui/`): 2 files, 49 tests, all pass. - Whole `ui/scripts/__tests__` suite (`pnpm exec vitest run scripts/__tests__` in `ui/`, head `bc79c8aa`): 190 files, 5608 passed, 2 skipped, 1 failed. The failure is `check-pre-install-import-graph.test.ts` > "finds no pre-install gate reaching a package" (`unresolved: scripts/objectstack-pin.mjs`, which the cutover deleted and `ui/.github/workflows/*` still name). It fails the same way on `c475b45b` with this diff's four files restored (37 tests, 1 failed), so it is red on `main` already and not this PR's. It is reported to the dispatcher. That file is in the always-on set, so it keeps running on every affected Test UI run. - `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --ran`: 63 derived, 59 run with exit 0, 4 NOT MEASURED (exit 3, built dists absent): `check:dts-closure`, `check:dual-build-cjs-loads`, `check:lean-entry-closure`, `check:sourcemap-no-sources-content`. These are Build Core's guards, and Build Core runs them on this PR (`ci.yml` is in the `core` filter). ## Acceptance notes - A spec-only PR still runs no Test UI (the `ui` filter decides that, unchanged). On a merge group, Test UI runs today for every entry, and this diff makes it full for an entry that touches the seven packages or a root install input, and affected otherwise. #25867 (merge_group ui path diff) is the card that decides whether such an entry runs Test UI at all. - Vitest import overhead (card's optional note): not measured here. --- _Generated by [Claude Code](https://claude.ai/code/session_015oJuSyTy7Y3cm91KJMEW2m)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 487056b commit cb0a2fc

5 files changed

Lines changed: 452 additions & 65 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 87 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3315,11 +3315,16 @@ jobs:
33153315
# vitest root, so the suite must run with cwd = ui/ (it does, below); run
33163316
# from this repo's root every key misses and every file weighs the median.
33173317
#
3318-
# Every event runs the whole suite. objectui's affected-test selector
3319-
# (ui/scripts/select-affected-tests.mjs) reads `git diff --name-only`, which
3320-
# prints paths relative to the GIT root — `ui/packages/...` here — against a
3321-
# package graph spelled relative to the objectui root, so it is not wired
3322-
# until objectui's repo-root lookups are migrated (sub-issue 2 of the epic).
3318+
# ── Test UI: the two speeds (#25868) ────────────────────────────────────
3319+
# `push`, `schedule` and `workflow_dispatch` run the whole suite: the full
3320+
# battery. `pull_request` and `merge_group` run the files
3321+
# ui/scripts/select-affected-tests.mjs selects -- the ui packages the change
3322+
# sits in and their dependents, plus the tests the package graph cannot
3323+
# place, which run on every change -- or the whole suite when the change
3324+
# reaches something every ui test runs through, outside ui/ included (a
3325+
# non-documentation root file, `patches/`, one of the seven packages the
3326+
# `ui_consumers` filter names). The selector's header states the rules and
3327+
# what it does not see; the full battery on `main` is the backstop.
33233328
test-ui:
33243329
name: Test UI (${{ matrix.shard }}/16)
33253330
needs: filter
@@ -3336,6 +3341,10 @@ jobs:
33363341
steps:
33373342
- name: Checkout repository
33383343
uses: actions/checkout@v7
3344+
with:
3345+
# The select step below diffs against the base commit, which needs
3346+
# the base's history in the clone.
3347+
fetch-depth: 0
33393348

33403349
- name: Setup Node.js
33413350
uses: actions/setup-node@v7
@@ -3386,13 +3395,85 @@ jobs:
33863395
--filter=@objectstack/sdui-parser... \
33873396
--filter=@objectstack/types...
33883397
3398+
# Fails OPEN: a selector that exits non-zero, prints no verdict, or has
3399+
# no base to diff against means the whole suite, never a skip. The exit
3400+
# status is captured beside the output, not through a command
3401+
# substitution. A pull request diffs from merge-base(origin/<base ref>,
3402+
# HEAD), never the payload's `base.sha`, which is frozen when the PR is
3403+
# opened: everything `main` gained since would be charged to this PR,
3404+
# and a lockfile change among it would make every long-open PR a full
3405+
# run (Test Core's "WHERE THE AFFECTED DIFF STARTS" above is the same
3406+
# fact). A merge group diffs from its `base_sha`. Every shard computes
3407+
# the same selection from the same checkout, so `--shard` partitions
3408+
# one set.
3409+
- name: Select the tests this change affects
3410+
id: select
3411+
env:
3412+
EVENT: ${{ github.event_name }}
3413+
PR_BASE_REF: ${{ github.event.pull_request.base.ref }}
3414+
QUEUE_BASE: ${{ github.event.merge_group.base_sha }}
3415+
run: |
3416+
base=''
3417+
case "$EVENT" in
3418+
pull_request)
3419+
if [ -n "$PR_BASE_REF" ]; then
3420+
base=$(git merge-base "refs/remotes/origin/$PR_BASE_REF" HEAD) || base=''
3421+
fi
3422+
;;
3423+
merge_group) base="$QUEUE_BASE" ;;
3424+
*)
3425+
echo 'mode=full' >> "$GITHUB_OUTPUT"
3426+
echo "A $EVENT run is the full battery: every ui test file, sixteen shards."
3427+
exit 0
3428+
;;
3429+
esac
3430+
status=0
3431+
if [ -z "$base" ]; then
3432+
echo "::warning::no diff base on this $EVENT event; running the full suite rather than guessing."
3433+
status=2
3434+
else
3435+
echo "Diff base: $base"
3436+
node ui/scripts/select-affected-tests.mjs --base "$base" --out "$RUNNER_TEMP/affected-tests.txt" \
3437+
> "$RUNNER_TEMP/select.out" || status=$?
3438+
cat "$RUNNER_TEMP/select.out"
3439+
fi
3440+
verdict=''
3441+
if [ "$status" -eq 0 ]; then
3442+
verdict=$(tail -n 1 "$RUNNER_TEMP/select.out")
3443+
fi
3444+
case "$verdict" in
3445+
mode=affected | mode=full) ;;
3446+
*)
3447+
echo "::warning::the affected-test selector reached no verdict (exit $status); running the full suite rather than skipping silently."
3448+
verdict='mode=full'
3449+
;;
3450+
esac
3451+
echo "$verdict" >> "$GITHUB_OUTPUT"
3452+
3453+
# `$MODE` is the VALUE of the select step's output `mode`: its
3454+
# `mode=affected` line is GitHub's `key=value` form, so what arrives here
3455+
# is `affected` or `full`, never the line itself.
3456+
# ui/scripts/__tests__/select-affected-tests-wiring.test.ts runs both
3457+
# steps as written, so neither end can be renamed alone. The filters are
3458+
# paths relative to ui/, the vitest root, passed as positionals with no
3459+
# `--`, the only spelling ui/scripts/vitest-invocation-guard.mjs admits.
3460+
#
33893461
# objectui's full-battery runs also pass its per-file timings reporter
33903462
# and upload the result for the weekly dataset refresh. Both belong to
33913463
# that refresh workflow, which is in the governance-workflow audit
33923464
# (sub-issue 4 of the epic); until it is ported the committed dataset
33933465
# is what the sequencer reads.
33943466
- name: Run the ui tests (shard ${{ matrix.shard }}/16)
3395-
run: pnpm --dir ui run test --shard=${{ matrix.shard }}/16 --reporter=default --reporter=github-actions
3467+
env:
3468+
MODE: ${{ steps.select.outputs.mode }}
3469+
run: |
3470+
if [ "$MODE" = 'affected' ]; then
3471+
mapfile -t filters < "$RUNNER_TEMP/affected-tests.txt"
3472+
echo "Affected speed: ${#filters[@]} vitest filter(s), shard ${{ matrix.shard }} of 16."
3473+
pnpm --dir ui exec vitest run --shard=${{ matrix.shard }}/16 --reporter=default --reporter=github-actions "${filters[@]}"
3474+
else
3475+
pnpm --dir ui run test --shard=${{ matrix.shard }}/16 --reporter=default --reporter=github-actions
3476+
fi
33963477
33973478
- name: Save Turbo cache (main only)
33983479
if: always() && github.event_name == 'push' && matrix.shard == 1

‎ui/scripts/__tests__/helpers/root-ci.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,11 @@
1111
* this module. It never reads a copy under `ui/.github/`: nothing runs one.
1212
*
1313
* What the port did not carry (the coverage lane, the docs job, the shard
14-
* aggregate, the affected-test and type-check-scope selectors, the timings
15-
* upload) has no job here, and the pins on those retired with a disposition
16-
* beside them rather than being pointed at a job that does something else.
14+
* aggregate, the type-check-scope selector, the timings upload) has no job
15+
* here, and the pins on those retired with a disposition beside them rather
16+
* than being pointed at a job that does something else. The affected-test
17+
* selector was wired into `test-ui` later (objectstack#25868), and its pins
18+
* read that job.
1719
*/
1820
import fs from 'node:fs';
1921
import path from 'node:path';

0 commit comments

Comments
 (0)