From 4939e38d06bde5b3bb3c267442fce6f36c07c8ee Mon Sep 17 00:00:00 2001 From: 0xisk <0xisk@proton.me> Date: Fri, 28 Aug 2026 11:31:14 +0200 Subject: [PATCH] refactor(release): extract package resolution into a tested script The package name to directory mapping was written out twice, once in each release workflow, so a new releasable package meant editing four case statements that could drift apart. It now lives in one map that both workflows read through `scripts/release/resolve-package.mjs`. The merge-path detection moves with it. Picking the released package from the changed `packages/*/package.json` paths is the part most worth a test: it has to refuse rather than guess when a merge bumps two packages or none. `yarn test:scripts` runs the suite. The repo-level scripts are not a workspace, so turbo's per-package `test` task does not reach them and checks.yml calls it as its own step. --- .github/workflows/checks.yml | 3 + .github/workflows/release-publish.yml | 41 +++--------- .github/workflows/release.yml | 15 +---- RELEASING.md | 7 ++ biome.json | 14 +++- package.json | 1 + scripts/release/packages.mjs | 55 +++++++++++++++ scripts/release/resolve-package.mjs | 59 ++++++++++++++++ scripts/release/test/packages.test.mjs | 93 ++++++++++++++++++++++++++ vitest.scripts.config.ts | 11 +++ 10 files changed, 255 insertions(+), 44 deletions(-) create mode 100644 scripts/release/packages.mjs create mode 100644 scripts/release/resolve-package.mjs create mode 100644 scripts/release/test/packages.test.mjs create mode 100644 vitest.scripts.config.ts diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index de71266..595ae7d 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -36,3 +36,6 @@ jobs: - name: Run tests run: yarn test + + - name: Run release script tests + run: yarn test:scripts diff --git a/.github/workflows/release-publish.yml b/.github/workflows/release-publish.yml index 6a3ff1f..c350ecf 100644 --- a/.github/workflows/release-publish.yml +++ b/.github/workflows/release-publish.yml @@ -59,39 +59,18 @@ jobs: INPUT_PACKAGE: ${{ inputs.package }} run: | if [[ "$EVENT" == "workflow_dispatch" ]]; then - PKG="$INPUT_PACKAGE" - case "$PKG" in - compact-builder) DIR="builder" ;; - compact-cli) DIR="cli" ;; - compact-simulator) DIR="simulator" ;; - *) - echo "::error::unknown package: $PKG" - exit 1 - ;; - esac + node scripts/release/resolve-package.mjs --package "$INPUT_PACKAGE" else - mapfile -t CHANGED < <(git diff --name-only "$MERGE_SHA^" "$MERGE_SHA" -- 'packages/*/package.json') - COUNT=${#CHANGED[@]} - if [[ "$COUNT" -ne 1 ]]; then - echo "::error::expected exactly one packages/*/package.json change, found $COUNT" - printf '%s\n' "${CHANGED[@]}" >&2 - exit 1 - fi - DIR=$(echo "${CHANGED[0]}" | awk -F/ '{print $2}') - case "$DIR" in - builder) PKG="compact-builder" ;; - cli) PKG="compact-cli" ;; - simulator) PKG="compact-simulator" ;; - *) - echo "::error::unknown package directory: $DIR" - exit 1 - ;; - esac + CHANGED=$(git diff --name-only "$MERGE_SHA^" "$MERGE_SHA" -- 'packages/*/package.json') + node scripts/release/resolve-package.mjs --changed-files "$CHANGED" fi - VERSION=$(node -p "require('./packages/$DIR/package.json').version") - echo "dir=$DIR" >> $GITHUB_OUTPUT - echo "name=$PKG" >> $GITHUB_OUTPUT - echo "version=$VERSION" >> $GITHUB_OUTPUT + + - name: Summarize + env: + EVENT: ${{ github.event_name }} + PKG: ${{ steps.pkg.outputs.name }} + VERSION: ${{ steps.pkg.outputs.version }} + run: | { echo "### Publishing" echo "- Package: $PKG" diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index b8f41ee..731229e 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -46,18 +46,9 @@ jobs: - name: Set package directory id: pkg - run: | - case "${{ inputs.package }}" in - "compact-builder") - echo "dir=builder" >> $GITHUB_OUTPUT - ;; - "compact-cli") - echo "dir=cli" >> $GITHUB_OUTPUT - ;; - "compact-simulator") - echo "dir=simulator" >> $GITHUB_OUTPUT - ;; - esac + env: + PACKAGE: ${{ inputs.package }} + run: node scripts/release/resolve-package.mjs --package "$PACKAGE" - name: Setup Environment uses: ./.github/actions/setup diff --git a/RELEASING.md b/RELEASING.md index c9635f6..5f19047 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -18,6 +18,13 @@ - Publish the package to npm. 6. Once published, go to "Releases" and create a GitHub release using the generated tag. +## Adding a package to the rotation + +Both workflows resolve package names and directories through +`scripts/release/packages.mjs`. Add the workspace to the `RELEASABLE` map +there, then add its name to the `package` choice list in `release.yml` and +`release-publish.yml`. `yarn test:scripts` covers the resolver. + ## First-release order There's a one-step dependency chain across the three published packages: diff --git a/biome.json b/biome.json index 8b8dffd..b8c3c52 100644 --- a/biome.json +++ b/biome.json @@ -109,5 +109,17 @@ "semicolons": "always", "indentStyle": "space" } - } + }, + "overrides": [ + { + "includes": ["scripts/**"], + "linter": { + "rules": { + "suspicious": { + "noUndeclaredEnvVars": "off" + } + } + } + } + ] } diff --git a/package.json b/package.json index 77fe325..c5dba28 100644 --- a/package.json +++ b/package.json @@ -9,6 +9,7 @@ "scripts": { "build": "turbo run build --log-prefix=none", "test": "turbo run test --log-prefix=none", + "test:scripts": "vitest run --config vitest.scripts.config.ts", "lint": "biome check .", "lint:fix": "biome check . --write", "lint:ci": "biome ci . --no-errors-on-unmatched", diff --git a/scripts/release/packages.mjs b/scripts/release/packages.mjs new file mode 100644 index 0000000..0641508 --- /dev/null +++ b/scripts/release/packages.mjs @@ -0,0 +1,55 @@ +/** + * Releasable workspaces, keyed by their directory under `packages/`. + * + * Both release workflows resolve names and directories through here, so + * adding a package to the release rotation is a one-line change. + */ +export const RELEASABLE = { + builder: 'compact-builder', + cli: 'compact-cli', + simulator: 'compact-simulator', +}; + +/** Workflow input name (`compact-cli`) to workspace directory (`cli`). */ +export function dirForPackage(name) { + const dir = Object.keys(RELEASABLE).find((key) => RELEASABLE[key] === name); + if (!dir) { + throw new Error(`unknown package: ${name}`); + } + return dir; +} + +/** Workspace directory (`cli`) to workflow input name (`compact-cli`). */ +export function packageForDir(dir) { + const name = RELEASABLE[dir]; + if (!name) { + throw new Error(`unknown package directory: ${dir}`); + } + return name; +} + +/** + * Resolve the released workspace from the `packages/*\/package.json` paths a + * merge touched. + * + * A release commit bumps exactly one package. Two or zero means the merge was + * not the one the publish workflow expected, and guessing which to ship would + * publish the wrong thing. + */ +export function dirFromChangedFiles(paths) { + const bumps = paths + .map((path) => path.trim()) + .filter((path) => /^packages\/[^/]+\/package\.json$/.test(path)); + + if (bumps.length !== 1) { + throw new Error( + `expected exactly one packages/*/package.json change, found ${bumps.length}${ + bumps.length ? `: ${bumps.join(', ')}` : '' + }`, + ); + } + + const dir = bumps[0].split('/')[1]; + packageForDir(dir); + return dir; +} diff --git a/scripts/release/resolve-package.mjs b/scripts/release/resolve-package.mjs new file mode 100644 index 0000000..2d35798 --- /dev/null +++ b/scripts/release/resolve-package.mjs @@ -0,0 +1,59 @@ +#!/usr/bin/env node +/** + * Resolve the package a release workflow is acting on, and append `dir`, + * `name`, and `version` to `$GITHUB_OUTPUT`. + * + * node scripts/release/resolve-package.mjs --package compact-cli + * node scripts/release/resolve-package.mjs --changed-files "$CHANGED" + * + * `--changed-files` takes the newline-separated output of `git diff + * --name-only`. Without `$GITHUB_OUTPUT` the result goes to stdout instead, + * so the script is runnable by hand. + */ +import { appendFileSync, readFileSync } from 'node:fs'; +import { + dirForPackage, + dirFromChangedFiles, + packageForDir, +} from './packages.mjs'; + +function parseArgs(argv) { + const args = {}; + for (let i = 0; i < argv.length; i += 2) { + args[argv[i]] = argv[i + 1]; + } + return args; +} + +function main(argv) { + const args = parseArgs(argv); + if ( + args['--package'] === undefined && + args['--changed-files'] === undefined + ) { + throw new Error('pass either --package or --changed-files'); + } + + const dir = + args['--package'] !== undefined + ? dirForPackage(args['--package']) + : dirFromChangedFiles(args['--changed-files'].split('\n')); + + const { version } = JSON.parse( + readFileSync(`packages/${dir}/package.json`, 'utf8'), + ); + const output = `dir=${dir}\nname=${packageForDir(dir)}\nversion=${version}\n`; + + if (process.env.GITHUB_OUTPUT) { + appendFileSync(process.env.GITHUB_OUTPUT, output); + } + process.stdout.write(output); +} + +try { + main(process.argv.slice(2)); +} catch (error) { + // Annotations are read from stdout, so this must not go to stderr. + process.stdout.write(`::error::${error.message}\n`); + process.exit(1); +} diff --git a/scripts/release/test/packages.test.mjs b/scripts/release/test/packages.test.mjs new file mode 100644 index 0000000..a7218c7 --- /dev/null +++ b/scripts/release/test/packages.test.mjs @@ -0,0 +1,93 @@ +import { describe, expect, it } from 'vitest'; +import { + dirForPackage, + dirFromChangedFiles, + packageForDir, + RELEASABLE, +} from '../packages.mjs'; + +describe('dirForPackage', () => { + it.each(Object.entries(RELEASABLE))( + 'maps %s to its directory', + (dir, name) => { + expect(dirForPackage(name)).toBe(dir); + }, + ); + + it('rejects a package outside the release rotation', () => { + expect(() => dirForPackage('compact-deployer')).toThrow(/unknown package/); + }); + + it('rejects a scoped name', () => { + expect(() => dirForPackage('@openzeppelin/compact-cli')).toThrow( + /unknown package/, + ); + }); +}); + +describe('packageForDir', () => { + it.each(Object.entries(RELEASABLE))( + 'maps %s to its package name', + (dir, name) => { + expect(packageForDir(dir)).toBe(name); + }, + ); + + it('rejects a directory outside the release rotation', () => { + expect(() => packageForDir('deployer')).toThrow( + /unknown package directory/, + ); + }); +}); + +describe('dirFromChangedFiles', () => { + it('resolves the single bumped package', () => { + expect(dirFromChangedFiles(['packages/simulator/package.json'])).toBe( + 'simulator', + ); + }); + + it('ignores files that are not a workspace manifest', () => { + expect( + dirFromChangedFiles([ + 'packages/cli/src/index.ts', + 'packages/cli/package.json', + 'package.json', + 'RELEASING.md', + ]), + ).toBe('cli'); + }); + + it('tolerates trailing whitespace from the git diff', () => { + expect( + dirFromChangedFiles(['packages/builder/package.json ', '', ' ']), + ).toBe('builder'); + }); + + it('refuses to guess when two packages were bumped', () => { + expect(() => + dirFromChangedFiles([ + 'packages/cli/package.json', + 'packages/builder/package.json', + ]), + ).toThrow( + /found 2: packages\/cli\/package\.json, packages\/builder\/package\.json/, + ); + }); + + it('refuses when no package was bumped', () => { + expect(() => dirFromChangedFiles(['README.md'])).toThrow(/found 0/); + }); + + it('does not treat a nested manifest as a workspace bump', () => { + expect(() => + dirFromChangedFiles(['packages/cli/node_modules/x/package.json']), + ).toThrow(/found 0/); + }); + + it('rejects a bump in a directory outside the release rotation', () => { + expect(() => + dirFromChangedFiles(['packages/deployer/package.json']), + ).toThrow(/unknown package directory: deployer/); + }); +}); diff --git a/vitest.scripts.config.ts b/vitest.scripts.config.ts new file mode 100644 index 0000000..9b75361 --- /dev/null +++ b/vitest.scripts.config.ts @@ -0,0 +1,11 @@ +import { defineConfig } from 'vitest/config'; + +// The repo-level release helpers are not a workspace, so turbo's per-package +// `test` task does not reach them. `yarn test:scripts` runs them. +export default defineConfig({ + test: { + environment: 'node', + include: ['scripts/**/test/**/*.test.mjs'], + reporters: 'verbose', + }, +});