From 0a2ccf998f64cd990c65fe7cbe1458a2e144d523 Mon Sep 17 00:00:00 2001 From: Juan Cruz Fortunatti Date: Thu, 1 Oct 2026 21:53:49 +0200 Subject: [PATCH 1/2] fix(ci): the bake tests that exercise the renderer run when the renderer changes --- .github/scripts/ci/affected-tests.mts | 35 ++++++++++++++++--- .github/scripts/ci/affected-tests.test.mts | 39 +++++++++++++++++----- .github/workflows/universe.yml | 6 +++- CONTRIBUTING.md | 2 +- 4 files changed, 66 insertions(+), 16 deletions(-) diff --git a/.github/scripts/ci/affected-tests.mts b/.github/scripts/ci/affected-tests.mts index 09e4482da3f..55c2f138453 100644 --- a/.github/scripts/ci/affected-tests.mts +++ b/.github/scripts/ci/affected-tests.mts @@ -9,7 +9,28 @@ import { resolve } from 'node:path'; import { pathToFileURL } from 'node:url'; export interface Workspace { readonly directory: string; readonly name: string; readonly dependencies: readonly string[]; } -export interface AffectedTests { readonly packages: 'all' | readonly string[]; readonly site: boolean; } +export interface AffectedTests { readonly packages: 'all' | readonly string[]; readonly site: boolean; readonly files: readonly string[]; } + +/** Tests that live in an offline tool but exercise another package: they ran on every change to that package until the + * renderer's tests that import the bake moved into the bake, which the dependent walk below does not follow (a tool joins + * only when a tool changed). Each runs whenever its owner, or a package the owner depends on, changed. `affected-tests.test.mts` + * proves every entry exists and imports its owner. */ +export const FOREIGN_TESTS: Readonly> = { + renderer: [ + 'packages/bake/src/contract/feature-fixture.test.mts', + 'packages/bake/src/contract/object-runtime.test.mts', + 'packages/bake/src/contract/object-selection-runtime.test.mts', + 'packages/bake/src/contract/prepared-material.test.mts', + 'packages/bake/src/contract/renderer-navigable-object-mount.test.ts', + 'packages/bake/src/objects/layers/paged-ellipsoid/renderer-surface-target.test.ts', + 'packages/bake/src/presentation/renderer-depth-partitions.test.ts', + 'packages/bake/src/sky/renderer-parallax.test.ts', + 'packages/bake/src/stars/renderer-point-field-bank.test.ts', + 'packages/bake/src/stars/renderer-validation.test.ts', + 'packages/bake/src/volume-leaves/renderer-prepared-leaf-frustum.test.ts', + 'packages/bake/src/volume-leaves/renderer-prepared-volume-runtime.test.ts', + ], +}; const SHARED = [/^package\.json$/u, /^pnpm-lock\.yaml$/u, /^pnpm-workspace\.yaml$/u, /^tsconfig[^/]*\.json$/u, /^\.github\/workflows\//u]; /** The offline preparation and archive tools. Nearly every change touches a package they import (the renderer), so a @@ -18,7 +39,7 @@ const TOOLS = new Set(['bake', 'telescope-cli']); const SITE = [/^site\//u, /^src\//u, /^integration\//u, /^\.github\//u, /^labs\/performance\//u]; export function affectedTests(paths: readonly string[] | null, packages: readonly Workspace[], siteDependencies: readonly string[]): AffectedTests { - if (paths === null || paths.some(path => SHARED.some(pattern => pattern.test(path)))) return { packages: 'all', site: true }; + if (paths === null || paths.some(path => SHARED.some(pattern => pattern.test(path)))) return { packages: 'all', site: true, files: [] }; const changed = new Set(paths.flatMap(path => /^packages\/([^/]+)\//u.exec(path)?.[1] ?? [])); // Everything that imports a changed package can break with it: walk the dependents until nothing new joins. const byName = new Map(packages.map(workspace => [workspace.name, workspace.directory])); @@ -31,7 +52,10 @@ export function affectedTests(paths: readonly string[] | null, packages: readonl for (const workspace of packages) if (!changed.has(workspace.directory) && joins(workspace)) { changed.add(workspace.directory); grew = true; } } const site = paths.some(path => SITE.some(pattern => pattern.test(path))) || siteDependencies.some(name => changed.has(byName.get(name) ?? '')); - return { packages: [...changed].sort(), site }; + // A foreign test whose own package already runs is in that package's glob. + const files = Object.entries(FOREIGN_TESTS).filter(([owner]) => changed.has(owner)) + .flatMap(([, tests]) => tests).filter(test => !changed.has(/^packages\/([^/]+)\//u.exec(test)?.[1] ?? '')).sort(); + return { packages: [...changed].sort(), site, files }; } function workspaceDependencies(manifest: Record): string[] { @@ -49,6 +73,7 @@ if (process.argv[1] && import.meta.url === pathToFileURL(resolve(process.argv[1] const paths = base ? execFileSync('git', ['diff', '--name-only', `${base}...HEAD`], { cwd: root, encoding: 'utf8' }).split('\n').filter(Boolean) : null; const result = affectedTests(paths, packages, workspaceDependencies(read('package.json'))); const testPackages = result.packages === 'all' ? 'all' : result.packages.join(' '); - console.log(`Packages to test: ${testPackages || '(none)'}; site: ${result.site}.`); - if (process.env.GITHUB_OUTPUT) appendFileSync(process.env.GITHUB_OUTPUT, `test_packages=${testPackages}\ntest_site=${result.site}\n`); + const testFiles = result.files.join(' '); + console.log(`Packages to test: ${testPackages || '(none)'}; extra files: ${testFiles || '(none)'}; site: ${result.site}.`); + if (process.env.GITHUB_OUTPUT) appendFileSync(process.env.GITHUB_OUTPUT, `test_packages=${testPackages}\ntest_files=${testFiles}\ntest_site=${result.site}\n`); } diff --git a/.github/scripts/ci/affected-tests.test.mts b/.github/scripts/ci/affected-tests.test.mts index d48238db81d..e8365162cd6 100644 --- a/.github/scripts/ci/affected-tests.test.mts +++ b/.github/scripts/ci/affected-tests.test.mts @@ -1,6 +1,8 @@ import assert from 'node:assert/strict'; import { test } from 'node:test'; -import { affectedTests } from './affected-tests.mts'; +import { existsSync, readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; +import { affectedTests, FOREIGN_TESTS } from './affected-tests.mts'; const packages = [ { directory: 'core', name: '@cssearth/core', dependencies: [] }, @@ -9,23 +11,42 @@ const packages = [ { directory: 'telescope-cli', name: '@cssearth/telescope-cli', dependencies: ['@cssearth/bake'] }, ]; const site = ['@cssearth/core', '@cssearth/renderer']; +const renderer = FOREIGN_TESTS.renderer!.slice().sort(); test('a changed package tests itself and every package that depends on it, and the site when the site imports one', () => { - assert.deepEqual(affectedTests(['packages/core/src/a.ts'], packages, site), { packages: ['core', 'renderer'], site: true }); - assert.deepEqual(affectedTests(['packages/telescope-cli/src/a.mts'], packages, site), { packages: ['telescope-cli'], site: false }); + assert.deepEqual(affectedTests(['packages/core/src/a.ts'], packages, site), { packages: ['core', 'renderer'], site: true, files: renderer }); + assert.deepEqual(affectedTests(['packages/telescope-cli/src/a.mts'], packages, site), { packages: ['telescope-cli'], site: false, files: [] }); }); test('the offline tools join when they or another tool changed, not when the renderer they import did', () => { - assert.deepEqual(affectedTests(['packages/renderer/src/a.ts'], packages, site), { packages: ['renderer'], site: true }); - assert.deepEqual(affectedTests(['packages/bake/src/a.ts'], packages, site), { packages: ['bake', 'telescope-cli'], site: false }); + assert.deepEqual(affectedTests(['packages/renderer/src/a.ts'], packages, site), { packages: ['renderer'], site: true, files: renderer }); + assert.deepEqual(affectedTests(['packages/bake/src/a.ts'], packages, site), { packages: ['bake', 'telescope-cli'], site: false, files: [] }); }); test('object data and the site test no package; documentation tests nothing', () => { - assert.deepEqual(affectedTests(['src/objects/mars/text.json'], packages, site), { packages: [], site: true }); - assert.deepEqual(affectedTests(['docs/ci-cd.md'], packages, site), { packages: [], site: false }); + assert.deepEqual(affectedTests(['src/objects/mars/text.json'], packages, site), { packages: [], site: true, files: [] }); + assert.deepEqual(affectedTests(['docs/ci-cd.md'], packages, site), { packages: [], site: false, files: [] }); }); test('a push or a shared configuration change tests everything', () => { - assert.deepEqual(affectedTests(null, packages, site), { packages: 'all', site: true }); - assert.deepEqual(affectedTests(['pnpm-lock.yaml', 'src/objects/mars/text.json'], packages, site), { packages: 'all', site: true }); + assert.deepEqual(affectedTests(null, packages, site), { packages: 'all', site: true, files: [] }); + assert.deepEqual(affectedTests(['pnpm-lock.yaml', 'src/objects/mars/text.json'], packages, site), { packages: 'all', site: true, files: [] }); +}); + +test('the bake tests that exercise the renderer run on a renderer-only change, though the bake does not join', () => { + const result = affectedTests(['packages/renderer/src/a.ts'], packages, site); + assert.deepEqual(result.packages, ['renderer']); + assert.ok(result.files.includes('packages/bake/src/contract/renderer-navigable-object-mount.test.ts')); + assert.deepEqual(affectedTests(['packages/core/src/a.ts'], packages, site).files, renderer, 'a package the renderer depends on reaches them too'); + assert.deepEqual(affectedTests(['packages/bake/src/a.ts'], packages, site).files, [], 'when the bake runs, its own glob already holds them'); + assert.deepEqual(affectedTests(['docs/ci-cd.md'], packages, site).files, []); +}); + +test('every foreign test exists, sits in an offline tool and imports its owner package', () => { + const root = resolve(import.meta.dirname, '../../..'); + for (const [owner, tests] of Object.entries(FOREIGN_TESTS)) for (const test of tests) { + assert.ok(existsSync(resolve(root, test)), `${test} is missing`); + assert.match(test, /^packages\/(bake|telescope-cli)\//u); + assert.match(readFileSync(resolve(root, test), 'utf8'), new RegExp(`@cssearth/${owner}[/'"]`, 'u'), `${test} does not import ${owner}`); + } }); diff --git a/.github/workflows/universe.yml b/.github/workflows/universe.yml index f3b416ec94e..843eb06db63 100644 --- a/.github/workflows/universe.yml +++ b/.github/workflows/universe.yml @@ -36,6 +36,7 @@ jobs: run_universe_preparation: ${{ steps.classify.outputs.run_universe_preparation }} run_nebula: ${{ steps.classify.outputs.run_nebula }} test_packages: ${{ steps.affected.outputs.test_packages }} + test_files: ${{ steps.affected.outputs.test_files }} test_site: ${{ steps.affected.outputs.test_site }} steps: - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 @@ -287,8 +288,9 @@ jobs: env: CI_UNIVERSE_LANE: ${{ matrix.lane }} CI_TEST_PACKAGES: ${{ needs.changes.outputs.test_packages }} + CI_TEST_FILES: ${{ needs.changes.outputs.test_files }} # A lane with nothing to test still reports, so the required check passes, but does no work. - CI_LANE_SKIP: ${{ (matrix.lane == 'packages' && needs.changes.outputs.test_packages == '') || (matrix.lane == 'site' && needs.changes.outputs.test_site != 'true') }} + CI_LANE_SKIP: ${{ (matrix.lane == 'packages' && needs.changes.outputs.test_packages == '' && needs.changes.outputs.test_files == '') || (matrix.lane == 'site' && needs.changes.outputs.test_site != 'true') }} needs: changes if: ${{ github.event_name != 'pull_request' || needs.changes.outputs.run_universe == 'true' }} runs-on: ubuntu-latest @@ -383,6 +385,8 @@ jobs: node packages/bake/cli/restore-source-inputs.mts --repository-volumes if [ "$CI_TEST_PACKAGES" != all ]; then tests=(); for name in $CI_TEST_PACKAGES; do tests+=("packages/$name/**/*.test.{ts,mts}"); done + # Tests that live in an offline tool but exercise a changed package (FOREIGN_TESTS in affected-tests.mts). + for file in $CI_TEST_FILES; do tests+=("$file"); done pnpm test:run "${tests[@]}" exit 0 fi diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index aab1d0ae60d..353569fad98 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -172,7 +172,7 @@ lanes are folders: `test:packages` runs `packages/`, and `test:site` runs `site/ A test that needs an input CI does not restore skips through `sourceTest()`. Tests stay beside their owners. -The required universe matrix runs the two lanes; both must pass. A pull request tests only what it can break: the packages it changed and every package that depends on them (the offline tools `bake` and `telescope-cli` only when a tool changed), and the site when it or a package it imports changed (`.github/scripts/ci/affected-tests.mts`). A push to main, or a change to shared configuration, tests everything. The preparation gate checks publication. Source +The required universe matrix runs the two lanes; both must pass. A pull request tests only what it can break: the packages it changed and every package that depends on them (the offline tools `bake` and `telescope-cli` only when a tool changed, except the bake tests listed in `FOREIGN_TESTS` that exercise the renderer, which run whenever it changes), and the site when it or a package it imports changed (`.github/scripts/ci/affected-tests.mts`). A push to main, or a change to shared configuration, tests everything. The preparation gate checks publication. Source catalogue reconciliation and broad bake reproduction run in the separate advisory audit. Native tests needing unavailable sources, prepared outputs or toolchains can skip through `@cssearth/objects/node/source-test`; a pass with skips does not prove From 90ac37fdc491338819b171a9461c270e94bc0093 Mon Sep 17 00:00:00 2001 From: Juan Cruz Fortunatti Date: Thu, 1 Oct 2026 21:57:31 +0200 Subject: [PATCH 2/2] fix(ci): the local CI run substitutes the test files output and the packages lane skip --- .github/scripts/ci/check-ci.mts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/scripts/ci/check-ci.mts b/.github/scripts/ci/check-ci.mts index 559e1ad0a37..e49e3a99308 100644 --- a/.github/scripts/ci/check-ci.mts +++ b/.github/scripts/ci/check-ci.mts @@ -19,7 +19,8 @@ const LOCAL_EXPRESSION_SUBSTITUTIONS:Record={ '${{ needs.changes.outputs.run_universe }}':'true', '${{ needs.changes.outputs.run_universe_preparation }}':'true', '${{ needs.changes.outputs.test_packages }}':'all', - "${{ (matrix.lane == 'packages' && needs.changes.outputs.test_packages == '') || (matrix.lane == 'site' && needs.changes.outputs.test_site != 'true') }}":'false', + '${{ needs.changes.outputs.test_files }}':'', + "${{ (matrix.lane == 'packages' && needs.changes.outputs.test_packages == '' && needs.changes.outputs.test_files == '') || (matrix.lane == 'site' && needs.changes.outputs.test_site != 'true') }}":'false', '${{ steps.build-tools-cache.outputs.cache-hit }}':'false', '${{ steps.ci-cache-key.outputs.build_digest }}':'', '${{ steps.package-cache.outputs.cache-hit }}':'false',