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/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', 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