diff --git a/.github/workflows/test-eql.yml b/.github/workflows/test-eql.yml index b3d0c2d60..03bfa6ba9 100644 --- a/.github/workflows/test-eql.yml +++ b/.github/workflows/test-eql.yml @@ -31,8 +31,8 @@ name: "Test EQL" # in every shard instead of saying what was missing. # # NB: NO path filter on `pull_request`. A workflow skipped by a path filter -# leaves its required checks stuck Pending and blocks merge. Relevance is -# computed by the `changes` job and applied per-job via `if:` instead. `push` +# leaves its required checks stuck Pending and blocks merge. The `selector` job +# decides which jobs a pull request needs, and each job's `if:` reads it. `push` # carries one because nothing waits on a push run, so a skipped one costs # nothing — and the two lists are kept identical by # `scripts/__tests__/eql-workflow-filters.test.mjs`. @@ -70,7 +70,7 @@ on: pull_request: {} push: branches: [main] - # Identical to the `relevant:` filter in the `changes` job below. GitHub + # Identical to the union of the `selector` job's filters below. GitHub # Actions has no YAML anchors, so the list is written twice; the guard named # above fails a one-sided edit. # @@ -148,14 +148,24 @@ concurrency: cancel-in-progress: ${{ github.event_name == 'pull_request' }} jobs: - # Runs on EVERY event and MUST always succeed (never skipped, never failed) — - # downstream heavy jobs `needs: [changes]`, and a skipped/failed `changes` - # would either skip the merge-queue matrix or deadlock `ci-required`. - changes: - name: "Detect relevant changes" - runs-on: blacksmith-16vcpu-ubuntu-2204 + # The selector job: it decides which gated jobs a pull request needs. It runs + # on EVERY event and is never skipped, because every gated job `needs:` it. + # When it cannot decide it FAILS, which fails `ci-required`: a selector that + # answered nothing would skip every gated job, and `ci-required` counts + # skipped as passed (ADR 0003). + # + # On a GitHub-hosted runner because every gated job waits for it, and a + # self-provisioned machine takes longer to start. + selector: + name: "Selector" + runs-on: ubuntu-latest + timeout-minutes: 10 + # Each output is named after the filter that decides it; + # `scripts/__tests__/eql-selector.test.mjs` follows a job's `if:` to its + # filter by that name. outputs: - relevant: ${{ steps.r.outputs.relevant }} + eql: ${{ steps.r.outputs.eql }} + rust_crates: ${{ steps.r.outputs.rust_crates }} steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 with: @@ -177,8 +187,9 @@ jobs: # matrix on changes that cannot affect EQL, and (worse) NOT firing on # EQL's own `src/`, since nothing at the repo root is called that. # - # Identical to the `push` `paths:` list at the top of this file — the - # same question asked of two mechanisms GitHub gives no way to share. + # The union of the two filters is identical to the `push` `paths:` + # list at the top of this file — the same question asked of two + # mechanisms GitHub gives no way to share. # `scripts/__tests__/eql-workflow-filters.test.mjs` compares them, and # also fails any entry here that matches no tracked file (a dead # `packages/eql/sql/**` sat in this list from the port: there is no @@ -191,8 +202,14 @@ jobs: # copies are held EQUAL — see the note above the `push` list, and the # reasoning on `bench-eql.yml`'s own filter for why equal beats # minimal. + # + # `rust_crates` adds the stack-* crates to `eql`. Only `rust-crates` + # compiles them, through eql-bindings' `stack-encrypt` feature; + # `tests/sqlx` and the codegen crates never enable it. With the crates + # on one shared list, a change to stack-encrypt alone ran the whole + # Postgres suite. dorny flattens the `*eql` alias into the list. filters: | - relevant: + eql: &eql - ".github/workflows/test-eql.yml" - ".github/actions/require-cs-secrets/**" - "packages/eql/src/**" @@ -207,6 +224,8 @@ jobs: - "packages/eql/docker/**" - "packages/eql/README.md" - "packages/eql/SUPABASE.md" + rust_crates: + - *eql - "packages/stack-encrypt/**" - "packages/stack-encrypt-derive/**" - "packages/stack-kms/**" @@ -217,25 +236,38 @@ jobs: # Explicit default (not `|| 'true'`, which trips GitHub's inconsistent # treatment of the string 'false'). push/schedule/merge_group/dispatch # never run the filter above — it needs a base ref that only a pull - # request has — so they take the default, and `push` is already narrowed + # request has — so they select every job, and `push` is already narrowed # by its own `on:`-level `paths:`. # - # The event arrives through `env:`, not through a `${{ }}` interpolated - # into the body. That is what lets + # On a pull request, an answer other than `true` or `false` means the + # filter did not decide, so the step fails rather than write it. + # + # The event and the answers arrive through `env:`, not through a `${{ }}` + # interpolated into the body. That is what lets + # `scripts/__tests__/eql-selector.test.mjs` and # `scripts/__tests__/eql-matrix-triggers.test.mjs` EXECUTE this script - # once per event and read back what it wrote, rather than reimplementing - # the branch and agreeing with a rewritten one by luck. + # and read back what it wrote, rather than reimplementing the branch and + # agreeing with a rewritten one by luck. - id: r env: EVENT_NAME: ${{ github.event_name }} - FILTER_RELEVANT: ${{ steps.f.outputs.relevant }} + EQL: ${{ steps.f.outputs.eql }} + RUST_CRATES: ${{ steps.f.outputs.rust_crates }} run: | set -euo pipefail - if [ "$EVENT_NAME" = "pull_request" ]; then - echo "relevant=$FILTER_RELEVANT" >> "$GITHUB_OUTPUT" - else - echo "relevant=true" >> "$GITHUB_OUTPUT" + if [ "$EVENT_NAME" != "pull_request" ]; then + EQL=true + RUST_CRATES=true fi + for answer in "eql=$EQL" "rust_crates=$RUST_CRATES"; do + case "${answer#*=}" in + true|false) echo "$answer" >> "$GITHUB_OUTPUT" ;; + *) + echo "::error::The path filter answered '${answer#*=}' for ${answer%%=*}. The selector cannot decide which jobs to run." + exit 1 + ;; + esac + done # Pure bash; no checkout/toolchain. Derives the PG-version + shard fan-out # from the event: PR and manual dispatch -> PG17 x 4 shards (latency lane); @@ -292,9 +324,10 @@ jobs: # when relevant files changed (docs-only PRs never pay the ~4-min compile). build-archive: name: "Build test archive" - needs: [changes] - # This repo is PUBLIC and accepts fork PRs (the approval policy only gates - # first-time contributors). build-archive is the sole holder of the CS_* + needs: [selector] + # This repo is PUBLIC and accepts fork PRs (a maintainer approves each + # outside contributor's run, and an approved run executes the fork's + # code). build-archive is the sole holder of the CS_* # credentials below, so it must never run on a fork PR. The trailing clause # is the only thing that skips it: a fork pull request. Every other event — # push, schedule, merge_group, dispatch, and a same-repo branch PR — runs; @@ -310,7 +343,7 @@ jobs: # `scripts/__tests__/workflow-dispatch-job-conditions.test.mjs`. if: >- (github.event_name != 'pull_request' - || needs.changes.outputs.relevant == 'true') + || needs.selector.outputs.eql == 'true') && (github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository) runs-on: blacksmith-16vcpu-ubuntu-2204 @@ -392,11 +425,11 @@ jobs: # shards start right after build-archive. test: name: "Shard PG${{ matrix.postgres-version }} ${{ matrix.shard }}/${{ needs.setup.outputs.shard-total }}" - needs: [changes, setup, build-archive] + needs: [selector, setup, build-archive] # Exclusion, not an allowlist — see the note on `build-archive`. if: >- github.event_name != 'pull_request' - || needs.changes.outputs.relevant == 'true' + || needs.selector.outputs.eql == 'true' runs-on: blacksmith-16vcpu-ubuntu-2204 strategy: fail-fast: false @@ -454,11 +487,11 @@ jobs: # of the old per-version test job so they run ONCE per version, not per shard. validate: name: "Validate (Postgres ${{ matrix.postgres-version }})" - needs: [changes, setup] + needs: [selector, setup] # Exclusion, not an allowlist — see the note on `build-archive`. if: >- github.event_name != 'pull_request' - || needs.changes.outputs.relevant == 'true' + || needs.selector.outputs.eql == 'true' runs-on: blacksmith-16vcpu-ubuntu-2204 strategy: fail-fast: false @@ -513,11 +546,11 @@ jobs: schema: name: "JSON Schema validation" - needs: [changes] + needs: [selector] # Exclusion, not an allowlist — see the note on `build-archive`. if: >- github.event_name != 'pull_request' - || needs.changes.outputs.relevant == 'true' + || needs.selector.outputs.eql == 'true' runs-on: blacksmith-16vcpu-ubuntu-2204 steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 @@ -542,11 +575,13 @@ jobs: rust-crates: name: "Rust workspace crates" - needs: [changes] - # Exclusion, not an allowlist — see the note on `build-archive`. + needs: [selector] + # Exclusion, not an allowlist — see the note on `build-archive`. Reads + # `rust_crates`, not `eql`: this is the one job that compiles the stack-* + # crates. if: >- github.event_name != 'pull_request' - || needs.changes.outputs.relevant == 'true' + || needs.selector.outputs.rust_crates == 'true' runs-on: blacksmith-16vcpu-ubuntu-2204 services: encryption-postgres: @@ -670,11 +705,11 @@ jobs: codegen: name: "Encrypted-domain codegen" - needs: [changes] + needs: [selector] # Exclusion, not an allowlist — see the note on `build-archive`. if: >- github.event_name != 'pull_request' - || needs.changes.outputs.relevant == 'true' + || needs.selector.outputs.eql == 'true' runs-on: blacksmith-16vcpu-ubuntu-2204 steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 @@ -702,11 +737,11 @@ jobs: self-contained-v3: name: "eql_v3 self-containment" - needs: [changes] + needs: [selector] # Exclusion, not an allowlist — see the note on `build-archive`. if: >- github.event_name != 'pull_request' - || needs.changes.outputs.relevant == 'true' + || needs.selector.outputs.eql == 'true' runs-on: blacksmith-16vcpu-ubuntu-2204 steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 @@ -738,11 +773,11 @@ jobs: matrix-coverage: name: "Matrix coverage inventory" - needs: [changes] + needs: [selector] # Exclusion, not an allowlist — see the note on `build-archive`. if: >- github.event_name != 'pull_request' - || needs.changes.outputs.relevant == 'true' + || needs.selector.outputs.eql == 'true' runs-on: blacksmith-16vcpu-ubuntu-2204 steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 @@ -780,11 +815,11 @@ jobs: splinter: name: "Supabase splinter" - needs: [changes] + needs: [selector] # Exclusion, not an allowlist — see the note on `build-archive`. if: >- github.event_name != 'pull_request' - || needs.changes.outputs.relevant == 'true' + || needs.selector.outputs.eql == 'true' runs-on: blacksmith-16vcpu-ubuntu-2204 env: POSTGRES_VERSION: "17" @@ -947,12 +982,12 @@ jobs: # fork-PR guard to keep the secrets off fork runs. e2e: name: "e2e property suite (fresh encryption)" - needs: [changes, setup] + needs: [selector, setup] # Exclusion, not an allowlist — see the note on `build-archive`, which # carries the same two clauses for the same two reasons. if: >- (github.event_name != 'pull_request' - || needs.changes.outputs.relevant == 'true') + || needs.selector.outputs.eql == 'true') && (github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository) runs-on: blacksmith-16vcpu-ubuntu-2204 @@ -1002,7 +1037,7 @@ jobs: # source job is itself in `needs` and reports failure. ci-required: name: "ci-required" - needs: [changes, setup, build-archive, test, validate, schema, rust-crates, + needs: [selector, setup, build-archive, test, validate, schema, rust-crates, codegen, self-contained-v3, matrix-coverage, splinter, docs-static, known-failures, doc-anchors, e2e] if: always() diff --git a/scripts/__tests__/eql-selector.test.mjs b/scripts/__tests__/eql-selector.test.mjs new file mode 100644 index 000000000..99afe4fab --- /dev/null +++ b/scripts/__tests__/eql-selector.test.mjs @@ -0,0 +1,212 @@ +import { spawnSync } from 'node:child_process' +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import yaml from 'js-yaml' +import { afterAll, describe, expect, it } from 'vitest' +import { runsWhen } from './lib/expressions.mjs' +import { pathsGlobToRegExp } from './lib/paths-filter.mjs' +import { readWorkflow } from './lib/workflows.mjs' + +/** + * Test EQL's selector job decides which checks a pull request runs, by + * matching the changed files against each check's trigger paths. + * + * Only `rust-crates` compiles the stack-* crates, through eql-bindings' + * `stack-encrypt` feature. Every other job builds `tests/sqlx` or the + * codegen crates, which do not enable it. While the crates sat on the one list + * that gated every job, a pull request touching only stack-encrypt ran the + * whole Postgres suite. + * + * The selector is exercised the way GitHub runs it: the filter results go + * into the real decision script, and each job's real `if:` reads its outputs. + */ + +const WORKFLOW = '.github/workflows/test-eql.yml' +const SELECTOR = 'selector' +const REPO = 'cipherstash/stack' + +const wf = readWorkflow(WORKFLOW) +const selector = wf?.jobs?.[SELECTOR] + +const stepOutput = (expression) => + /steps\.([A-Za-z0-9_-]+)\.outputs\.([A-Za-z0-9_-]+)/.exec( + String(expression ?? ''), + ) + +const filterStep = (selector?.steps ?? []).find((step) => + String(step?.uses ?? '').startsWith('dorny/paths-filter@'), +) + +// dorny flattens a list that includes an anchored list; so does this. +const FILTERS = Object.fromEntries( + Object.entries(yaml.load(filterStep?.with?.filters ?? '') ?? {}).map( + ([name, globs]) => [name, [globs].flat(Infinity)], + ), +) + +const decideStepId = stepOutput(Object.values(selector?.outputs ?? {})[0])?.[1] +const decideStep = (selector?.steps ?? []).find( + (step) => step?.id === decideStepId, +) + +const GATED = Object.entries(wf?.jobs ?? {}) + .map(([id, job]) => ({ + id, + job, + output: new RegExp(`needs\\.${SELECTOR}\\.outputs\\.([A-Za-z0-9_-]+)`).exec( + String(job?.if ?? ''), + )?.[1], + })) + .filter((entry) => entry.output) + +const SCRATCH = mkdtempSync(join(tmpdir(), 'eql-selector-')) +afterAll(() => rmSync(SCRATCH, { recursive: true, force: true })) +let runCounter = 0 + +/** + * Runs the decision script with the filter results GitHub would substitute. + * An env entry that names no filter result gets the empty string, as GitHub + * substitutes for an expression that resolves to nothing. + */ +function decide(eventName, filterResults) { + runCounter += 1 + const outputFile = join(SCRATCH, `outputs-${runCounter}.txt`) + writeFileSync(outputFile, '') + const env = { PATH: process.env.PATH, GITHUB_OUTPUT: outputFile } + for (const [key, value] of Object.entries(decideStep?.env ?? {})) { + const text = String(value) + if (/github\.event_name/.test(text)) env[key] = eventName + else env[key] = filterResults[stepOutput(text)?.[2]] ?? '' + } + const run = spawnSync('bash', ['-c', String(decideStep?.run)], { + cwd: SCRATCH, + env, + encoding: 'utf8', + }) + const written = Object.fromEntries( + readFileSync(outputFile, 'utf8') + .split('\n') + .filter(Boolean) + .map((line) => [ + line.slice(0, line.indexOf('=')), + line.slice(line.indexOf('=') + 1), + ]), + ) + const outputs = Object.fromEntries( + Object.entries(selector?.outputs ?? {}).map(([name, expression]) => [ + name, + written[stepOutput(expression)?.[2]], + ]), + ) + return { status: run.status, stderr: `${run.stdout}${run.stderr}`, outputs } +} + +/** The gated jobs a same-repository pull request runs, given its files. */ +function jobsRunFor(changedFiles) { + const filterResults = Object.fromEntries( + Object.entries(FILTERS).map(([name, globs]) => [ + name, + String( + changedFiles.some((file) => + globs.some((glob) => pathsGlobToRegExp(glob).test(file)), + ), + ), + ]), + ) + const { status, stderr, outputs } = decide('pull_request', filterResults) + expect(status, stderr).toBe(0) + const context = { + github: { + event_name: 'pull_request', + repository: REPO, + event: { pull_request: { head: { repo: { full_name: REPO } } } }, + }, + needs: { [SELECTOR]: { result: 'success', outputs } }, + vars: {}, + } + return GATED.filter(({ job }) => runsWhen(job.if, context)) + .map(({ id }) => id) + .sort() +} + +const ALL_GATED = GATED.map(({ id }) => id).sort() + +describe('the Test EQL selector job', () => { + it('finds the selector, its filters, and the jobs it gates', () => { + expect(selector, `${WORKFLOW} has no \`${SELECTOR}\` job.`).toBeTruthy() + expect( + Object.keys(FILTERS), + 'The selector has no path filters.', + ).not.toEqual([]) + expect( + decideStep, + 'No step produces the selector job outputs.', + ).toBeTruthy() + expect(ALL_GATED).toContain('rust-crates') + expect(ALL_GATED.length).toBeGreaterThan(1) + }) + + it('runs on a GitHub-hosted runner', () => { + // ADR 0003: a RunsOn machine takes 40 to 50 seconds to start, and every + // other job in the run waits for this one. + expect(String(selector?.['runs-on'])).toMatch(/^ubuntu-/) + }) + + it('names each output after the path filter that decides it', () => { + // eql-suite-ci.test.mjs follows a job's `if:` to its filter by this name. + for (const { id, output } of GATED) { + expect( + Object.keys(FILTERS), + `"${id}" reads \`needs.${SELECTOR}.outputs.${output}\`, and the selector has no path filter called "${output}".`, + ).toContain(output) + } + }) + + it('runs only rust-crates for a change to a stack crate', () => { + expect(jobsRunFor(['packages/stack-encrypt/src/lib.rs'])).toEqual([ + 'rust-crates', + ]) + expect(jobsRunFor(['Cargo.toml'])).toEqual(['rust-crates']) + }) + + it('runs every gated job for a change to EQL', () => { + expect(jobsRunFor(['packages/eql/src/v3/schema.sql'])).toEqual(ALL_GATED) + expect(jobsRunFor([WORKFLOW])).toEqual(ALL_GATED) + }) + + it('runs no gated job for a change outside EQL and the crates', () => { + expect( + jobsRunFor(['languages/typescript/packages/stack/src/index.ts']), + ).toEqual([]) + }) + + it('opens every output on events other than pull_request', () => { + // The filter runs only on pull_request. Every other event takes the + // default, and a wrong default skips every gated job behind a green run. + for (const event of [ + 'push', + 'schedule', + 'merge_group', + 'workflow_dispatch', + ]) { + const { status, outputs } = decide(event, {}) + expect(status).toBe(0) + for (const name of Object.keys(selector?.outputs ?? {})) + expect(outputs[name], `${name} on ${event}`).toBe('true') + } + }) + + it('fails when a pull request filter gives no answer', () => { + // A missing answer would skip every gated job, and `ci-required` counts + // skipped as passed: a green run that ran nothing. + expect(decideStep?.run).toBeTruthy() + for (const bad of [ + {}, + Object.fromEntries(Object.keys(FILTERS).map((name) => [name, 'maybe'])), + ]) { + const { status } = decide('pull_request', bad) + expect(status).not.toBe(0) + } + }) +}) diff --git a/scripts/__tests__/eql-suite-ci.test.mjs b/scripts/__tests__/eql-suite-ci.test.mjs index ee00032a8..2f37dcc38 100644 --- a/scripts/__tests__/eql-suite-ci.test.mjs +++ b/scripts/__tests__/eql-suite-ci.test.mjs @@ -209,13 +209,15 @@ describe('the relevance filter still selects the imported tree', () => { * parsed rather than pattern-matched — a regex over the raw block would also * match the surrounding commentary. */ - const filterPaths = Object.values(wf?.jobs ?? {}) - .flatMap((job) => job?.steps ?? []) - .filter((step) => (step?.uses ?? '').startsWith('dorny/paths-filter@')) - .flatMap((step) => { - const filters = yaml.load(step?.with?.filters ?? '') - return Object.values(filters ?? {}).flat() - }) + const filters = Object.assign( + {}, + ...Object.values(wf?.jobs ?? {}) + .flatMap((job) => job?.steps ?? []) + .filter((step) => (step?.uses ?? '').startsWith('dorny/paths-filter@')) + .map((step) => yaml.load(step?.with?.filters ?? '') ?? {}), + ) + // A filter that includes an anchored list parses to a nested list. + const filterPaths = [...new Set(Object.values(filters).flat(Infinity))] it('finds the paths-filter step', () => { // Without this the check below iterates an empty list and passes having @@ -395,17 +397,29 @@ describe('the relevance filter still selects the imported tree', () => { // The other direction: a path dependency outside the subtree that the // filter does NOT select is a job that compiles a crate and never runs on // a change to it — the gap commit 7219ea1ad repaired by hand, now derived. + // + // Checked against the filter the `rust-crates` job reads, not the union: + // that job is the one that compiles them, and the other jobs' filter + // leaves them out on purpose (eql-selector.test.mjs). + const filterName = /needs\.selector\.outputs\.([A-Za-z0-9_-]+)/.exec( + String(wf?.jobs?.['rust-crates']?.if ?? ''), + )?.[1] + const rustCratesPaths = [filters[filterName] ?? []].flat(Infinity) + expect( + rustCratesPaths, + `The \`rust-crates\` job in ${EQL_WORKFLOW} reads no selector filter, so this check has nothing to compare.`, + ).not.toEqual([]) const missing = externalPathDependencies.dirs .map((dir) => `${dir}/**`) - .filter((glob) => !filterPaths.includes(glob)) + .filter((glob) => !rustCratesPaths.includes(glob)) if ( externalPathDependencies.inherits && - !filterPaths.includes('Cargo.toml') + !rustCratesPaths.includes('Cargo.toml') ) missing.push('Cargo.toml') expect( missing, - `The EQL workspace compiles these by path and the relevance filter in ${EQL_WORKFLOW} does not select them, so the \`rust-crates\` job skips on a change that can break it. Add each to the filter — and, because the three copies are held equal, to the \`push: paths:\` list and to bench-eql.yml in the same edit.\n${missing.map((p) => ` ${p}`).join('\n')}`, + `The EQL workspace compiles these by path and the \`${filterName}\` filter in ${EQL_WORKFLOW} does not select them, so the \`rust-crates\` job skips on a change that can break it. Add each to that filter — and, because the copies are held equal, to the \`push: paths:\` list and to bench-eql.yml in the same edit.\n${missing.map((p) => ` ${p}`).join('\n')}`, ).toEqual([]) }) }) diff --git a/scripts/__tests__/eql-workflow-filters.test.mjs b/scripts/__tests__/eql-workflow-filters.test.mjs index 5f230772d..a6fb91960 100644 --- a/scripts/__tests__/eql-workflow-filters.test.mjs +++ b/scripts/__tests__/eql-workflow-filters.test.mjs @@ -3,6 +3,7 @@ import { readFileSync } from 'node:fs' import { join } from 'node:path' import yaml from 'js-yaml' import { describe, expect, it } from 'vitest' +import { pathsGlobToRegExp } from './lib/paths-filter.mjs' import { REPO_ROOT } from './lib/repo-root.mjs' import { readWorkflow } from './lib/workflows.mjs' @@ -12,9 +13,10 @@ import { readWorkflow } from './lib/workflows.mjs' * * `test-eql.yml` writes its list TWICE and in two different mechanisms: an * `on: push: paths:` filter (which decides whether the workflow starts at all - * after a merge) and a `dorny/paths-filter` `relevant:` filter (which decides - * whether the heavy jobs run inside a pull-request run). GitHub gives no way to - * share them. `bench-eql.yml` writes a third copy, for the same tree, on its own + * after a merge) and the selector job's `dorny/paths-filter` filters (which + * decide which heavy jobs run inside a pull-request run). GitHub gives no way + * to share them. The selector splits its list by job, so the copy compared + * here is the union of its filters. `bench-eql.yml` writes a third copy, for the same tree, on its own * `push` trigger. * * Three properties, and each of them was violated by the imported files: @@ -73,21 +75,26 @@ function pushPaths(relPath) { /** * Every path listed by a `dorny/paths-filter` step, across every filter it - * declares. `filters:` is a YAML document embedded in a YAML string, so it is - * parsed rather than pattern-matched — a regex over the raw block would also - * match the commentary around it. + * declares, once each. `filters:` is a YAML document embedded in a YAML string, + * so it is parsed rather than pattern-matched — a regex over the raw block + * would also match the commentary around it. */ function dornyFilterPaths(relPath) { const wf = readWorkflow(relPath) - return Object.values(wf?.jobs ?? {}) - .flatMap((job) => job?.steps ?? []) - .filter((step) => - String(step?.uses ?? '').startsWith('dorny/paths-filter@'), - ) - .flatMap((step) => - Object.values(yaml.load(step?.with?.filters ?? '') ?? {}), - ) - .flat() + return [ + ...new Set( + Object.values(wf?.jobs ?? {}) + .flatMap((job) => job?.steps ?? []) + .filter((step) => + String(step?.uses ?? '').startsWith('dorny/paths-filter@'), + ) + .flatMap((step) => + Object.values(yaml.load(step?.with?.filters ?? '') ?? {}), + ) + // An entry that is an anchored list parses to a nested list. + .flat(Infinity), + ), + ] } /** The local composite actions a workflow reaches through `uses: ./…`. */ @@ -114,37 +121,7 @@ function trackedUnder(pathspec) { .filter(Boolean) } -/** - * Approximates picomatch, which is what `dorny/paths-filter` and GitHub's own - * `paths:` matcher are built on: `*` stops at a separator, `**` does not. - * - * No implicit-descendants tail here, unlike the `@actions/glob` approximation - * in `ffi-binding-action.test.mjs` — a bare directory name is NOT a prefix - * match in a `paths:` filter, which is the difference that makes a - * `.../require-cs-secrets` entry without its `/**` cover nothing. - */ -function globToRegExp(pattern) { - let source = '^' - for (let i = 0; i < pattern.length; i += 1) { - const ch = pattern[i] - if (ch === '*') { - if (pattern[i + 1] === '*') { - source += '.*' - i += 1 - if (pattern[i + 1] === '/') i += 1 - } else { - source += '[^/]*' - } - } else if (ch === '?') { - source += '[^/]' - } else { - source += ch.replace(/[.+^${}()|[\]\\]/g, '\\$&') - } - } - return new RegExp(`${source}$`) -} - -const matches = (pattern, path) => globToRegExp(pattern).test(path) +const matches = (pattern, path) => pathsGlobToRegExp(pattern).test(path) /** Everything in the index a filter entry could plausibly select. */ const TRACKED = trackedUnder('.') @@ -152,7 +129,7 @@ const TRACKED = trackedUnder('.') /** The list each mechanism carries, keyed for the messages below. */ const LISTS = [ { id: `${TEST_EQL} (on.push.paths)`, entries: pushPaths(TEST_EQL) }, - { id: `${TEST_EQL} (dorny relevant:)`, entries: dornyFilterPaths(TEST_EQL) }, + { id: `${TEST_EQL} (selector filters)`, entries: dornyFilterPaths(TEST_EQL) }, { id: `${BENCH_EQL} (on.push.paths)`, entries: pushPaths(BENCH_EQL) }, ] diff --git a/scripts/__tests__/lib/paths-filter.mjs b/scripts/__tests__/lib/paths-filter.mjs index 29710f536..57e77c10c 100644 --- a/scripts/__tests__/lib/paths-filter.mjs +++ b/scripts/__tests__/lib/paths-filter.mjs @@ -1,3 +1,28 @@ +// Approximates picomatch, the matcher behind dorny/paths-filter and GitHub's +// `paths:`: `*` stops at a separator and `**` does not. A bare directory name +// is not a prefix match, so `.../require-cs-secrets` without `/**` covers +// nothing. +export function pathsGlobToRegExp(pattern) { + let source = '^' + for (let i = 0; i < pattern.length; i += 1) { + const ch = pattern[i] + if (ch === '*') { + if (pattern[i + 1] === '*') { + source += '.*' + i += 1 + if (pattern[i + 1] === '/') i += 1 + } else { + source += '[^/]*' + } + } else if (ch === '?') { + source += '[^/]' + } else { + source += ch.replace(/[.+^${}()|[\]\\]/g, '\\$&') + } + } + return new RegExp(`${source}$`) +} + const globPrefix = (pattern) => { const segments = pattern.split('/') const at = segments.findIndex((segment) => /[*?[]/.test(segment)) diff --git a/scripts/__tests__/workflow-dispatch-job-conditions.test.mjs b/scripts/__tests__/workflow-dispatch-job-conditions.test.mjs index 3b02b86e7..e727e71fe 100644 --- a/scripts/__tests__/workflow-dispatch-job-conditions.test.mjs +++ b/scripts/__tests__/workflow-dispatch-job-conditions.test.mjs @@ -174,13 +174,13 @@ function extractForkClause(condition) { * held open — otherwise a job that skips for an unrelated reason reads as a * job the fork guard skipped, and the assertion below would be satisfied by * the wrong mechanism. `test-eql.yml`'s two credentialed jobs are `&&`-ed with - * `needs.changes.outputs.relevant == 'true'`, which is that unrelated reason: + * `needs.selector.outputs.eql == 'true'`, which is that unrelated reason: * with `needs` absent the path resolves to null, `null == 'true'` is false, * and both jobs read as skipped on EVERY event including the ones that must * run. */ const PERMISSIVE_NEEDS = { - changes: { outputs: { relevant: 'true' } }, + selector: { outputs: { eql: 'true', rust_crates: 'true' } }, // The release workflows' gates, none of which is about how the run was // triggered. Held open, or "skips because there is nothing to publish" would // be indistinguishable from "skips on a dispatch". @@ -447,7 +447,7 @@ describe('the fork-PR guard keeps its meaning', () => { expect( verdicts, - ` if: ${condition}\nEvaluated with every OTHER gate held open (needs.changes.outputs.relevant = 'true'), so a false here is the fork guard's doing and nothing else.`, + ` if: ${condition}\nEvaluated with every OTHER gate held open (needs.selector.outputs.* = 'true'), so a false here is the fork guard's doing and nothing else.`, ).toEqual(expected) }) } diff --git a/scripts/__tests__/workflow-paths-filter-parity.test.mjs b/scripts/__tests__/workflow-paths-filter-parity.test.mjs index 5e719237c..74ab3af62 100644 --- a/scripts/__tests__/workflow-paths-filter-parity.test.mjs +++ b/scripts/__tests__/workflow-paths-filter-parity.test.mjs @@ -159,9 +159,9 @@ const EXPECTED_ASYMMETRIES = new Map([ // `push` one, so the PR introducing a change never runs the suite — is // unreachable when `pull_request` has no filter at all. The push list is // not unchecked: `eql-workflow-filters.test.mjs` compares it against the - // `relevant:` filter in the `changes` job, which is the second copy of the + // union of the selector job's filters, which is the second copy of the // same list and the one this file cannot see. - 'pull_request is deliberately unfiltered (per-job relevance instead), so PRs run a superset of what push runs; the push list is compared against the `changes` job filter by eql-workflow-filters.test.mjs', + 'pull_request is deliberately unfiltered (per-job relevance instead), so PRs run a superset of what push runs; the push list is compared against the selector job filters by eql-workflow-filters.test.mjs', ], [ '.github/workflows/bench-eql.yml',