diff --git a/src/prerelease-workflow.test.ts b/src/prerelease-workflow.test.ts index b0dd3e51..371be2c2 100644 --- a/src/prerelease-workflow.test.ts +++ b/src/prerelease-workflow.test.ts @@ -2,9 +2,16 @@ import test from 'node:test'; import assert from 'node:assert/strict'; import { readFileSync } from 'node:fs'; import { resolve } from 'node:path'; +import { + jobSteps, + readWorkflow, + stepsMissingEnvDeclaration, + templateExpressionsInRunBodies, +} from './workflow-test-helpers'; const prereleaseWorkflowPath = resolve(__dirname, '../.github/workflows/prerelease.yml'); const prereleaseWorkflow = readFileSync(prereleaseWorkflowPath, 'utf8').replace(/\r\n/g, '\n'); +const parsedPrereleaseWorkflow = readWorkflow(prereleaseWorkflowPath); const packageJsonPath = resolve(__dirname, '../package.json'); const packageJson = JSON.parse(readFileSync(packageJsonPath, 'utf8')) as { scripts: Record; @@ -128,29 +135,24 @@ test('prerelease workflow rejects committed notes generated for a different beta packageJson.scripts['changelog:check-prerelease-notes'], 'bun run scripts/build-changelog.ts check-prerelease-notes', ); - assert.match( - prereleaseWorkflow, - /bun run changelog:check-prerelease-notes --version "\$RELEASE_VERSION"/, + + const steps = jobSteps(parsedPrereleaseWorkflow, 'release'); + const checkIndex = steps.findIndex((step) => + step.run?.includes('changelog:check-prerelease-notes'), ); + const publishIndex = steps.findIndex((step) => /gh release (create|edit)/.test(step.run ?? '')); - // The staleness check has to run before the release is created or edited, - // otherwise stale notes are already published by the time it fails. - const checkIndex = prereleaseWorkflow.indexOf('changelog:check-prerelease-notes'); - const createIndex = prereleaseWorkflow.indexOf('gh release create'); - const editIndex = prereleaseWorkflow.indexOf('gh release edit'); assert.notEqual(checkIndex, -1); - assert.ok(checkIndex < createIndex); - assert.ok(checkIndex < editIndex); + assert.notEqual(publishIndex, -1); + // Stale notes are already published if the check runs after the release. + assert.ok(checkIndex < publishIndex); + assert.match( + steps[checkIndex]!.run!, + /changelog:check-prerelease-notes --version "\$RELEASE_VERSION"/, + ); }); -// GitHub substitutes ${{ }} into the run script before the shell parses it, so a -// tag-derived value used that way is executed as script rather than read as data. -test('tag-derived values reach shell bodies through env, not template interpolation', () => { - const rawVersionUses = prereleaseWorkflow - .split('\n') - .filter((line) => line.includes('steps.version.outputs.VERSION')) - .filter( - (line) => !/^\s*RELEASE_VERSION: \$\{\{ steps\.version\.outputs\.VERSION \}\}$/.test(line), - ); - assert.deepEqual(rawVersionUses, []); +test('prerelease workflow keeps tag-derived values out of shell bodies', () => { + assert.deepEqual(templateExpressionsInRunBodies(parsedPrereleaseWorkflow), []); + assert.deepEqual(stepsMissingEnvDeclaration(parsedPrereleaseWorkflow, 'RELEASE_VERSION'), []); }); diff --git a/src/release-workflow.test.ts b/src/release-workflow.test.ts index 7325dba7..e3cc3c7d 100644 --- a/src/release-workflow.test.ts +++ b/src/release-workflow.test.ts @@ -2,11 +2,18 @@ import test from 'node:test'; import assert from 'node:assert/strict'; import { readFileSync } from 'node:fs'; import { resolve } from 'node:path'; +import { + readWorkflow, + stepsMissingEnvDeclaration, + templateExpressionsInRunBodies, +} from './workflow-test-helpers'; const releaseWorkflowPath = resolve(__dirname, '../.github/workflows/release.yml'); const releaseWorkflow = readFileSync(releaseWorkflowPath, 'utf8'); const docsPagesWorkflowPath = resolve(__dirname, '../.github/workflows/docs-pages.yml'); const docsPagesWorkflow = readFileSync(docsPagesWorkflowPath, 'utf8'); +const parsedReleaseWorkflow = readWorkflow(releaseWorkflowPath); +const parsedDocsPagesWorkflow = readWorkflow(docsPagesWorkflowPath); const makefilePath = resolve(__dirname, '../Makefile'); const makefile = readFileSync(makefilePath, 'utf8'); const packageJsonPath = resolve(__dirname, '../package.json'); @@ -279,26 +286,13 @@ test('Makefile uninstall targets remove bundled runtime plugin app-data copies', assert.match(makefile, /Removed:[\s\S]*\$\(MACOS_DATA_DIR\)\/plugin\/subminer/); }); -// GitHub substitutes ${{ }} into the run script before the shell parses it, so a -// tag-derived value used that way is executed as script rather than read as data. -// The release and docs workflows must route tag values through env and -// reference them as shell variables. -test('tag-derived values reach shell bodies through env, not template interpolation', () => { - const rawVersionUses = releaseWorkflow - .split('\n') - .filter((line) => line.includes('steps.version.outputs.VERSION')) - .filter( - (line) => !/^\s*RELEASE_VERSION: \$\{\{ steps\.version\.outputs\.VERSION \}\}$/.test(line), - ); - assert.deepEqual(rawVersionUses, []); - - const rawRefNameUses = docsPagesWorkflow - .split('\n') - .filter((line) => line.includes('github.ref_name')) - .filter((line) => !/^\s*TAG_NAME: \$\{\{ github\.ref_name \}\}$/.test(line)) - // `if:` conditions are evaluated by Actions itself, never handed to a shell. - .filter((line) => !/^\s*if:/.test(line)); - assert.deepEqual(rawRefNameUses, []); +test('release and docs workflows keep tag-derived values out of shell bodies', () => { + assert.deepEqual(templateExpressionsInRunBodies(parsedReleaseWorkflow), []); + assert.deepEqual(templateExpressionsInRunBodies(parsedDocsPagesWorkflow), []); + assert.deepEqual(stepsMissingEnvDeclaration(parsedReleaseWorkflow, 'RELEASE_VERSION'), []); + assert.deepEqual(stepsMissingEnvDeclaration(parsedDocsPagesWorkflow, 'TAG_NAME'), []); + // The docs tag guard must test the shell variable, not an interpolated value + // that would be substituted into the condition before the shell reads it. assert.match(docsPagesWorkflow, /if \[\[ ! "\$TAG_NAME" =~/); }); diff --git a/src/workflow-test-helpers.ts b/src/workflow-test-helpers.ts new file mode 100644 index 00000000..6771988c --- /dev/null +++ b/src/workflow-test-helpers.ts @@ -0,0 +1,65 @@ +import { readFileSync } from 'node:fs'; + +export type WorkflowStep = { + name?: string; + run?: string; + env?: Record; +}; + +export type ParsedWorkflow = { + jobs?: Record; +}; + +// Workflow tests only ever run under `bun test`, which parses YAML natively. +function parseWorkflowYaml(source: string): ParsedWorkflow { + const bunRuntime = globalThis as typeof globalThis & { + Bun?: { YAML?: { parse?: (input: string) => unknown } }; + }; + const parse = bunRuntime.Bun?.YAML?.parse; + if (!parse) { + throw new Error('Bun.YAML.parse is unavailable; workflow tests must run under bun.'); + } + return parse(source) as ParsedWorkflow; +} + +export function readWorkflow(workflowPath: string): ParsedWorkflow { + return parseWorkflowYaml(readFileSync(workflowPath, 'utf8')); +} + +// Steps of one job, in declaration order. Throws on an unknown job so a renamed +// job fails loudly instead of silently emptying an ordering assertion. +export function jobSteps(workflow: ParsedWorkflow, jobName: string): WorkflowStep[] { + const job = workflow.jobs?.[jobName]; + if (!job) { + throw new Error(`Workflow has no job named ${jobName}.`); + } + return job.steps ?? []; +} + +function allSteps(workflow: ParsedWorkflow): Array<{ job: string; step: WorkflowStep }> { + return Object.entries(workflow.jobs ?? {}).flatMap(([job, definition]) => + (definition?.steps ?? []).map((step) => ({ job, step })), + ); +} + +// GitHub substitutes ${{ }} into a run script before the shell parses it, so any +// value used that way is executed as script rather than read as data. Reporting +// every expression (rather than allow-listing known-safe ones) also covers +// alternate spellings such as ${{ steps.version.outputs['VERSION'] }}. +export function templateExpressionsInRunBodies(workflow: ParsedWorkflow): string[] { + return allSteps(workflow).flatMap(({ job, step }) => + (typeof step.run === 'string' ? (step.run.match(/\$\{\{[\s\S]*?\}\}/g) ?? []) : []).map( + (expression) => `${job}/${step.name ?? ''}: ${expression}`, + ), + ); +} + +// Steps whose shell body reads $NAME without the step declaring it in env, which +// would silently expand to an empty string at run time. +export function stepsMissingEnvDeclaration(workflow: ParsedWorkflow, name: string): string[] { + const reference = new RegExp(`\\$${name}\\b|\\$\\{${name}\\b`); + return allSteps(workflow) + .filter(({ step }) => typeof step.run === 'string' && reference.test(step.run)) + .filter(({ step }) => !Object.prototype.hasOwnProperty.call(step.env ?? {}, name)) + .map(({ job, step }) => `${job}/${step.name ?? ''}`); +}