Skip to content

Commit 1248c83

Browse files
committed
ci(ui-merge): Test UI runs only the tests a pull request or merge group affects
Wire ui/scripts/select-affected-tests.mjs into the root test-ui job and teach the selector the changes outside ui/ that every ui test runs through (a non-documentation workspace-root file, patches/, one of the monorepo packages the ui tests read built). Claude-Session: https://claude.ai/code/session_015oJuSyTy7Y3cm91KJMEW2m Co-Authored-By: Claude <noreply@anthropic.com>
1 parent c475b45 commit 1248c83

2 files changed

Lines changed: 227 additions & 23 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 87 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3292,11 +3292,16 @@ jobs:
32923292
# vitest root, so the suite must run with cwd = ui/ (it does, below); run
32933293
# from this repo's root every key misses and every file weighs the median.
32943294
#
3295-
# Every event runs the whole suite. objectui's affected-test selector
3296-
# (ui/scripts/select-affected-tests.mjs) reads `git diff --name-only`, which
3297-
# prints paths relative to the GIT root — `ui/packages/...` here — against a
3298-
# package graph spelled relative to the objectui root, so it is not wired
3299-
# until objectui's repo-root lookups are migrated (sub-issue 2 of the epic).
3295+
# ── Test UI: the two speeds (#25868) ────────────────────────────────────
3296+
# `push`, `schedule` and `workflow_dispatch` run the whole suite: the full
3297+
# battery. `pull_request` and `merge_group` run the files
3298+
# ui/scripts/select-affected-tests.mjs selects -- the ui packages the change
3299+
# sits in and their dependents, plus the tests the package graph cannot
3300+
# place, which run on every change -- or the whole suite when the change
3301+
# reaches something every ui test runs through, outside ui/ included (a
3302+
# non-documentation root file, `patches/`, one of the seven packages the
3303+
# `ui_consumers` filter names). The selector's header states the rules and
3304+
# what it does not see; the full battery on `main` is the backstop.
33003305
test-ui:
33013306
name: Test UI (${{ matrix.shard }}/16)
33023307
needs: filter
@@ -3313,6 +3318,10 @@ jobs:
33133318
steps:
33143319
- name: Checkout repository
33153320
uses: actions/checkout@v7
3321+
with:
3322+
# The select step below diffs against the base commit, which needs
3323+
# the base's history in the clone.
3324+
fetch-depth: 0
33163325

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

‎ui/scripts/select-affected-tests.mjs‎

Lines changed: 140 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,11 @@
6868
* one);
6969
* - an affected package that a BROAD root setup file imports by name:
7070
* the light DOM setup installs the i18n catalogues into every `dom`
71-
* test, an edge no test file states.
71+
* test, an edge no test file states;
72+
* - nested as `ui/` of the objectstack monorepo, a change OUTSIDE the
73+
* tree that its tests install or build from: a non-documentation
74+
* file at the workspace root, `patches/`, or a monorepo package the
75+
* tree depends on (`outsideTreeReasons` says which).
7276
*
7377
* 4. THE HEAVY SETUP. The setup of the `dom-heavy` project registers four
7478
* packages' components into every test of that project and of each
@@ -110,7 +114,13 @@ import { execFileSync } from 'node:child_process';
110114
import { fileURLToPath } from 'node:url';
111115

112116
import { isEntrypoint } from './invoked-as.mjs';
113-
import { parseWorkspaceGlobs, workspaceGlobs } from './workspace-root.mjs';
117+
import {
118+
nestedPrefix,
119+
parseWorkspaceGlobs,
120+
workspaceGlobs,
121+
workspaceManifestPath,
122+
workspaceRoot,
123+
} from './workspace-root.mjs';
114124

115125
export const REPO_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..');
116126

@@ -141,6 +151,9 @@ const DISK_SPECIFIERS = new Set([
141151
'tinyglobby',
142152
]);
143153

154+
const DEPENDENCY_FIELDS = ['dependencies', 'devDependencies', 'peerDependencies', 'optionalDependencies'];
155+
const RUNTIME_DEPENDENCY_FIELDS = DEPENDENCY_FIELDS.filter((field) => field !== 'devDependencies');
156+
144157
const RESOLVE_EXTENSIONS = ['', '.ts', '.tsx', '.mts', '.cts', '.js', '.jsx', '.mjs', '.cjs', '.json'];
145158

146159
const toPosix = (p) => p.split(path.sep).join('/');
@@ -154,21 +167,19 @@ const toPosix = (p) => p.split(path.sep).join('/');
154167
export { parseWorkspaceGlobs };
155168

156169
/**
157-
* Every workspace package: its name, its repository-relative directory and the
158-
* workspace packages it names in any dependency field.
170+
* The named manifests under `globs` (`dir` or `dir/*`, already parsed),
171+
* relative to `base`.
159172
*
160-
* @param {string} root
161-
* @returns {Array<{ name: string, dir: string, deps: string[] }>}
173+
* @param {string} base
174+
* @param {string[]} globs
175+
* @returns {Array<{ name: string, dir: string, pkg: Record<string, any> }>}
162176
*/
163-
export function listWorkspacePackages(root = REPO_ROOT) {
164-
// Relative to `root` in both layouts: the objectui root's own file, or the
165-
// monorepo root's `ui/...` globs with the prefix stripped (objectstack#25723).
166-
const globs = workspaceGlobs(root);
177+
function readManifests(base, globs) {
167178
const dirs = [];
168179
for (const glob of globs) {
169180
if (glob.endsWith('/*')) {
170181
const parent = glob.slice(0, -2);
171-
const abs = path.join(root, parent);
182+
const abs = path.join(base, parent);
172183
if (!fs.existsSync(abs)) continue;
173184
for (const entry of fs.readdirSync(abs, { withFileTypes: true })) {
174185
if (entry.isDirectory()) dirs.push(`${parent}/${entry.name}`);
@@ -179,16 +190,30 @@ export function listWorkspacePackages(root = REPO_ROOT) {
179190
}
180191
const manifests = [];
181192
for (const dir of dirs.sort()) {
182-
const file = path.join(root, dir, 'package.json');
193+
const file = path.join(base, dir, 'package.json');
183194
if (!fs.existsSync(file)) continue;
184195
const pkg = JSON.parse(fs.readFileSync(file, 'utf8'));
185196
if (typeof pkg.name !== 'string') continue;
186197
manifests.push({ name: pkg.name, dir, pkg });
187198
}
199+
return manifests;
200+
}
201+
202+
/**
203+
* Every workspace package: its name, its repository-relative directory and the
204+
* workspace packages it names in any dependency field.
205+
*
206+
* @param {string} root
207+
* @returns {Array<{ name: string, dir: string, deps: string[] }>}
208+
*/
209+
export function listWorkspacePackages(root = REPO_ROOT) {
210+
// Relative to `root` in both layouts: the objectui root's own file, or the
211+
// monorepo root's `ui/...` globs with the prefix stripped (objectstack#25723).
212+
const manifests = readManifests(root, workspaceGlobs(root));
188213
const names = new Set(manifests.map((m) => m.name));
189214
return manifests.map(({ name, dir, pkg }) => {
190215
const deps = new Set();
191-
for (const field of ['dependencies', 'devDependencies', 'peerDependencies', 'optionalDependencies']) {
216+
for (const field of DEPENDENCY_FIELDS) {
192217
for (const dep of Object.keys(pkg[field] ?? {})) if (names.has(dep) && dep !== name) deps.add(dep);
193218
}
194219
return { name, dir, deps: [...deps].sort() };
@@ -441,6 +466,100 @@ export function changedFiles(root, base, head = 'HEAD') {
441466
.filter(Boolean);
442467
}
443468

469+
/**
470+
* The changed files OUTSIDE this tree, relative to the workspace root -- the
471+
* monorepo's own files when the tree is nested (`ui/` of objectstack), and
472+
* nothing standalone, where the tree is the whole repository. `changedFiles`
473+
* leaves them out by construction (`--relative`), so this is the second half
474+
* of the same diff: git runs at the workspace root with `--relative` again,
475+
* and the tree's own prefix is dropped from what it prints.
476+
*/
477+
export function changedOutsideTree(root, base, head = 'HEAD') {
478+
const prefix = nestedPrefix(root);
479+
if (!prefix) return [];
480+
return changedFiles(workspaceRoot(root), base, head).filter((f) => !f.startsWith(prefix));
481+
}
482+
483+
/**
484+
* Why a change outside this tree reaches every test in it, one reason per
485+
* file that does (an empty list means none does). The tree's tests install
486+
* from the workspace root, so its install and toolchain inputs are this tree's
487+
* too, and they import the monorepo's own packages the tree depends on from
488+
* their built output (the CI job builds them first):
489+
*
490+
* - a workspace-root file that is not documentation (the one lockfile, the
491+
* workspace file, the root manifest, `turbo.json`, a root `tsconfig`), or
492+
* anything under `patches/` -- the rule item 3 states for this tree's own
493+
* root, applied to the root it installs from;
494+
* - a file in a workspace package outside this tree that a package of this
495+
* tree depends on (any dependency field), or that such a package depends
496+
* on at run time (`dependencies`, `peerDependencies`,
497+
* `optionalDependencies`; an outer package's devDependencies are its own
498+
* test tooling, not its built output). On objectstack that is the seven
499+
* packages the `ui_consumers` filter of `ci.yml` names. Which of this
500+
* tree's tests read the changed export is not something the package graph
501+
* can say, so every test runs.
502+
*
503+
* Every other path -- another package of the monorepo, its docs, scripts and
504+
* workflows -- reaches this tree's tests only through the always-on set, as a
505+
* file outside every package does inside it.
506+
*
507+
* @param {{ root?: string, outside: string[] }} input `outside` relative to the workspace root
508+
* @returns {string[]}
509+
*/
510+
export function outsideTreeReasons({ root = REPO_ROOT, outside }) {
511+
if (outside.length === 0) return [];
512+
const prefix = nestedPrefix(root);
513+
const wsRoot = workspaceRoot(root);
514+
const own = new Set(listWorkspacePackages(root).map((p) => p.name));
515+
const globs = parseWorkspaceGlobs(fs.readFileSync(workspaceManifestPath(root), 'utf8')).filter(
516+
(g) => !g.startsWith(prefix) && `${g}/` !== prefix,
517+
);
518+
const outer = readManifests(wsRoot, globs).filter((m) => !own.has(m.name));
519+
const outerNames = new Set(outer.map((m) => m.name));
520+
const depsOf = (pkg, fields = DEPENDENCY_FIELDS) => fields.flatMap((field) => Object.keys(pkg[field] ?? {}));
521+
522+
// The outer packages this tree reaches, each with the tree package that
523+
// reaches it first (named in the reason).
524+
const reachedBy = new Map();
525+
const queue = [];
526+
for (const m of readManifests(root, workspaceGlobs(root))) {
527+
for (const dep of depsOf(m.pkg)) {
528+
if (outerNames.has(dep) && !reachedBy.has(dep)) {
529+
reachedBy.set(dep, m.name);
530+
queue.push(dep);
531+
}
532+
}
533+
}
534+
const byName = new Map(outer.map((m) => [m.name, m]));
535+
while (queue.length) {
536+
const name = queue.shift();
537+
for (const dep of depsOf(byName.get(name).pkg, RUNTIME_DEPENDENCY_FIELDS)) {
538+
if (outerNames.has(dep) && !reachedBy.has(dep)) {
539+
reachedBy.set(dep, reachedBy.get(name));
540+
queue.push(dep);
541+
}
542+
}
543+
}
544+
545+
const reasons = [];
546+
for (const rel of outside) {
547+
if (!rel.includes('/') && !ROOT_DOC_FILE.test(rel)) {
548+
reasons.push(`${rel}: a workspace-root file that is not documentation (install, toolchain or config input of this tree)`);
549+
continue;
550+
}
551+
if (rel.startsWith('patches/')) {
552+
reasons.push(`${rel}: a dependency patch changes what is installed for every test`);
553+
continue;
554+
}
555+
const owner = owningPackage(rel, outer);
556+
if (owner && reachedBy.has(owner.name)) {
557+
reasons.push(`${rel}: ${owner.name} changed, and this tree's tests read its built output (through ${reachedBy.get(owner.name)})`);
558+
}
559+
}
560+
return reasons;
561+
}
562+
444563
/** The root vitest config and every `vitest.setup*` file beside it. */
445564
export function rootTestConfigFiles(tracked) {
446565
return tracked.filter((f) => ROOT_TEST_CONFIG.includes(f) || /^vitest\.setup[^/]*\.[cm]?[jt]sx?$/.test(f)).sort();
@@ -474,10 +593,12 @@ export function readHeavyProject(configText) {
474593
/**
475594
* The verdict for one change.
476595
*
477-
* @param {{ root?: string, changed: string[], tracked?: string[] }} input
596+
* @param {{ root?: string, changed: string[], outside?: string[], tracked?: string[] }} input
597+
* `changed` relative to `root`; `outside` the changed files outside the tree,
598+
* relative to the workspace root (`changedOutsideTree`)
478599
* @returns {{ mode: 'full', reasons: string[] } | { mode: 'affected', changedPackages: string[], affectedPackages: string[], heavyEdge: string[], alwaysOn: Array<{ file: string, why: string }>, filters: string[], testFileCount: number, selectedTestFileCount: number }}
479600
*/
480-
export function selectAffectedTests({ root = REPO_ROOT, changed, tracked = trackedFiles(root) }) {
601+
export function selectAffectedTests({ root = REPO_ROOT, changed, outside = [], tracked = trackedFiles(root) }) {
481602
const packages = listWorkspacePackages(root);
482603
const names = new Set(packages.map((p) => p.name));
483604
const { dependencyClosure, dependentClosure } = packageClosures(packages);
@@ -537,7 +658,7 @@ export function selectAffectedTests({ root = REPO_ROOT, changed, tracked = track
537658
})
538659
.map((f) => `${path.posix.dirname(f)}/`);
539660

540-
const reasons = [];
661+
const reasons = outsideTreeReasons({ root, outside });
541662
const changedPackages = new Set();
542663
for (const rel of changed) {
543664
const base = rel.split('/').pop() ?? rel;
@@ -658,8 +779,10 @@ function main(argv) {
658779
return 2;
659780
}
660781
const changed = changedFiles(REPO_ROOT, base);
782+
const outside = changedOutsideTree(REPO_ROOT, base);
661783
console.log(`changed files against the merge base of ${base}: ${changed.length}`);
662-
const verdict = selectAffectedTests({ changed });
784+
if (nestedPrefix(REPO_ROOT)) console.log(`changed files outside ${nestedPrefix(REPO_ROOT)}: ${outside.length}`);
785+
const verdict = selectAffectedTests({ changed, outside });
663786
if (verdict.mode === 'full') {
664787
console.log('FULL RUN — this change reaches something every test runs through:');
665788
for (const r of verdict.reasons) console.log(` - ${r}`);

0 commit comments

Comments
 (0)