diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 9b0b6e30b..618b98ef8 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -99,7 +99,9 @@ jobs: run: | set -euo pipefail list="$RUNNER_TEMP/changed-files.txt" - changed_between() { git -c core.quotePath=false diff --name-only "$1" "$2" > "$list"; } + # --no-renames lists a moved file under its old path too, so a move + # out of an area still runs that area. + changed_between() { git -c core.quotePath=false diff --no-renames --name-only "$1" "$2" > "$list"; } case "$EVENT" in pull_request) changed_between HEAD^1 HEAD diff --git a/scripts/ci/changed-areas.mjs b/scripts/ci/changed-areas.mjs index 424f914a1..87f63aeba 100644 --- a/scripts/ci/changed-areas.mjs +++ b/scripts/ci/changed-areas.mjs @@ -27,6 +27,12 @@ import { fileURLToPath } from 'node:url'; // runs everything. const ALWAYS = ['.github/workflows/test.yml', 'scripts/ci/']; +// Files the test suites read straight from disk, outside the packages: +// config-templates.test.ts pins docker-compose.yml and the Helm chart to the +// code defaults, help.test.ts serves wiki pages and images, and the client's +// help registry test checks every help link against the wiki headings. +const READ_BY_TESTS = ['charts/', /^docker-compose[^/]*\.ya?ml$/, 'wiki/']; + export const AREAS = { // The type, lint, test, coverage, bundle and end-to-end jobs. npm workspaces // hoist every dependency into the root lockfile, so a dependency bump touches @@ -42,14 +48,16 @@ export const AREAS = { '.nvmrc', 'sonar-project.properties', 'Dockerfile', + ...READ_BY_TESTS, ...ALWAYS, ], // The image build and its boot check: everything the Dockerfile copies or - // installs from. + // installs from. The image serves the in-app help from wiki/. image: [ 'server/', 'client/', 'shared/', + 'wiki/', 'package.json', 'package-lock.json', '.nvmrc', @@ -61,9 +69,11 @@ export const AREAS = { deploy: ['charts/', /^docker-compose[^/]*\.ya?ml$/, ...ALWAYS], }; -// Markdown inside the packages (CLAUDE.md, READMEs, PATTERN.md) changes no -// test, no build and no scan. -const isMarkdown = (path) => path.endsWith('.md'); +// Markdown outside wiki/ (the root README, CLAUDE.md, the package READMEs, +// PATTERN.md) changes no test, no build and no scan. The wiki does: the image +// ships it as the in-app help and tests read its pages and headings, so a +// wiki page is classified like any other file. +const isInertMarkdown = (path) => path.endsWith('.md') && !path.startsWith('wiki/'); function matches(rule, path) { if (rule instanceof RegExp) return rule.test(path); @@ -86,7 +96,7 @@ export function classify(files) { const result = {}; for (const [area, rules] of Object.entries(AREAS)) { - result[area] = paths.some((path) => !isMarkdown(path) && rules.some((rule) => matches(rule, path))); + result[area] = paths.some((path) => !isInertMarkdown(path) && rules.some((rule) => matches(rule, path))); } return result; } diff --git a/scripts/ci/changed-areas.test.mjs b/scripts/ci/changed-areas.test.mjs index a3110ce41..2a4fb1a8c 100644 --- a/scripts/ci/changed-areas.test.mjs +++ b/scripts/ci/changed-areas.test.mjs @@ -1,13 +1,14 @@ import assert from 'node:assert/strict'; -import { spawnSync } from 'node:child_process'; -import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { execFileSync, spawnSync } from 'node:child_process'; +import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; -import { join } from 'node:path'; +import { basename, dirname, join } from 'node:path'; import { after, describe, it } from 'node:test'; import { fileURLToPath } from 'node:url'; import { AREAS, classify } from './changed-areas.mjs'; const script = fileURLToPath(new URL('./changed-areas.mjs', import.meta.url)); +const workflow = fileURLToPath(new URL('../../.github/workflows/test.yml', import.meta.url)); const scratch = mkdtempSync(join(tmpdir(), 'changed-areas-')); after(() => rmSync(scratch, { recursive: true, force: true })); @@ -22,8 +23,15 @@ function run(args, env = {}) { } describe('classify', () => { - it('runs nothing for a docs-only change', () => { - assert.deepEqual(classify(['wiki/Home.md', 'README.md', 'server/CLAUDE.md', 'docs/logo-trek-dark.svg']), none); + it('runs nothing for a docs-only change outside the wiki', () => { + assert.deepEqual(classify(['README.md', 'server/CLAUDE.md', 'charts/README.md', 'docs/logo-trek-dark.svg']), none); + }); + + it('runs the tests and the image for a wiki change, markdown or not', () => { + // The image ships wiki/ as the in-app help, help.test.ts serves its pages + // and images, and the client's help registry test reads its headings. + assert.deepEqual(classify(['wiki/Updating.md']), { code: true, image: true, deploy: false }); + assert.deepEqual(classify(['wiki/assets/TripPlanner.png']), { code: true, image: true, deploy: false }); }); it('runs the tests and the image for a server change', () => { @@ -38,11 +46,16 @@ describe('classify', () => { assert.deepEqual(classify(['.dockerignore']), { code: false, image: true, deploy: false }); }); - it('runs the chart and compose checks for a chart template or a compose file', () => { - assert.deepEqual(classify(['charts/trek/templates/deployment.yaml']), { code: false, image: false, deploy: true }); - assert.deepEqual(classify(['docker-compose.minio-test.yml']), { code: false, image: false, deploy: true }); + it('runs the chart checks and the tests for a chart template or a compose file', () => { + // config-templates.test.ts reads docker-compose.yml and the chart to pin + // them to the code defaults, so a change to either runs the tests too. + assert.deepEqual(classify(['charts/trek/templates/deployment.yaml']), { code: true, image: false, deploy: true }); + assert.deepEqual(classify(['charts/trek/values.yaml']), { code: true, image: false, deploy: true }); + assert.deepEqual(classify(['docker-compose.yml']), { code: true, image: false, deploy: true }); + assert.deepEqual(classify(['docker-compose.minio-test.yml']), { code: true, image: false, deploy: true }); assert.equal(classify(['charts/README.md']).deploy, false); assert.equal(classify(['server/docker-compose.yml']).deploy, false); + assert.equal(classify(['docs/docker-compose.yml']).code, false); }); it('runs everything when the workflow or the classifier itself changes', () => { @@ -51,7 +64,7 @@ describe('classify', () => { }); it('matches whole file names, not prefixes of them', () => { - assert.deepEqual(classify(['package.json.bak', 'Dockerfile.old', 'serverless/x.ts']), none); + assert.deepEqual(classify(['package.json.bak', 'Dockerfile.old', 'serverless/x.ts', 'wikis/x.png']), none); }); it('runs everything when there is no list, an empty one or a quoted path', () => { @@ -61,7 +74,8 @@ describe('classify', () => { }); it('reads CRLF lists the same as LF ones', () => { - assert.deepEqual(classify(['wiki/Home.md\r', 'server/src/index.ts\r']), { code: true, image: true, deploy: false }); + assert.deepEqual(classify(['README.md\r', 'server/src/index.ts\r']), { code: true, image: true, deploy: false }); + assert.deepEqual(classify(['README.md\r', 'charts/trek/values.yaml\r']), { code: true, image: false, deploy: true }); }); }); @@ -73,8 +87,8 @@ describe('the command line', () => { writeFileSync(output, ''); const result = run(['--files', list], { GITHUB_OUTPUT: output }); assert.equal(result.status, 0, result.stderr); - assert.equal(result.stdout, 'code=false\nimage=false\ndeploy=true\n'); - assert.equal(readFileSync(output, 'utf8'), 'code=false\nimage=false\ndeploy=true\n'); + assert.equal(result.stdout, 'code=true\nimage=false\ndeploy=true\n'); + assert.equal(readFileSync(output, 'utf8'), 'code=true\nimage=false\ndeploy=true\n'); }); it('turns everything on with --all', () => { @@ -98,3 +112,52 @@ describe('the command line', () => { } }); }); + +describe('the file list test.yml builds', () => { + // The git command of the changed_between helper, read out of test.yml so the + // test runs what CI runs rather than a copy of it. + function diffArgs(from, to) { + const found = [...readFileSync(workflow, 'utf8').matchAll(/changed_between\(\) \{ git (.+?) > "\$list"; \}/g)]; + assert.equal(found.length, 1, 'test.yml must define changed_between exactly once'); + const args = found[0][1].split(/\s+/).map((arg) => arg.replace(/^"(.*)"$/, '$1')); + assert.ok(args.includes('$1') && args.includes('$2'), `changed_between does not diff "$1" "$2": ${args.join(' ')}`); + return args.map((arg) => (arg === '$1' ? from : arg === '$2' ? to : arg)); + } + + // A repository of its own, cut off from the user's and the system's git + // config so neither commit signing nor a rename setting leaks into it. + function scratchRepo() { + const dir = mkdtempSync(join(scratch, 'repo-')); + const globalConfig = join(scratch, `${basename(dir)}.gitconfig`); + writeFileSync(globalConfig, ''); + const env = { ...process.env, GIT_CONFIG_GLOBAL: globalConfig, GIT_CONFIG_NOSYSTEM: '1' }; + const git = (...args) => execFileSync('git', args, { cwd: dir, env, encoding: 'utf8' }); + git('init', '-q'); + git('config', 'user.name', 'test'); + git('config', 'user.email', 'test@example.invalid'); + return { dir, git }; + } + + function changedAfterMove(from, to) { + const { dir, git } = scratchRepo(); + mkdirSync(dirname(join(dir, from)), { recursive: true }); + writeFileSync(join(dir, from), 'export const value = 1;\n'.repeat(20)); + git('add', '-A'); + git('commit', '-q', '-m', 'add'); + mkdirSync(dirname(join(dir, to)), { recursive: true }); + git('mv', from, to); + git('commit', '-q', '-m', 'move'); + return git(...diffArgs('HEAD^1', 'HEAD')).split('\n'); + } + + it('lists a moved file under its old path, so a move out of an area still runs it', () => { + const files = changedAfterMove('server/src/a.ts', 'docs/a.ts'); + assert.ok(files.includes('server/src/a.ts'), `old path missing from ${JSON.stringify(files)}`); + assert.deepEqual(classify(files), { code: true, image: true, deploy: false }); + }); + + it('runs the chart checks when a chart template moves out of charts/', () => { + const files = changedAfterMove('charts/trek/templates/secret.yaml', 'docs/secret.yaml'); + assert.equal(classify(files).deploy, true); + }); +});