From 88955f1bc3623bf9af105518c226e30c4c695ff4 Mon Sep 17 00:00:00 2001 From: Richard McDaniel <1130888+rmcdaniel@users.noreply.github.com> Date: Sat, 24 Oct 2026 06:35:39 +0000 Subject: [PATCH 1/2] Report one Shared Server fixtures result on every pull request A required check and the merge queue both need a result that always reports. The pull_request path filter meant a docs-only pull request got no Shared Server fixtures result at all, so the workflow could not be required. - Pull requests now run the workflow without a path filter. A short `changes` job applies the push trigger's path list to the pull request's merge commit (renames count on both sides), so one list governs both triggers. Push, merge_group and manual runs run every fixture job, as does any pull request that cannot be classified. - `published-sdk-adapters` runs only when the classifier says so; the other fixture jobs need it, so they skip with it. - A terminal `Shared Server fixtures result` job runs with always(). It passes only when every fixture job passed, or when no input changed and every fixture job was skipped. A failed, cancelled or unexpectedly skipped job, a failed classifier, or an unreadable needs context fails it. - ServerParityAggregateTest covers the workflow structure, the result decisions and the classifier against scratch pull request merge commits. Repository contracts runs it. No permissions, secrets, actions or artifact paths change. Making the result a required check, and enabling the merge queue, stay separate repository-setting steps. --- .github/workflows/phpunit-feature.yml | 3 + .github/workflows/server-parity.yml | 104 ++++----- .../conformance/server-parity/aggregate.mjs | 51 +++++ scripts/conformance/server-parity/changes.mjs | 76 +++++++ tests/Unit/MergeGroupTriggersTest.mjs | 6 +- tests/Unit/ServerParityAggregateTest.mjs | 214 ++++++++++++++++++ 6 files changed, 396 insertions(+), 58 deletions(-) create mode 100644 scripts/conformance/server-parity/aggregate.mjs create mode 100644 scripts/conformance/server-parity/changes.mjs create mode 100644 tests/Unit/ServerParityAggregateTest.mjs diff --git a/.github/workflows/phpunit-feature.yml b/.github/workflows/phpunit-feature.yml index 676bd0319..a55b16f72 100644 --- a/.github/workflows/phpunit-feature.yml +++ b/.github/workflows/phpunit-feature.yml @@ -81,6 +81,9 @@ jobs: - name: Run the required checks for merge queue groups run: node --test tests/Unit/MergeGroupTriggersTest.mjs + - name: Report one Shared Server fixtures result on every pull request + run: node --test tests/Unit/ServerParityAggregateTest.mjs + - name: Validate embedded Workflow package authority run: php scripts/ci/test-workflow-package-authority.php diff --git a/.github/workflows/server-parity.yml b/.github/workflows/server-parity.yml index 1416deba5..f5759de26 100644 --- a/.github/workflows/server-parity.yml +++ b/.github/workflows/server-parity.yml @@ -1,62 +1,9 @@ name: Shared Server fixtures on: + # Pull requests run without a path filter so that the terminal result always + # reports. The `changes` job applies the push path list below to them. pull_request: - paths: - - .github/workflows/server-parity.yml - - scripts/conformance/server-parity* - - scripts/conformance/server-parity/** - - tests/Fixtures/ServerParity/** - - tests/Fixtures/ServerParityBaselines/** - - tests/Fixtures/ServerParityNormalization/** - - tests/Unit/ServerParityRunnerTest.mjs - - tests/Unit/ServerParityArtifactSummaryTest.mjs - - tests/Unit/ServerParityQueryObserverTest.mjs - - tests/Unit/ServerParityCodecDeadlineLoadTest.mjs - - tests/Unit/ServerParityCooperativeContractTest.mjs - - tests/Unit/ServerParityChildCancellationContractTest.mjs - - tests/Support/ServerParityChildCancellationObservation.mjs - - tests/Unit/ServerParityScheduleContractTest.mjs - - tests/Support/ServerParityScheduleObservation.mjs - - tests/Fixtures/ServerParityPending/** - - tests/Fixtures/ServerParityExpected/** - - tests/Fixtures/ServerParityProfiles/** - - tests/Unit/ServerParityVisibilityContractTest.mjs - - tests/Unit/ServerParityAdmissionContractTest.mjs - - tests/Unit/ServerParityRoleTokenContractTest.mjs - - tests/Unit/ServerParityWorkerDeregistrationContractTest.mjs - - tests/Unit/ServerParityLateWorkflowCompletionContractTest.mjs - - tests/Unit/ServerParitySearchAttributeContractTest.mjs - - tests/Unit/ServerParityStorageTestContractTest.mjs - - tests/Unit/ServerParitySchedulePoliciesContractTest.mjs - - tests/Unit/ServerParitySignalsBeforeWorkerContractTest.mjs - - tests/Unit/ServerParityScheduleCatalogContractTest.mjs - - tests/Unit/ServerParityWorkerListingContractTest.mjs - - tests/Unit/ServerParityWorkerReplacementContractTest.mjs - - tests/Unit/ServerParityOperatorMetricsContractTest.mjs - - tests/Unit/ServerParityActivityLeaseRenewalContractTest.mjs - - tests/Unit/ServerParityServiceCatalogContractTest.mjs - - tests/Unit/ServerParitySystemMetricsContractTest.mjs - - tests/Unit/ServerParityOperatorDashboardTypesContractTest.mjs - - tests/Unit/ServerParityPrometheusMetricsContractTest.mjs - - tests/Unit/ServerParityPayloadCleanupContractTest.mjs - - tests/Unit/ServerParityRepairPassContractTest.mjs - - tests/Unit/ServerParityClusterInfoContractTest.mjs - - tests/Unit/ServerParityNamespaceDeleteContractTest.mjs - - tests/Unit/ServerParityUpdateValidationContractTest.mjs - - tests/Unit/ServerParityWaitingHistoryContractTest.mjs - - tests/Unit/ServerParityTaskRetryContractTest.mjs - - tests/Unit/ServerParityPublishedPatchContractTest.mjs - - tests/Unit/ServerParityPublishedPatchTransportTest.mjs - - tests/Unit/ServerParityLegacyMarkerContractTest.mjs - - tests/Unit/ServerParityRangeRepairContractTest.mjs - - tests/Unit/ServerParitySdkDirectionContractTest.mjs - - tests/Unit/ServerParitySdkChildDirectionContractTest.mjs - - tests/Unit/ServerParitySdkTimerAuthorContractTest.mjs - - tests/Unit/ServerParitySdkSignalDirectionContractTest.mjs - - tests/Support/ServerParityAdmissionObservation.mjs - - tests/Support/ServerParityVisibilityObservation.mjs - - rust/** push: branches: [main] paths: @@ -134,7 +81,27 @@ env: DW_PARITY_CONTINUE_MEMO_UPSERTS: '1' jobs: + changes: + name: Shared Server fixture inputs + runs-on: ubuntu-latest + timeout-minutes: 5 + outputs: + run: ${{ steps.classify.outputs.run }} + steps: + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 + with: + fetch-depth: 2 + persist-credentials: false + - name: Decide whether fixture inputs changed + id: classify + env: + EVENT_NAME: ${{ github.event_name }} + PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: node scripts/conformance/server-parity/changes.mjs >> "$GITHUB_OUTPUT" + published-sdk-adapters: + needs: changes + if: ${{ needs.changes.outputs.run == 'true' }} runs-on: ubuntu-latest timeout-minutes: 25 env: @@ -1883,3 +1850,30 @@ jobs: path: parity-summary retention-days: 1 if-no-files-found: error + + # The one Shared Server fixtures result. It always runs, including after a + # failure or cancellation, and passes only when every fixture job passed, or + # when no fixture input changed and every fixture job was skipped for that + # reason. + result: + name: Shared Server fixtures result + needs: + - changes + - published-sdk-adapters + - codec-deadline-health + - mysql-execution + - postgres-storage + - execution-and-codec + if: ${{ always() }} + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 + with: + sparse-checkout: scripts/conformance/server-parity/aggregate.mjs + sparse-checkout-cone-mode: false + persist-credentials: false + - name: Require every fixture job to pass, or every one to be skipped for unchanged inputs + env: + NEEDS: ${{ toJSON(needs) }} + run: node scripts/conformance/server-parity/aggregate.mjs diff --git a/scripts/conformance/server-parity/aggregate.mjs b/scripts/conformance/server-parity/aggregate.mjs new file mode 100644 index 000000000..6ee2481e7 --- /dev/null +++ b/scripts/conformance/server-parity/aggregate.mjs @@ -0,0 +1,51 @@ +#!/usr/bin/env node +// The Shared Server fixtures result. NEEDS is the result job's `needs` +// context as JSON. +// +// Passes only when every fixture job passed, or when the classifier reported +// that no fixture input changed and every fixture job was skipped for that +// reason. A failed, cancelled or unexpectedly skipped job fails the result, +// and so does anything that cannot be read. +import {pathToFileURL} from 'node:url'; + +export function decide(needs) { + if (needs === null || typeof needs !== 'object' || Array.isArray(needs)) { + return {pass: false, reason: 'The needs context is not an object.'}; + } + const {changes, ...fixtureJobs} = needs; + const results = Object.entries(fixtureJobs).map(([job, value]) => [job, value?.result]); + const summary = results.map(([job, result]) => `${job}=${result}`).join(', '); + if (changes?.result !== 'success') { + return {pass: false, reason: `The change classifier did not succeed (${changes?.result}).`}; + } + if (results.length === 0) { + return {pass: false, reason: 'No fixture jobs reported.'}; + } + const run = changes.outputs?.run; + const expected = {true: 'success', false: 'skipped'}[run]; + if (expected === undefined) { + return {pass: false, reason: `The change classifier reported no decision (${run}).`}; + } + const unexpected = results.filter(([, result]) => result !== expected); + if (unexpected.length > 0) { + return { + pass: false, + reason: `Every fixture job must be ${expected} when run=${run}: ${unexpected.map(([job, result]) => `${job}=${result}`).join(', ')}.`, + }; + } + return { + pass: true, + reason: run === 'true' ? `Every fixture job passed: ${summary}.` : `No fixture input changed; every fixture job was skipped: ${summary}.`, + }; +} + +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { + let decision; + try { + decision = decide(JSON.parse(process.env.NEEDS ?? '')); + } catch (error) { + decision = {pass: false, reason: `The needs context could not be read: ${error.message}`}; + } + (decision.pass ? console.log : console.error)(decision.reason); + process.exitCode = decision.pass ? 0 : 1; +} diff --git a/scripts/conformance/server-parity/changes.mjs b/scripts/conformance/server-parity/changes.mjs new file mode 100644 index 000000000..2db3f34e8 --- /dev/null +++ b/scripts/conformance/server-parity/changes.mjs @@ -0,0 +1,76 @@ +#!/usr/bin/env node +// Decides whether Shared Server fixtures must run for this event. +// +// Pull requests run the workflow without a path filter, so its terminal +// result always reports. This script applies the workflow's own push path +// filter to the pull request's changes, so one list governs both triggers. +// Every other event runs every fixture job, and so does any pull request +// whose changes cannot be classified. +// +// Writes `run=true` or `run=false` to stdout, for $GITHUB_OUTPUT. +import {execFileSync} from 'node:child_process'; +import {readFileSync} from 'node:fs'; +import {pathToFileURL} from 'node:url'; + +// The `paths` list of the workflow's `on.push` trigger. +export function pushPathPatterns(workflowText) { + const lines = workflowText.split('\n'); + const on = lines.indexOf('on:'); + const push = lines.indexOf(' push:', on); + if (on < 0 || push < 0) throw new Error('The workflow has no on.push trigger'); + const patterns = []; + let inPaths = false; + for (const line of lines.slice(push + 1)) { + if (/^ {0,3}\S/.test(line)) break; + if (line === ' paths:') { + inPaths = true; + } else if (inPaths && line.startsWith(' - ')) { + patterns.push(line.slice(' - '.length).trim()); + } else if (inPaths) { + break; + } + } + if (patterns.length === 0) throw new Error('The push trigger has no path filter'); + return patterns; +} + +// GitHub's filter syntax, limited to the forms this workflow uses: +// `**` matches any characters, `*` matches any characters except `/`. +export function patternRegExp(pattern) { + if (!/^[A-Za-z0-9._/*-]+$/.test(pattern)) throw new Error(`Unsupported path filter pattern: ${pattern}`); + const body = pattern.split('**').map(part => part.split('*').map(text => text.replace(/[.]/g, '\\.')).join('[^/]*')).join('.*'); + return new RegExp(`^${body}$`); +} + +export function matchingFiles(patterns, files) { + const expressions = patterns.map(patternRegExp); + return files.filter(file => expressions.some(expression => expression.test(file))); +} + +function classify({event, headSha, workflowPath}) { + if (event !== 'pull_request') return {run: true, reason: `${event} runs every fixture job`}; + // The checkout is the pull request's merge commit: base, then head. + const git = (...args) => execFileSync('git', args, {encoding: 'utf8'}).trim(); + let files; + try { + const parents = git('rev-list', '--parents', '-n', '1', 'HEAD').split(' '); + if (parents.length !== 3 || parents[2] !== headSha) throw new Error('HEAD is not this pull request\'s merge commit'); + files = git('diff', '--name-only', '--no-renames', 'HEAD^1', 'HEAD').split('\n').filter(Boolean); + } catch (error) { + return {run: true, reason: `changes could not be classified (${error.message}); running every fixture job`}; + } + const matched = matchingFiles(pushPathPatterns(readFileSync(workflowPath, 'utf8')), files); + return matched.length > 0 + ? {run: true, reason: `fixture inputs changed: ${matched.join(', ')}`} + : {run: false, reason: `no fixture input among ${files.length} changed files`}; +} + +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { + const decision = classify({ + event: process.env.EVENT_NAME ?? '', + headSha: process.env.PR_HEAD_SHA ?? '', + workflowPath: '.github/workflows/server-parity.yml', + }); + console.error(decision.reason); + console.log(`run=${decision.run}`); +} diff --git a/tests/Unit/MergeGroupTriggersTest.mjs b/tests/Unit/MergeGroupTriggersTest.mjs index e8efba169..dead41835 100644 --- a/tests/Unit/MergeGroupTriggersTest.mjs +++ b/tests/Unit/MergeGroupTriggersTest.mjs @@ -1,7 +1,7 @@ // The Actions checks a GitHub merge queue waits for must run on merge_group. -// Source-only: enabling the queue is a separate repository setting, and its -// Sonar gate and terminal Shared Server fixtures result are separate -// prerequisites (Server #725). +// Source-only: enabling the queue is a separate repository setting. Its Sonar +// gate is a separate prerequisite (Server #725); the terminal Shared Server +// fixtures result is covered by ServerParityAggregateTest.mjs. import test from 'node:test'; import assert from 'node:assert/strict'; import {readFileSync} from 'node:fs'; diff --git a/tests/Unit/ServerParityAggregateTest.mjs b/tests/Unit/ServerParityAggregateTest.mjs new file mode 100644 index 000000000..768e699fe --- /dev/null +++ b/tests/Unit/ServerParityAggregateTest.mjs @@ -0,0 +1,214 @@ +// The Shared Server fixtures workflow reports one terminal result on every +// pull request, so that result can become a required check (Server #725). +// Pull requests run without a path filter; the `changes` job applies the push +// path list to them, and the `result` job fails closed on anything other than +// all fixture jobs passing, or all of them skipped for unchanged inputs. +import test from 'node:test'; +import assert from 'node:assert/strict'; +import {execFileSync, spawnSync} from 'node:child_process'; +import {copyFileSync, mkdirSync, mkdtempSync, readFileSync, renameSync, rmSync, writeFileSync} from 'node:fs'; +import {tmpdir} from 'node:os'; +import {dirname, join, resolve} from 'node:path'; +import {matchingFiles, patternRegExp, pushPathPatterns} from '../../scripts/conformance/server-parity/changes.mjs'; +import {decide} from '../../scripts/conformance/server-parity/aggregate.mjs'; + +const workflowPath = '.github/workflows/server-parity.yml'; +const workflowText = readFileSync(workflowPath, 'utf8'); +const classifier = resolve('scripts/conformance/server-parity/changes.mjs'); +const aggregate = resolve('scripts/conformance/server-parity/aggregate.mjs'); + +// Top-level job ids and each job's block of text. +function jobs(text) { + const body = text.slice(text.indexOf('\njobs:\n') + '\njobs:\n'.length); + const blocks = {}; + let current; + for (const line of body.split('\n')) { + const header = /^ {2}([a-z0-9-]+):$/.exec(line); + if (header) { + current = header[1]; + blocks[current] = ''; + } else if (current) { + blocks[current] += `${line}\n`; + } + } + return blocks; +} + +function triggers(text) { + const start = text.indexOf('\non:\n') + '\non:\n'.length; + const rest = text.slice(start); + return rest.slice(0, rest.search(/\n[a-z_-]+:/)); +} + +test('pull requests run without a path filter and push keeps the one path list', () => { + const on = triggers(workflowText); + const pullRequest = on.slice(on.indexOf(' pull_request:\n'), on.indexOf(' push:\n')); + assert.doesNotMatch(pullRequest, /paths|branches/); + const patterns = pushPathPatterns(workflowText); + assert.ok(patterns.includes('rust/**')); + assert.ok(patterns.includes('.github/workflows/server-parity.yml')); + assert.ok(patterns.includes('scripts/conformance/server-parity/**')); +}); + +test('path patterns follow GitHub filter semantics for the forms in use', () => { + assert.ok(patternRegExp('rust/**').test('rust/src/main.rs')); + assert.ok(patternRegExp('scripts/conformance/server-parity*').test('scripts/conformance/server-parity.sh')); + assert.ok(!patternRegExp('scripts/conformance/server-parity*').test('scripts/conformance/server-parity/run.mjs')); + assert.ok(!patternRegExp('tests/Unit/ServerParityRunnerTest.mjs').test('tests/Unit/ServerParityRunnerTestXmjs')); + assert.throws(() => patternRegExp('docs/{a,b}/**'), /Unsupported path filter pattern/); + assert.deepEqual(matchingFiles(['rust/**', 'docs/x.md'], ['README.md', 'rust/Cargo.toml', 'docs/x.md']), ['rust/Cargo.toml', 'docs/x.md']); +}); + +test('the fixture jobs run only through the classifier and the result waits for all of them', () => { + const blocks = jobs(workflowText); + const ids = Object.keys(blocks); + assert.equal(ids[0], 'changes'); + assert.match(blocks.changes, /^ {4}timeout-minutes: \d+$/m); + assert.match(blocks.changes, /run: \$\{\{ steps\.classify\.outputs\.run \}\}/); + assert.match(blocks['published-sdk-adapters'], /^ {4}needs: changes\n {4}if: \$\{\{ needs\.changes\.outputs\.run == 'true' \}\}$/m); + const fixtureJobs = ids.filter(id => id !== 'changes' && id !== 'result'); + for (const id of fixtureJobs.filter(id => id !== 'published-sdk-adapters')) { + // A skipped published-sdk-adapters skips these jobs; no job-level + // condition may override that. + assert.match(blocks[id], /^ {4}needs: published-sdk-adapters$/m, id); + assert.doesNotMatch(blocks[id], /^ {4}if:/m, id); + } + const result = blocks.result; + assert.equal(ids.at(-1), 'result'); + assert.match(result, /^ {4}name: Shared Server fixtures result$/m); + assert.doesNotMatch(result, /strategy:/); + assert.match(result, /^ {4}if: \$\{\{ always\(\) \}\}$/m); + const needs = [...result.matchAll(/^ {6}- ([a-z0-9-]+)$/gm)].map(match => match[1]); + assert.deepEqual(needs.sort(), ['changes', ...fixtureJobs].sort()); + assert.match(result, /NEEDS: \$\{\{ toJSON\(needs\) \}\}/); + assert.match(result, /run: node scripts\/conformance\/server-parity\/aggregate\.mjs/); +}); + +function runAggregate(needs) { + const text = typeof needs === 'string' ? needs : JSON.stringify(needs); + return spawnSync('node', [aggregate], {env: {...process.env, NEEDS: text}, encoding: 'utf8'}); +} + +const fixture = (result, run) => ({ + changes: {result: 'success', outputs: run === undefined ? {} : {run}}, + 'published-sdk-adapters': {result, outputs: {}}, + 'codec-deadline-health': {result, outputs: {}}, + 'mysql-execution': {result, outputs: {}}, + 'postgres-storage': {result, outputs: {}}, + 'execution-and-codec': {result, outputs: {}}, +}); + +test('the result passes when every fixture job passed', () => { + const outcome = runAggregate(fixture('success', 'true')); + assert.equal(outcome.status, 0, outcome.stderr); + assert.match(outcome.stdout, /Every fixture job passed/); +}); + +test('the result passes when no input changed and every fixture job was skipped', () => { + const outcome = runAggregate(fixture('skipped', 'false')); + assert.equal(outcome.status, 0, outcome.stderr); + assert.match(outcome.stdout, /every fixture job was skipped/); +}); + +for (const [label, needs, reason] of [ + ['a failed fixture job', {...fixture('success', 'true'), 'mysql-execution': {result: 'failure', outputs: {}}}, /must be success when run=true: mysql-execution=failure\./], + ['a cancelled fixture job', {...fixture('success', 'true'), 'execution-and-codec': {result: 'cancelled', outputs: {}}}, /must be success when run=true: execution-and-codec=cancelled\./], + ['a skipped fixture job when inputs changed', {...fixture('success', 'true'), 'postgres-storage': {result: 'skipped', outputs: {}}}, /must be success when run=true: postgres-storage=skipped\./], + ['a fixture job that ran when no input changed', {...fixture('skipped', 'false'), 'codec-deadline-health': {result: 'success', outputs: {}}}, /must be skipped when run=false: codec-deadline-health=success\./], + ['a failed classifier', {...fixture('skipped', 'false'), changes: {result: 'failure', outputs: {}}}, /classifier did not succeed \(failure\)/], + ['a cancelled classifier', {...fixture('skipped'), changes: {result: 'cancelled', outputs: {}}}, /classifier did not succeed \(cancelled\)/], + ['a missing classifier', (({changes, ...rest}) => rest)(fixture('success', 'true')), /classifier did not succeed \(undefined\)/], + ['a classifier with no decision', fixture('skipped'), /reported no decision \(undefined\)/], + ['a classifier with an unknown decision', fixture('skipped', 'maybe'), /reported no decision \(maybe\)/], + ['no fixture jobs', {changes: {result: 'success', outputs: {run: 'true'}}}, /No fixture jobs reported/], + ['a context that is not an object', [], /not an object/], +]) { + test(`the result fails on ${label}`, () => { + const decision = decide(needs); + assert.equal(decision.pass, false); + assert.match(decision.reason, reason); + const outcome = runAggregate(needs); + assert.equal(outcome.status, 1); + assert.match(outcome.stderr, reason); + }); +} + +test('the result fails when the needs context cannot be read', () => { + for (const text of ['', 'not json']) { + const outcome = runAggregate(text); + assert.equal(outcome.status, 1); + assert.match(outcome.stderr, /could not be read/); + } +}); + +// A pull request merge commit in a scratch repository with this workflow. +function pullRequest(change) { + const root = mkdtempSync(join(tmpdir(), 'parity-changes-')); + const git = (...args) => execFileSync('git', ['-c', 'user.name=t', '-c', 'user.email=t@example.invalid', ...args], {cwd: root, encoding: 'utf8'}).trim(); + const write = (path, text) => { + mkdirSync(dirname(join(root, path)), {recursive: true}); + writeFileSync(join(root, path), text); + }; + git('init', '-q', '-b', 'main'); + mkdirSync(join(root, '.github/workflows'), {recursive: true}); + copyFileSync(workflowPath, join(root, workflowPath)); + write('README.md', 'base\n'); + write('tests/Fixtures/ServerParity/case.json', '{}\n'); + git('add', '-A'); + git('commit', '-q', '-m', 'base'); + git('checkout', '-q', '-b', 'head'); + change({root, write}); + git('add', '-A'); + git('commit', '-q', '-m', 'head'); + const head = git('rev-parse', 'HEAD'); + git('checkout', '-q', 'main'); + write('MAIN.md', 'main moved\n'); + git('add', '-A'); + git('commit', '-q', '-m', 'main'); + git('merge', '-q', '--no-ff', '-m', 'merge', 'head'); + return {root, head}; +} + +function classify(root, env) { + const outcome = spawnSync('node', [classifier], {cwd: root, env: {...process.env, ...env}, encoding: 'utf8'}); + assert.equal(outcome.status, 0, outcome.stderr); + return outcome.stdout.trim(); +} + +for (const [label, change, expected] of [ + ['a documentation-only change skips the fixtures', ({write}) => write('docs/note.md', 'x\n'), 'run=false'], + ['a Rust change runs the fixtures', ({write}) => write('rust/src/lib.rs', 'x\n'), 'run=true'], + ['moving a fixture out of the corpus runs the fixtures', ({root, write}) => { + write('docs/.keep', ''); + renameSync(join(root, 'tests/Fixtures/ServerParity/case.json'), join(root, 'docs/case.json')); + }, 'run=true'], +]) { + test(`pull request classification: ${label}`, () => { + const {root, head} = pullRequest(change); + try { + assert.equal(classify(root, {EVENT_NAME: 'pull_request', PR_HEAD_SHA: head}), expected); + } finally { + rmSync(root, {recursive: true, force: true}); + } + }); +} + +test('pull request classification runs every fixture when HEAD is not that pull request', () => { + const {root} = pullRequest(({write}) => write('docs/note.md', 'x\n')); + try { + assert.equal(classify(root, {EVENT_NAME: 'pull_request', PR_HEAD_SHA: '0'.repeat(40)}), 'run=true'); + } finally { + rmSync(root, {recursive: true, force: true}); + } +}); + +for (const event of ['push', 'merge_group', 'workflow_dispatch', '']) { + test(`${event || 'an unnamed event'} runs every fixture job`, () => { + const {root, head} = pullRequest(({write}) => write('docs/note.md', 'x\n')); + try { + assert.equal(classify(root, {EVENT_NAME: event, PR_HEAD_SHA: head}), 'run=true'); + } finally { + rmSync(root, {recursive: true, force: true}); + } + }); +} From f086ab720b9bd78785d5db422803ca1a2dc0183f Mon Sep 17 00:00:00 2001 From: Richard McDaniel <1130888+rmcdaniel@users.noreply.github.com> Date: Sat, 24 Oct 2026 06:42:11 +0000 Subject: [PATCH 2/2] Run the fixtures classifier and result through shell helpers The Docker workflow isolation policy accepts only literal .sh and .py repository helpers in workflow steps, so the new jobs' direct node calls failed Public Boundary. changes.sh and aggregate.sh now run the Node scripts from the repository root, as the workflow does, and the classifier tests go through changes.sh. --- .github/workflows/server-parity.yml | 8 +++++--- .../conformance/server-parity/aggregate.sh | 5 +++++ scripts/conformance/server-parity/changes.sh | 5 +++++ tests/Unit/ServerParityAggregateTest.mjs | 20 ++++++++++++++++--- 4 files changed, 32 insertions(+), 6 deletions(-) create mode 100755 scripts/conformance/server-parity/aggregate.sh create mode 100755 scripts/conformance/server-parity/changes.sh diff --git a/.github/workflows/server-parity.yml b/.github/workflows/server-parity.yml index f5759de26..af8700d94 100644 --- a/.github/workflows/server-parity.yml +++ b/.github/workflows/server-parity.yml @@ -97,7 +97,7 @@ jobs: env: EVENT_NAME: ${{ github.event_name }} PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }} - run: node scripts/conformance/server-parity/changes.mjs >> "$GITHUB_OUTPUT" + run: bash scripts/conformance/server-parity/changes.sh >> "$GITHUB_OUTPUT" published-sdk-adapters: needs: changes @@ -1870,10 +1870,12 @@ jobs: steps: - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 with: - sparse-checkout: scripts/conformance/server-parity/aggregate.mjs + sparse-checkout: | + scripts/conformance/server-parity/aggregate.sh + scripts/conformance/server-parity/aggregate.mjs sparse-checkout-cone-mode: false persist-credentials: false - name: Require every fixture job to pass, or every one to be skipped for unchanged inputs env: NEEDS: ${{ toJSON(needs) }} - run: node scripts/conformance/server-parity/aggregate.mjs + run: bash scripts/conformance/server-parity/aggregate.sh diff --git a/scripts/conformance/server-parity/aggregate.sh b/scripts/conformance/server-parity/aggregate.sh new file mode 100755 index 000000000..013908c66 --- /dev/null +++ b/scripts/conformance/server-parity/aggregate.sh @@ -0,0 +1,5 @@ +#!/usr/bin/env bash +# The Shared Server fixtures result, from the repository root. NEEDS is the +# result job's needs context as JSON. The decision rules live in aggregate.mjs. +set -euo pipefail +exec node scripts/conformance/server-parity/aggregate.mjs diff --git a/scripts/conformance/server-parity/changes.sh b/scripts/conformance/server-parity/changes.sh new file mode 100755 index 000000000..4f8635f0a --- /dev/null +++ b/scripts/conformance/server-parity/changes.sh @@ -0,0 +1,5 @@ +#!/usr/bin/env bash +# Prints run=true or run=false for the Shared Server fixtures workflow, from +# the repository root. The decision rules live in changes.mjs. +set -euo pipefail +exec node scripts/conformance/server-parity/changes.mjs diff --git a/tests/Unit/ServerParityAggregateTest.mjs b/tests/Unit/ServerParityAggregateTest.mjs index 768e699fe..0eab0f4df 100644 --- a/tests/Unit/ServerParityAggregateTest.mjs +++ b/tests/Unit/ServerParityAggregateTest.mjs @@ -14,8 +14,9 @@ import {decide} from '../../scripts/conformance/server-parity/aggregate.mjs'; const workflowPath = '.github/workflows/server-parity.yml'; const workflowText = readFileSync(workflowPath, 'utf8'); -const classifier = resolve('scripts/conformance/server-parity/changes.mjs'); const aggregate = resolve('scripts/conformance/server-parity/aggregate.mjs'); +const aggregateWrapper = 'scripts/conformance/server-parity/aggregate.sh'; +const classifierWrapper = 'scripts/conformance/server-parity/changes.sh'; // Top-level job ids and each job's block of text. function jobs(text) { @@ -65,6 +66,7 @@ test('the fixture jobs run only through the classifier and the result waits for assert.equal(ids[0], 'changes'); assert.match(blocks.changes, /^ {4}timeout-minutes: \d+$/m); assert.match(blocks.changes, /run: \$\{\{ steps\.classify\.outputs\.run \}\}/); + assert.match(blocks.changes, /run: bash scripts\/conformance\/server-parity\/changes\.sh >> "\$GITHUB_OUTPUT"$/m); assert.match(blocks['published-sdk-adapters'], /^ {4}needs: changes\n {4}if: \$\{\{ needs\.changes\.outputs\.run == 'true' \}\}$/m); const fixtureJobs = ids.filter(id => id !== 'changes' && id !== 'result'); for (const id of fixtureJobs.filter(id => id !== 'published-sdk-adapters')) { @@ -81,7 +83,8 @@ test('the fixture jobs run only through the classifier and the result waits for const needs = [...result.matchAll(/^ {6}- ([a-z0-9-]+)$/gm)].map(match => match[1]); assert.deepEqual(needs.sort(), ['changes', ...fixtureJobs].sort()); assert.match(result, /NEEDS: \$\{\{ toJSON\(needs\) \}\}/); - assert.match(result, /run: node scripts\/conformance\/server-parity\/aggregate\.mjs/); + assert.match(result, /run: bash scripts\/conformance\/server-parity\/aggregate\.sh$/m); + assert.match(result, /sparse-checkout: \|\n {12}scripts\/conformance\/server-parity\/aggregate\.sh\n {12}scripts\/conformance\/server-parity\/aggregate\.mjs\n/); }); function runAggregate(needs) { @@ -133,6 +136,14 @@ for (const [label, needs, reason] of [ }); } +test('the workflow wrapper passes the result through, from the repository root', () => { + const run = needs => spawnSync('bash', [aggregateWrapper], {env: {...process.env, NEEDS: JSON.stringify(needs)}, encoding: 'utf8'}); + assert.equal(run(fixture('success', 'true')).status, 0); + const failed = run({...fixture('success', 'true'), 'mysql-execution': {result: 'failure', outputs: {}}}); + assert.equal(failed.status, 1); + assert.match(failed.stderr, /mysql-execution=failure/); +}); + test('the result fails when the needs context cannot be read', () => { for (const text of ['', 'not json']) { const outcome = runAggregate(text); @@ -152,6 +163,8 @@ function pullRequest(change) { git('init', '-q', '-b', 'main'); mkdirSync(join(root, '.github/workflows'), {recursive: true}); copyFileSync(workflowPath, join(root, workflowPath)); + mkdirSync(join(root, dirname(classifierWrapper)), {recursive: true}); + for (const file of [classifierWrapper, 'scripts/conformance/server-parity/changes.mjs']) copyFileSync(file, join(root, file)); write('README.md', 'base\n'); write('tests/Fixtures/ServerParity/case.json', '{}\n'); git('add', '-A'); @@ -170,7 +183,8 @@ function pullRequest(change) { } function classify(root, env) { - const outcome = spawnSync('node', [classifier], {cwd: root, env: {...process.env, ...env}, encoding: 'utf8'}); + // The workflow runs the wrapper from the repository root. + const outcome = spawnSync('bash', [classifierWrapper], {cwd: root, env: {...process.env, ...env}, encoding: 'utf8'}); assert.equal(outcome.status, 0, outcome.stderr); return outcome.stdout.trim(); }