From b90905273a52de0cb0014cc27324bb81293fba48 Mon Sep 17 00:00:00 2001 From: Alassane Ndoye Date: Fri, 17 Jul 2026 14:52:11 +0000 Subject: [PATCH 1/4] Add changeset check workflow and docs Add a GitHub Actions workflow that enforces a changeset on PRs (.github/workflows/changeset.yaml). The check fails PRs that do not include a .changeset/*.md file unless a maintainer applied the `no changeset required` label (exemptions are recorded in the step summary). Update the PR template and README to document the label-based exemption and provide guidance on running `pnpm changeset` when required. --- .github/pull_request_template.md | 4 +- .github/workflows/changeset.yaml | 75 ++++++++++++++++++++++++++++++++ README.md | 11 +++-- 3 files changed, 85 insertions(+), 5 deletions(-) create mode 100644 .github/workflows/changeset.yaml diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index dbb790808..1e4542595 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -29,8 +29,8 @@ Before merging, the reviewer should check the following items: - [ ] If this PR includes breaking changes, do we need to update any jobs in production? Is it safe to release? - [ ] Are there any unit tests? -- [ ] Is there a changeset associated with this PR? Should there be? Note that - dev only changes don't need a changeset. +- [ ] Is there a changeset associated with this PR? If this PR does not affect a + release, has a maintainer applied the `no changeset required` label? - [ ] If there is a changeset, was `pnpm run version` used to bump versions (not `pnpm changeset version` directly)? This ensures changelog dates are stamped correctly. - [ ] Have you ticked a box under AI Usage? diff --git a/.github/workflows/changeset.yaml b/.github/workflows/changeset.yaml new file mode 100644 index 000000000..0363d083b --- /dev/null +++ b/.github/workflows/changeset.yaml @@ -0,0 +1,75 @@ +name: Changeset + +on: + pull_request: + types: [opened, synchronize, reopened, labeled, unlabeled] + +permissions: + contents: read + +jobs: + changeset: + name: Changeset required + runs-on: ubuntu-latest + + steps: + - name: Record changeset exemption + if: contains(github.event.pull_request.labels.*.name, 'no changeset required') + run: | + echo "Changeset check skipped by the 'no changeset required' label." + { + echo "## Changeset check skipped" + echo + echo "A maintainer applied the \`no changeset required\` label." + } >> "$GITHUB_STEP_SUMMARY" + + - name: Checkout repository + if: "${{ !contains(github.event.pull_request.labels.*.name, 'no changeset required') }}" + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Check for a changeset + if: "${{ !contains(github.event.pull_request.labels.*.name, 'no changeset required') }}" + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} + run: | + changeset_files=() + + while IFS= read -r file; do + if [[ "$(basename "$file")" != "README.md" ]]; then + changeset_files+=("$file") + fi + done < <( + git diff --name-only --diff-filter=AM \ + "$BASE_SHA"...HEAD -- '.changeset/*.md' + ) + + if (( ${#changeset_files[@]} == 0 )); then + docs_url="https://github.com/OpenFn/adaptors/blob/main/README.md#changesets" + message="This PR must include a changeset. Run 'pnpm changeset', commit the generated file, and push it. If the change does not need a release, ask a maintainer to apply the 'no changeset required' label. See ${docs_url}." + + echo "::error title=Changeset required::${message}" + { + echo "## Changeset required" + echo + echo "This PR does not add or modify a changeset file." + echo + echo "1. Run \`pnpm changeset\`." + echo "2. Commit the generated \`.changeset/*.md\` file." + echo "3. Push the commit to update this check." + echo + echo "If this PR does not need a release, ask a maintainer to apply the \`no changeset required\` label." + echo + echo "See the [Changesets documentation](${docs_url}) for details." + } >> "$GITHUB_STEP_SUMMARY" + + exit 1 + fi + + printf 'Changeset found: %s\n' "${changeset_files[@]}" + { + echo "## Changeset found" + echo + printf -- '- `%s`\n' "${changeset_files[@]}" + } >> "$GITHUB_STEP_SUMMARY" diff --git a/README.md b/README.md index e5ad2aac4..1cb37dbfc 100644 --- a/README.md +++ b/README.md @@ -222,8 +222,10 @@ in the wiki. ## Changesets -Any submitted PRs should have an accompanying -[changeset](https://github.com/changesets/changesets). +Every submitted PR must have an accompanying +[changeset](https://github.com/changesets/changesets), unless a maintainer +applies the `no changeset required` label. The **Changeset required** GitHub +check enforces this policy. - Create a new changeset with `pnpm changeset`. This will prompt you for the changes: @@ -233,9 +235,12 @@ pnpm changeset ``` > Follow the prompts to describe your changes. This will create a new changeset -> file in the `.changesets` folder. +> file in the `.changeset` folder. - Commit the changeset to the repo when you're ready. +- If the PR does not affect a release, such as a development-only change, ask a + maintainer to apply the `no changeset required` label. Adding or removing the + label reruns the check. ## Versioning From 8ceaffcd63101d6907090bfef493b71d50d0f34f Mon Sep 17 00:00:00 2001 From: Alassane Ndoye Date: Tue, 21 Jul 2026 17:36:27 +0000 Subject: [PATCH 2/4] Update checkout action to v7 and disable creds Bumps actions/checkout from v4 to v7 in the changeset workflow and sets persist-credentials: false for the Checkout repository step. This reduces exposure of the default GITHUB_TOKEN to subsequent steps (least-privilege), while keeping fetch-depth: 0. File changed: .github/workflows/changeset.yaml. Note: if any downstream steps require pushing or authenticated git operations, they will need an explicit token or to re-enable credential persistence. --- .github/workflows/changeset.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/changeset.yaml b/.github/workflows/changeset.yaml index 0363d083b..6eeff1657 100644 --- a/.github/workflows/changeset.yaml +++ b/.github/workflows/changeset.yaml @@ -25,9 +25,10 @@ jobs: - name: Checkout repository if: "${{ !contains(github.event.pull_request.labels.*.name, 'no changeset required') }}" - uses: actions/checkout@v4 + uses: actions/checkout@v7 with: fetch-depth: 0 + persist-credentials: false - name: Check for a changeset if: "${{ !contains(github.event.pull_request.labels.*.name, 'no changeset required') }}" From 576f502054366cc9efa2f02f9da938eb4afbdfff Mon Sep 17 00:00:00 2001 From: Alassane Ndoye Date: Fri, 24 Jul 2026 14:47:53 +0000 Subject: [PATCH 3/4] Enforce per-adaptor changesets --- .github/pull_request_template.md | 5 +- .github/workflows/changeset.yaml | 80 ++--- README.md | 20 +- package.json | 5 +- pnpm-lock.yaml | 3 + scripts/check-changesets.mjs | 506 ++++++++++++++++++++++++++++++ scripts/check-changesets.test.mjs | 284 +++++++++++++++++ 7 files changed, 844 insertions(+), 59 deletions(-) create mode 100644 scripts/check-changesets.mjs create mode 100644 scripts/check-changesets.test.mjs diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 1e4542595..7113586cf 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -29,8 +29,9 @@ Before merging, the reviewer should check the following items: - [ ] If this PR includes breaking changes, do we need to update any jobs in production? Is it safe to release? - [ ] Are there any unit tests? -- [ ] Is there a changeset associated with this PR? If this PR does not affect a - release, has a maintainer applied the `no changeset required` label? +- [ ] Does every changed adaptor have a corresponding changeset? If an adaptor + change does not affect a release, has a maintainer applied the + `no changeset required` label? Repository-only changes need neither. - [ ] If there is a changeset, was `pnpm run version` used to bump versions (not `pnpm changeset version` directly)? This ensures changelog dates are stamped correctly. - [ ] Have you ticked a box under AI Usage? diff --git a/.github/workflows/changeset.yaml b/.github/workflows/changeset.yaml index 6eeff1657..ade9696fa 100644 --- a/.github/workflows/changeset.yaml +++ b/.github/workflows/changeset.yaml @@ -13,64 +13,46 @@ jobs: runs-on: ubuntu-latest steps: - - name: Record changeset exemption - if: contains(github.event.pull_request.labels.*.name, 'no changeset required') - run: | - echo "Changeset check skipped by the 'no changeset required' label." - { - echo "## Changeset check skipped" - echo - echo "A maintainer applied the \`no changeset required\` label." - } >> "$GITHUB_STEP_SUMMARY" - - name: Checkout repository - if: "${{ !contains(github.event.pull_request.labels.*.name, 'no changeset required') }}" uses: actions/checkout@v7 with: fetch-depth: 0 persist-credentials: false - - name: Check for a changeset - if: "${{ !contains(github.event.pull_request.labels.*.name, 'no changeset required') }}" + - name: Check whether a changeset is required + id: requirements env: BASE_SHA: ${{ github.event.pull_request.base.sha }} - run: | - changeset_files=() - - while IFS= read -r file; do - if [[ "$(basename "$file")" != "README.md" ]]; then - changeset_files+=("$file") - fi - done < <( - git diff --name-only --diff-filter=AM \ - "$BASE_SHA"...HEAD -- '.changeset/*.md' - ) + run: node scripts/check-changesets.mjs detect --base "$BASE_SHA" - if (( ${#changeset_files[@]} == 0 )); then - docs_url="https://github.com/OpenFn/adaptors/blob/main/README.md#changesets" - message="This PR must include a changeset. Run 'pnpm changeset', commit the generated file, and push it. If the change does not need a release, ask a maintainer to apply the 'no changeset required' label. See ${docs_url}." - - echo "::error title=Changeset required::${message}" - { - echo "## Changeset required" - echo - echo "This PR does not add or modify a changeset file." - echo - echo "1. Run \`pnpm changeset\`." - echo "2. Commit the generated \`.changeset/*.md\` file." - echo "3. Push the commit to update this check." - echo - echo "If this PR does not need a release, ask a maintainer to apply the \`no changeset required\` label." - echo - echo "See the [Changesets documentation](${docs_url}) for details." - } >> "$GITHUB_STEP_SUMMARY" - - exit 1 - fi - - printf 'Changeset found: %s\n' "${changeset_files[@]}" + - name: Record changeset exemption + if: >- + steps.requirements.outputs.required == 'true' && + contains(github.event.pull_request.labels.*.name, 'no changeset required') + run: | + echo "Changeset check skipped by the 'no changeset required' label." { - echo "## Changeset found" + echo "## Changeset check skipped" echo - printf -- '- `%s`\n' "${changeset_files[@]}" + echo "A maintainer applied the \`no changeset required\` label." } >> "$GITHUB_STEP_SUMMARY" + + - name: Setup pnpm + if: >- + steps.requirements.outputs.required == 'true' && + !contains(github.event.pull_request.labels.*.name, 'no changeset required') + uses: pnpm/action-setup@v4 + + - name: Install changeset checker dependencies + if: >- + steps.requirements.outputs.required == 'true' && + !contains(github.event.pull_request.labels.*.name, 'no changeset required') + run: pnpm install --filter adaptors --frozen-lockfile + + - name: Validate changeset coverage + if: >- + steps.requirements.outputs.required == 'true' && + !contains(github.event.pull_request.labels.*.name, 'no changeset required') + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} + run: node scripts/check-changesets.mjs validate --base "$BASE_SHA" diff --git a/README.md b/README.md index 1cb37dbfc..67a1ad3e4 100644 --- a/README.md +++ b/README.md @@ -222,10 +222,14 @@ in the wiki. ## Changesets -Every submitted PR must have an accompanying -[changeset](https://github.com/changesets/changesets), unless a maintainer -applies the `no changeset required` label. The **Changeset required** GitHub -check enforces this policy. +Every submitted PR that changes one or more adaptors under `packages/` must have +an accompanying [changeset](https://github.com/changesets/changesets) for each +changed adaptor. A single changeset may target multiple adaptors. Repository +tooling and top-level documentation changes do not require a changeset. + +The **Changeset required** GitHub check enforces this policy. It detects changed +adaptors and verifies that every corresponding package appears in an added or +modified changeset. - Create a new changeset with `pnpm changeset`. This will prompt you for the changes: @@ -238,9 +242,11 @@ pnpm changeset > file in the `.changeset` folder. - Commit the changeset to the repo when you're ready. -- If the PR does not affect a release, such as a development-only change, ask a - maintainer to apply the `no changeset required` label. Adding or removing the - label reruns the check. +- Run `pnpm changeset:check` to verify changeset coverage before pushing. +- Test, documentation, and development-only changes inside an adaptor directory + still count as adaptor changes. If such a change does not need a release, ask + a maintainer to apply the exact `no changeset required` label. Adding or + removing the label reruns the check. ## Versioning diff --git a/package.json b/package.json index b6c7c06f9..74a65c0f5 100644 --- a/package.json +++ b/package.json @@ -10,6 +10,7 @@ "build:tools": "pnpm --filter \"./tools/**\" build", "build": "pnpm build:tools && pnpm build:adaptors", "changelog": "pnpm -C tools/changelog update-latest-versions", + "changeset:check": "node scripts/check-changesets.mjs validate", "docs:build": "pnpm exec scripts/build-docs.sh", "docs:publish": "pnpm exec scripts/publish-docs.sh", "docs:watch": "chokidar packages/*/docs/*.md -c 'pnpm docs:build'", @@ -21,7 +22,8 @@ "setup": "pnpm --filter \"./tools/**\" install", "slack:notify": "cd tools/slack && pnpm notify", "test:imports": "cd tools/import-tests && pnpm test", - "test": "pnpm lint && pnpm --filter \"./packages/**\" test && pnpm test:imports", + "test": "pnpm lint && pnpm --filter \"./packages/**\" test && pnpm test:imports && pnpm test:changesets", + "test:changesets": "node --test scripts/check-changesets.test.mjs", "test:git": "pnpm exec scripts/status-check.sh", "validate:schemas": "node scripts/validate-configuration-schemas.mjs", "version": "pnpm changeset version && pnpm run changelog" @@ -30,6 +32,7 @@ "license": "ISC", "devDependencies": { "@changesets/cli": "^2.29.8", + "@changesets/parse": "^0.4.2", "@openfn/adaptor-apis": "workspace:*", "@openfn/buildtools": "workspace:^1.0.2", "@openfn/generate-fhir": "workspace:^", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 7cf1be25c..c89895846 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -20,6 +20,9 @@ importers: '@changesets/cli': specifier: ^2.29.8 version: 2.29.8(@types/node@18.17.5) + '@changesets/parse': + specifier: ^0.4.2 + version: 0.4.2 '@openfn/adaptor-apis': specifier: workspace:* version: link:tools/adaptor-apis diff --git a/scripts/check-changesets.mjs b/scripts/check-changesets.mjs new file mode 100644 index 000000000..17f4889d8 --- /dev/null +++ b/scripts/check-changesets.mjs @@ -0,0 +1,506 @@ +#!/usr/bin/env node +import { execFileSync } from 'node:child_process'; +import fs from 'node:fs/promises'; +import path from 'node:path'; +import process from 'node:process'; +import { fileURLToPath, pathToFileURL } from 'node:url'; + +const rootDir = path.resolve( + path.dirname(fileURLToPath(import.meta.url)), + '..' +); +const changesetsDocsUrl = + 'https://github.com/OpenFn/adaptors/blob/main/README.md#changesets'; + +const runGit = (args, { cwd = rootDir, allowFailure = false } = {}) => { + try { + return execFileSync('git', args, { + cwd, + encoding: 'utf8', + stdio: ['ignore', 'pipe', allowFailure ? 'ignore' : 'pipe'], + }); + } catch (error) { + if (allowFailure) return null; + + const detail = error.stderr?.trim() || error.message; + throw new Error(`git ${args.join(' ')} failed: ${detail}`); + } +}; + +const normalizePath = file => file.split(path.sep).join('/'); + +export const parseNameStatus = output => { + const tokens = output.split('\0'); + if (tokens.at(-1) === '') tokens.pop(); + + const entries = []; + for (let index = 0; index < tokens.length; ) { + const status = tokens[index++]; + const pathCount = ['C', 'R'].includes(status[0]) ? 2 : 1; + const paths = tokens.slice(index, index + pathCount); + + if (paths.length !== pathCount) { + throw new Error(`Could not parse git diff entry with status "${status}".`); + } + + entries.push({ status, paths: paths.map(normalizePath) }); + index += pathCount; + } + + return entries; +}; + +export const findChangedAdaptorDirectories = entries => { + const adaptors = new Map(); + + for (const entry of entries) { + const changedPaths = + entry.status[0] === 'C' ? [entry.paths.at(-1)] : entry.paths; + + for (const file of changedPaths) { + const match = /^packages\/([^/]+)(?:\/|$)/.exec(file); + if (!match) continue; + + const directory = match[1]; + const paths = adaptors.get(directory) ?? new Set(); + paths.add(file); + adaptors.set(directory, paths); + } + } + + return adaptors; +}; + +export const findChangesetFiles = entries => { + const files = new Set(); + + for (const entry of entries) { + if (!['A', 'M'].includes(entry.status[0])) continue; + + const file = entry.paths.at(-1); + if ( + /^\.changeset\/[^/]+\.md$/.test(file) && + path.posix.basename(file) !== 'README.md' + ) { + files.add(file); + } + } + + return [...files].sort(); +}; + +const readJson = (contents, source) => { + try { + return JSON.parse(contents); + } catch (error) { + throw new Error(`Invalid JSON in ${source}: ${error.message}`); + } +}; + +const readPackageAtBase = (directory, mergeBase, cwd) => { + const packagePath = `packages/${directory}/package.json`; + const contents = runGit(['show', `${mergeBase}:${packagePath}`], { + cwd, + allowFailure: true, + }); + + if (contents === null) { + throw new Error( + `Could not find ${packagePath} in the working tree or at ${mergeBase}.` + ); + } + + return readJson(contents, `${mergeBase}:${packagePath}`); +}; + +const readPackage = async (directory, mergeBase, cwd) => { + const packagePath = path.join(cwd, 'packages', directory, 'package.json'); + + try { + return readJson( + await fs.readFile(packagePath, 'utf8'), + normalizePath(path.relative(cwd, packagePath)) + ); + } catch (error) { + if (error.code !== 'ENOENT') throw error; + return readPackageAtBase(directory, mergeBase, cwd); + } +}; + +export const resolveChangedPackages = async ( + adaptorDirectories, + mergeBase, + { cwd = rootDir } = {} +) => { + const packagesByName = new Map(); + + for (const [directory, changedPaths] of adaptorDirectories) { + const packageJson = await readPackage(directory, mergeBase, cwd); + + if ( + typeof packageJson.name !== 'string' || + !packageJson.name.startsWith('@openfn/language-') + ) { + throw new Error( + `packages/${directory}/package.json does not define an OpenFn adaptor package name.` + ); + } + + const existing = packagesByName.get(packageJson.name) ?? { + name: packageJson.name, + directories: new Set(), + paths: new Set(), + }; + existing.directories.add(directory); + for (const file of changedPaths) existing.paths.add(file); + packagesByName.set(packageJson.name, existing); + } + + return [...packagesByName.values()] + .map(pkg => ({ + name: pkg.name, + directories: [...pkg.directories].sort(), + paths: [...pkg.paths].sort(), + })) + .sort((a, b) => a.name.localeCompare(b.name)); +}; + +const getDefaultBaseRef = async cwd => { + let baseBranch = 'main'; + + try { + const config = readJson( + await fs.readFile(path.join(cwd, '.changeset', 'config.json'), 'utf8'), + '.changeset/config.json' + ); + if (typeof config.baseBranch === 'string') { + baseBranch = config.baseBranch; + } + } catch (error) { + if (error.code !== 'ENOENT') throw error; + } + + for (const candidate of [`origin/${baseBranch}`, baseBranch]) { + const commit = runGit( + ['rev-parse', '--verify', '--quiet', `${candidate}^{commit}`], + { cwd, allowFailure: true } + ); + if (commit !== null) return candidate; + } + + throw new Error( + `Could not resolve origin/${baseBranch} or ${baseBranch}. Pass --base .` + ); +}; + +const getUntrackedEntries = cwd => { + const output = runGit( + ['ls-files', '--others', '--exclude-standard', '-z'], + { cwd } + ); + + return output + .split('\0') + .filter(Boolean) + .map(file => ({ status: 'A', paths: [normalizePath(file)] })); +}; + +export const inspectRepository = async ({ + cwd = rootDir, + baseRef, +} = {}) => { + const resolvedBase = baseRef ?? (await getDefaultBaseRef(cwd)); + const mergeBase = runGit(['merge-base', resolvedBase, 'HEAD'], { + cwd, + }).trim(); + + if (!mergeBase) { + throw new Error(`Could not find a merge base for ${resolvedBase} and HEAD.`); + } + + const entries = [ + ...parseNameStatus( + runGit( + [ + 'diff', + '--name-status', + '-z', + '--find-renames', + '--find-copies', + mergeBase, + '--', + ], + { cwd } + ) + ), + ...getUntrackedEntries(cwd), + ]; + const adaptorDirectories = findChangedAdaptorDirectories(entries); + const changedPackages = await resolveChangedPackages( + adaptorDirectories, + mergeBase, + { cwd } + ); + + return { + baseRef: resolvedBase, + mergeBase, + entries, + changedPackages, + changesetFiles: findChangesetFiles(entries), + }; +}; + +export const parseChangesetTargets = async ( + files, + { cwd = rootDir, parser } = {} +) => { + const parse = parser ?? (await import('@changesets/parse')).default; + const targets = new Set(); + const parsedFiles = []; + const errors = []; + + for (const file of files) { + try { + const changeset = parse(await fs.readFile(path.join(cwd, file), 'utf8')); + + if (!changeset.releases.length) { + throw new Error('the changeset does not target any packages'); + } + + const releases = changeset.releases.map(release => release.name).sort(); + for (const name of releases) targets.add(name); + parsedFiles.push({ file, releases }); + } catch (error) { + errors.push({ file, message: error.message }); + } + } + + return { targets, parsedFiles, errors }; +}; + +export const findMissingPackages = (changedPackages, targets) => + changedPackages.filter(pkg => !targets.has(pkg.name)); + +const appendFileFromEnvironment = async (variable, contents, environment) => { + const file = environment[variable]; + if (file) await fs.appendFile(file, `${contents.trimEnd()}\n`); +}; + +const appendSummary = (contents, environment) => + appendFileFromEnvironment('GITHUB_STEP_SUMMARY', contents, environment); + +const writeOutputs = (values, environment) => + appendFileFromEnvironment( + 'GITHUB_OUTPUT', + Object.entries(values) + .map(([key, value]) => `${key}=${value}`) + .join('\n'), + environment + ); + +const formatPackageList = packages => + packages.map(pkg => `- \`${pkg.name}\``).join('\n'); + +const printNoChangesetRequired = async environment => { + console.log('No adaptor packages changed; a changeset is not required.'); + await appendSummary( + `## Changeset not required + +No files under \`packages/*\` changed in this pull request.`, + environment + ); +}; + +const runDetect = async (inspection, environment) => { + const required = inspection.changedPackages.length > 0; + + await writeOutputs( + { + required: String(required), + 'changed-packages': JSON.stringify( + inspection.changedPackages.map(pkg => pkg.name) + ), + }, + environment + ); + + if (!required) { + await printNoChangesetRequired(environment); + return; + } + + console.log('Changeset coverage is required for:'); + for (const pkg of inspection.changedPackages) console.log(`- ${pkg.name}`); + + await appendSummary( + `## Changeset required + +The following adaptor packages changed: + +${formatPackageList(inspection.changedPackages)}`, + environment + ); +}; + +const formatFailure = (missingPackages, errors) => { + const lines = ['Changeset coverage check failed.', '']; + + if (missingPackages.length) { + lines.push('Missing changesets for:'); + for (const pkg of missingPackages) { + lines.push(`- ${pkg.name}`); + for (const file of pkg.paths) lines.push(` - ${file}`); + } + lines.push(''); + } + + if (errors.length) { + lines.push('Invalid changeset files:'); + for (const error of errors) { + lines.push(`- ${error.file}: ${error.message}`); + } + lines.push(''); + } + + lines.push("Run 'pnpm changeset' and commit the generated file."); + lines.push( + "If no release is needed, ask a maintainer to apply the exact 'no changeset required' label." + ); + lines.push(`See ${changesetsDocsUrl}.`); + + return lines.join('\n'); +}; + +const formatFailureSummary = (missingPackages, errors) => { + const lines = ['## Changeset coverage failed', '']; + + if (missingPackages.length) { + lines.push('### Missing adaptor packages', ''); + for (const pkg of missingPackages) { + lines.push(`- \`${pkg.name}\``); + for (const file of pkg.paths) lines.push(` - \`${file}\``); + } + lines.push(''); + } + + if (errors.length) { + lines.push('### Invalid changeset files', ''); + for (const error of errors) { + lines.push(`- \`${error.file}\`: ${error.message}`); + } + lines.push(''); + } + + lines.push( + 'Run `pnpm changeset`, commit the generated file, and push it.', + '', + 'If the change does not need a release, ask a maintainer to apply the exact `no changeset required` label.', + '', + `See the [Changesets documentation](${changesetsDocsUrl}) for details.` + ); + + return lines.join('\n'); +}; + +const escapeWorkflowCommand = value => + value.replace(/%/g, '%25').replace(/\r/g, '%0D').replace(/\n/g, '%0A'); + +const runValidate = async (inspection, environment) => { + if (!inspection.changedPackages.length) { + await printNoChangesetRequired(environment); + return true; + } + + const parsed = await parseChangesetTargets(inspection.changesetFiles); + const missingPackages = findMissingPackages( + inspection.changedPackages, + parsed.targets + ); + + if (missingPackages.length || parsed.errors.length) { + const failure = formatFailure(missingPackages, parsed.errors); + console.error(failure); + + if (environment.GITHUB_ACTIONS === 'true') { + console.error( + `::error title=Changeset required::${escapeWorkflowCommand(failure)}` + ); + } + + await appendSummary( + formatFailureSummary(missingPackages, parsed.errors), + environment + ); + return false; + } + + console.log('Changesets cover every changed adaptor package.'); + for (const pkg of inspection.changedPackages) console.log(`- ${pkg.name}`); + + const changesetList = parsed.parsedFiles + .map(item => `- \`${item.file}\``) + .join('\n'); + await appendSummary( + `## Changeset coverage passed + +Covered adaptor packages: + +${formatPackageList(inspection.changedPackages)} + +Changeset files: + +${changesetList}`, + environment + ); + return true; +}; + +const parseArguments = argv => { + const [mode, ...args] = argv; + if (!['detect', 'validate'].includes(mode)) { + throw new Error( + 'Usage: node scripts/check-changesets.mjs [--base ]' + ); + } + + let baseRef; + for (let index = 0; index < args.length; index += 1) { + if (args[index] !== '--base' || !args[index + 1]) { + throw new Error(`Unknown or incomplete argument: ${args[index]}`); + } + baseRef = args[++index]; + } + + return { mode, baseRef }; +}; + +export const main = async ( + argv = process.argv.slice(2), + environment = process.env +) => { + const { mode, baseRef } = parseArguments(argv); + const inspection = await inspectRepository({ baseRef }); + + if (mode === 'detect') { + await runDetect(inspection, environment); + return true; + } + + return runValidate(inspection, environment); +}; + +const isMain = + process.argv[1] && + import.meta.url === pathToFileURL(path.resolve(process.argv[1])).href; + +if (isMain) { + main() + .then(success => { + if (!success) process.exitCode = 1; + }) + .catch(error => { + console.error(error.message); + process.exitCode = 1; + }); +} diff --git a/scripts/check-changesets.test.mjs b/scripts/check-changesets.test.mjs new file mode 100644 index 000000000..a27139ab3 --- /dev/null +++ b/scripts/check-changesets.test.mjs @@ -0,0 +1,284 @@ +import assert from 'node:assert/strict'; +import { execFileSync } from 'node:child_process'; +import fs from 'node:fs/promises'; +import os from 'node:os'; +import path from 'node:path'; +import test from 'node:test'; + +import { + findChangedAdaptorDirectories, + findChangesetFiles, + findMissingPackages, + inspectRepository, + parseChangesetTargets, + parseNameStatus, +} from './check-changesets.mjs'; + +const runGit = (cwd, ...args) => + execFileSync('git', args, { cwd, encoding: 'utf8' }); + +const write = async (cwd, file, contents) => { + const target = path.join(cwd, file); + await fs.mkdir(path.dirname(target), { recursive: true }); + await fs.writeFile(target, contents); +}; + +const packageJson = name => JSON.stringify({ name }, null, 2); + +const createRepository = async () => { + const cwd = await fs.mkdtemp(path.join(os.tmpdir(), 'changeset-check-')); + runGit(cwd, 'init', '--initial-branch=main'); + runGit(cwd, 'config', 'user.name', 'Changeset Test'); + runGit(cwd, 'config', 'user.email', 'changeset@example.com'); + + await write( + cwd, + '.changeset/config.json', + JSON.stringify({ baseBranch: 'main' }) + ); + await write( + cwd, + 'packages/common/package.json', + packageJson('@openfn/language-common') + ); + await write( + cwd, + 'packages/gmail/package.json', + packageJson('@openfn/language-gmail') + ); + await write(cwd, 'README.md', '# Test repository\n'); + + runGit(cwd, 'add', '.'); + runGit(cwd, 'commit', '-m', 'Initial commit'); + runGit(cwd, 'switch', '-c', 'feature'); + + return cwd; +}; + +const withRepository = async callback => { + const cwd = await createRepository(); + try { + await callback(cwd); + } finally { + await fs.rm(cwd, { recursive: true, force: true }); + } +}; + +const changeset = releases => { + const frontmatter = releases + .map(([name, type]) => `'${name}': ${type}`) + .join('\n'); + return `---\n${frontmatter}\n---\n\nTest change.\n`; +}; + +test('parses deleted and renamed files from a name-status diff', () => { + const entries = parseNameStatus( + 'D\0packages/common/src/old.js\0R100\0packages/gmail/src/old.js\0packages/gmail/src/new.js\0' + ); + + assert.deepEqual(entries, [ + { status: 'D', paths: ['packages/common/src/old.js'] }, + { + status: 'R100', + paths: ['packages/gmail/src/old.js', 'packages/gmail/src/new.js'], + }, + ]); + + const directories = findChangedAdaptorDirectories(entries); + assert.deepEqual([...directories.keys()], ['common', 'gmail']); +}); + +test('only treats the destination of a copied file as changed', () => { + const entries = parseNameStatus( + 'C100\0packages/common/src/source.js\0packages/gmail/src/copy.js\0' + ); + + const directories = findChangedAdaptorDirectories(entries); + + assert.deepEqual([...directories.keys()], ['gmail']); +}); + +test('ignores repository files and the changeset README', () => { + const entries = [ + { status: 'M', paths: ['README.md'] }, + { status: 'M', paths: ['.changeset/README.md'] }, + { status: 'A', paths: ['.changeset/valid.md'] }, + ]; + + assert.equal(findChangedAdaptorDirectories(entries).size, 0); + assert.deepEqual(findChangesetFiles(entries), ['.changeset/valid.md']); +}); + +test('repository-only changes do not require a changeset', async () => + withRepository(async cwd => { + await write(cwd, 'README.md', '# Updated repository\n'); + + const inspection = await inspectRepository({ cwd, baseRef: 'main' }); + + assert.deepEqual(inspection.changedPackages, []); + })); + +test('detects one changed adaptor without a changeset', async () => + withRepository(async cwd => { + await write(cwd, 'packages/common/index.js', 'export const value = 1;\n'); + + const inspection = await inspectRepository({ cwd, baseRef: 'main' }); + const parsed = await parseChangesetTargets(inspection.changesetFiles, { + cwd, + }); + + assert.deepEqual( + findMissingPackages(inspection.changedPackages, parsed.targets).map( + pkg => pkg.name + ), + ['@openfn/language-common'] + ); + })); + +test('passes when one changed adaptor is targeted', async () => + withRepository(async cwd => { + await write(cwd, 'packages/common/index.js', 'export const value = 1;\n'); + await write( + cwd, + '.changeset/common.md', + changeset([['@openfn/language-common', 'patch']]) + ); + + const inspection = await inspectRepository({ cwd, baseRef: 'main' }); + const parsed = await parseChangesetTargets(inspection.changesetFiles, { + cwd, + }); + + assert.deepEqual( + findMissingPackages(inspection.changedPackages, parsed.targets), + [] + ); + })); + +test('fails when only one of two changed adaptors is targeted', async () => + withRepository(async cwd => { + await write(cwd, 'packages/common/index.js', 'export const common = 1;\n'); + await write(cwd, 'packages/gmail/index.js', 'export const gmail = 1;\n'); + await write( + cwd, + '.changeset/gmail.md', + changeset([['@openfn/language-gmail', 'minor']]) + ); + + const inspection = await inspectRepository({ cwd, baseRef: 'main' }); + const parsed = await parseChangesetTargets(inspection.changesetFiles, { + cwd, + }); + + assert.deepEqual( + findMissingPackages(inspection.changedPackages, parsed.targets).map( + pkg => pkg.name + ), + ['@openfn/language-common'] + ); + })); + +test('passes when one changeset targets two changed adaptors', async () => + withRepository(async cwd => { + await write(cwd, 'packages/common/index.js', 'export const common = 1;\n'); + await write(cwd, 'packages/gmail/index.js', 'export const gmail = 1;\n'); + await write( + cwd, + '.changeset/both.md', + changeset([ + ['@openfn/language-common', 'patch'], + ['@openfn/language-gmail', 'minor'], + ]) + ); + + const inspection = await inspectRepository({ cwd, baseRef: 'main' }); + const parsed = await parseChangesetTargets(inspection.changesetFiles, { + cwd, + }); + + assert.deepEqual( + findMissingPackages(inspection.changedPackages, parsed.targets), + [] + ); + })); + +test('passes when separate changesets target two changed adaptors', async () => + withRepository(async cwd => { + await write(cwd, 'packages/common/index.js', 'export const common = 1;\n'); + await write(cwd, 'packages/gmail/index.js', 'export const gmail = 1;\n'); + await write( + cwd, + '.changeset/common.md', + changeset([['@openfn/language-common', 'patch']]) + ); + await write( + cwd, + '.changeset/gmail.md', + changeset([['@openfn/language-gmail', 'minor']]) + ); + + const inspection = await inspectRepository({ cwd, baseRef: 'main' }); + const parsed = await parseChangesetTargets(inspection.changesetFiles, { + cwd, + }); + + assert.deepEqual( + findMissingPackages(inspection.changedPackages, parsed.targets), + [] + ); + })); + +test('an unrelated target does not cover a changed adaptor', () => { + const changedPackages = [ + { + name: '@openfn/language-common', + directories: ['common'], + paths: ['packages/common/index.js'], + }, + ]; + + assert.deepEqual( + findMissingPackages( + changedPackages, + new Set(['@openfn/language-gmail']) + ), + changedPackages + ); +}); + +test('reports empty and malformed changesets', async () => + withRepository(async cwd => { + await write(cwd, '.changeset/empty.md', '---\n---\n'); + await write(cwd, '.changeset/malformed.md', '---\ninvalid: [\n---\n'); + + const parsed = await parseChangesetTargets( + ['.changeset/empty.md', '.changeset/malformed.md'], + { cwd } + ); + + assert.deepEqual( + parsed.errors.map(error => error.file), + ['.changeset/empty.md', '.changeset/malformed.md'] + ); + })); + +test('resolves added and deleted package names from the correct revision', async () => + withRepository(async cwd => { + await fs.rm(path.join(cwd, 'packages/common'), { + recursive: true, + force: true, + }); + await write( + cwd, + 'packages/new/package.json', + packageJson('@openfn/language-new') + ); + await write(cwd, 'packages/new/index.js', 'export const value = 1;\n'); + + const inspection = await inspectRepository({ cwd, baseRef: 'main' }); + + assert.deepEqual( + inspection.changedPackages.map(pkg => pkg.name), + ['@openfn/language-common', '@openfn/language-new'] + ); + })); From a70c3a373bc0e3682b8ed9c273f6e39ebb659e5c Mon Sep 17 00:00:00 2001 From: Alassane Ndoye Date: Sat, 1 Aug 2026 19:12:53 +0000 Subject: [PATCH 4/4] Simplify changeset coverage check --- scripts/check-changesets.mjs | 528 ++++++++++-------------------- scripts/check-changesets.test.mjs | 236 +++++-------- 2 files changed, 253 insertions(+), 511 deletions(-) diff --git a/scripts/check-changesets.mjs b/scripts/check-changesets.mjs index 17f4889d8..f01948497 100644 --- a/scripts/check-changesets.mjs +++ b/scripts/check-changesets.mjs @@ -9,10 +9,10 @@ const rootDir = path.resolve( path.dirname(fileURLToPath(import.meta.url)), '..' ); -const changesetsDocsUrl = +const docsUrl = 'https://github.com/OpenFn/adaptors/blob/main/README.md#changesets'; -const runGit = (args, { cwd = rootDir, allowFailure = false } = {}) => { +const git = (args, { cwd = rootDir, allowFailure = false } = {}) => { try { return execFileSync('git', args, { cwd, @@ -21,458 +21,269 @@ const runGit = (args, { cwd = rootDir, allowFailure = false } = {}) => { }); } catch (error) { if (allowFailure) return null; - - const detail = error.stderr?.trim() || error.message; - throw new Error(`git ${args.join(' ')} failed: ${detail}`); + throw new Error(error.stderr?.trim() || error.message); } }; -const normalizePath = file => file.split(path.sep).join('/'); - -export const parseNameStatus = output => { - const tokens = output.split('\0'); - if (tokens.at(-1) === '') tokens.pop(); - - const entries = []; - for (let index = 0; index < tokens.length; ) { - const status = tokens[index++]; - const pathCount = ['C', 'R'].includes(status[0]) ? 2 : 1; - const paths = tokens.slice(index, index + pathCount); +const splitFiles = output => output.split('\0').filter(Boolean); - if (paths.length !== pathCount) { - throw new Error(`Could not parse git diff entry with status "${status}".`); +const resolveBase = cwd => { + for (const ref of ['origin/main', 'main']) { + if ( + git(['rev-parse', '--verify', '--quiet', `${ref}^{commit}`], { + cwd, + allowFailure: true, + }) + ) { + return ref; } - - entries.push({ status, paths: paths.map(normalizePath) }); - index += pathCount; } - - return entries; + throw new Error('Could not find origin/main or main. Pass --base .'); }; -export const findChangedAdaptorDirectories = entries => { - const adaptors = new Map(); - - for (const entry of entries) { - const changedPaths = - entry.status[0] === 'C' ? [entry.paths.at(-1)] : entry.paths; - - for (const file of changedPaths) { - const match = /^packages\/([^/]+)(?:\/|$)/.exec(file); - if (!match) continue; +const readPackage = async (directory, mergeBase, cwd) => { + const packageFile = `packages/${directory}/package.json`; - const directory = match[1]; - const paths = adaptors.get(directory) ?? new Set(); - paths.add(file); - adaptors.set(directory, paths); + try { + return JSON.parse(await fs.readFile(path.join(cwd, packageFile), 'utf8')); + } catch (error) { + if (error.code !== 'ENOENT') { + throw new Error(`Could not read ${packageFile}: ${error.message}`); } - } - return adaptors; -}; - -export const findChangesetFiles = entries => { - const files = new Set(); - - for (const entry of entries) { - if (!['A', 'M'].includes(entry.status[0])) continue; - - const file = entry.paths.at(-1); - if ( - /^\.changeset\/[^/]+\.md$/.test(file) && - path.posix.basename(file) !== 'README.md' - ) { - files.add(file); + const contents = git(['show', `${mergeBase}:${packageFile}`], { + cwd, + allowFailure: true, + }); + if (!contents) { + throw new Error(`Could not find ${packageFile}.`); } - } - - return [...files].sort(); -}; - -const readJson = (contents, source) => { - try { return JSON.parse(contents); - } catch (error) { - throw new Error(`Invalid JSON in ${source}: ${error.message}`); } }; -const readPackageAtBase = (directory, mergeBase, cwd) => { - const packagePath = `packages/${directory}/package.json`; - const contents = runGit(['show', `${mergeBase}:${packagePath}`], { - cwd, - allowFailure: true, - }); - - if (contents === null) { - throw new Error( - `Could not find ${packagePath} in the working tree or at ${mergeBase}.` - ); - } - - return readJson(contents, `${mergeBase}:${packagePath}`); -}; +export const inspectRepository = async ({ + cwd = rootDir, + baseRef, +} = {}) => { + const base = baseRef ?? resolveBase(cwd); + const mergeBase = git(['merge-base', base, 'HEAD'], { cwd }).trim(); + const untrackedFiles = splitFiles( + git(['ls-files', '--others', '--exclude-standard', '-z'], { cwd }) + ); + const changedFiles = [ + ...splitFiles( + git(['diff', '--name-only', '--no-renames', '-z', mergeBase, '--'], { + cwd, + }) + ), + ...untrackedFiles, + ]; -const readPackage = async (directory, mergeBase, cwd) => { - const packagePath = path.join(cwd, 'packages', directory, 'package.json'); + const pathsByDirectory = new Map(); + for (const file of new Set(changedFiles)) { + const match = /^packages\/([^/]+)\//.exec(file); + if (!match) continue; - try { - return readJson( - await fs.readFile(packagePath, 'utf8'), - normalizePath(path.relative(cwd, packagePath)) - ); - } catch (error) { - if (error.code !== 'ENOENT') throw error; - return readPackageAtBase(directory, mergeBase, cwd); + const paths = pathsByDirectory.get(match[1]) ?? []; + paths.push(file); + pathsByDirectory.set(match[1], paths); } -}; -export const resolveChangedPackages = async ( - adaptorDirectories, - mergeBase, - { cwd = rootDir } = {} -) => { const packagesByName = new Map(); - - for (const [directory, changedPaths] of adaptorDirectories) { + for (const [directory, files] of pathsByDirectory) { const packageJson = await readPackage(directory, mergeBase, cwd); - - if ( - typeof packageJson.name !== 'string' || - !packageJson.name.startsWith('@openfn/language-') - ) { + if (!packageJson.name?.startsWith('@openfn/language-')) { throw new Error( - `packages/${directory}/package.json does not define an OpenFn adaptor package name.` + `packages/${directory}/package.json has no OpenFn adaptor package name.` ); } - const existing = packagesByName.get(packageJson.name) ?? { - name: packageJson.name, - directories: new Set(), - paths: new Set(), - }; - existing.directories.add(directory); - for (const file of changedPaths) existing.paths.add(file); - packagesByName.set(packageJson.name, existing); + const paths = packagesByName.get(packageJson.name) ?? []; + paths.push(...files); + packagesByName.set(packageJson.name, paths); } - - return [...packagesByName.values()] - .map(pkg => ({ - name: pkg.name, - directories: [...pkg.directories].sort(), - paths: [...pkg.paths].sort(), - })) + const changedPackages = [...packagesByName] + .map(([name, files]) => ({ name, paths: [...new Set(files)].sort() })) .sort((a, b) => a.name.localeCompare(b.name)); -}; -const getDefaultBaseRef = async cwd => { - let baseBranch = 'main'; - - try { - const config = readJson( - await fs.readFile(path.join(cwd, '.changeset', 'config.json'), 'utf8'), - '.changeset/config.json' - ); - if (typeof config.baseBranch === 'string') { - baseBranch = config.baseBranch; - } - } catch (error) { - if (error.code !== 'ENOENT') throw error; - } - - for (const candidate of [`origin/${baseBranch}`, baseBranch]) { - const commit = runGit( - ['rev-parse', '--verify', '--quiet', `${candidate}^{commit}`], - { cwd, allowFailure: true } - ); - if (commit !== null) return candidate; - } - - throw new Error( - `Could not resolve origin/${baseBranch} or ${baseBranch}. Pass --base .` - ); -}; - -const getUntrackedEntries = cwd => { - const output = runGit( - ['ls-files', '--others', '--exclude-standard', '-z'], - { cwd } - ); - - return output - .split('\0') - .filter(Boolean) - .map(file => ({ status: 'A', paths: [normalizePath(file)] })); -}; - -export const inspectRepository = async ({ - cwd = rootDir, - baseRef, -} = {}) => { - const resolvedBase = baseRef ?? (await getDefaultBaseRef(cwd)); - const mergeBase = runGit(['merge-base', resolvedBase, 'HEAD'], { - cwd, - }).trim(); - - if (!mergeBase) { - throw new Error(`Could not find a merge base for ${resolvedBase} and HEAD.`); - } - - const entries = [ - ...parseNameStatus( - runGit( + const changesetFiles = [ + ...splitFiles( + git( [ 'diff', - '--name-status', + '--name-only', '-z', - '--find-renames', - '--find-copies', + '--diff-filter=AM', mergeBase, '--', + '.changeset/*.md', ], { cwd } ) ), - ...getUntrackedEntries(cwd), - ]; - const adaptorDirectories = findChangedAdaptorDirectories(entries); - const changedPackages = await resolveChangedPackages( - adaptorDirectories, - mergeBase, - { cwd } - ); + ...untrackedFiles.filter(file => /^\.changeset\/[^/]+\.md$/.test(file)), + ] + .filter(file => file !== '.changeset/README.md') + .filter((file, index, files) => files.indexOf(file) === index) + .sort(); - return { - baseRef: resolvedBase, - mergeBase, - entries, - changedPackages, - changesetFiles: findChangesetFiles(entries), - }; + return { changedPackages, changesetFiles }; }; -export const parseChangesetTargets = async ( - files, - { cwd = rootDir, parser } = {} -) => { +export const evaluateCoverage = async ({ + cwd = rootDir, + baseRef, + parser, +} = {}) => { + const inspection = await inspectRepository({ cwd, baseRef }); + if (!inspection.changedPackages.length) { + return { ...inspection, covered: [], missing: [], errors: [] }; + } + const parse = parser ?? (await import('@changesets/parse')).default; - const targets = new Set(); - const parsedFiles = []; + const covered = new Set(); const errors = []; - for (const file of files) { + for (const file of inspection.changesetFiles) { try { const changeset = parse(await fs.readFile(path.join(cwd, file), 'utf8')); - if (!changeset.releases.length) { - throw new Error('the changeset does not target any packages'); + throw new Error('does not target any packages'); } - - const releases = changeset.releases.map(release => release.name).sort(); - for (const name of releases) targets.add(name); - parsedFiles.push({ file, releases }); + for (const release of changeset.releases) covered.add(release.name); } catch (error) { - errors.push({ file, message: error.message }); + errors.push(`${file}: ${error.message}`); } } - return { targets, parsedFiles, errors }; + return { + ...inspection, + covered: [...covered].sort(), + missing: inspection.changedPackages.filter( + pkg => !covered.has(pkg.name) + ), + errors, + }; }; -export const findMissingPackages = (changedPackages, targets) => - changedPackages.filter(pkg => !targets.has(pkg.name)); - -const appendFileFromEnvironment = async (variable, contents, environment) => { - const file = environment[variable]; - if (file) await fs.appendFile(file, `${contents.trimEnd()}\n`); +const appendEnvironmentFile = async (environment, name, contents) => { + if (environment[name]) { + await fs.appendFile(environment[name], `${contents.trimEnd()}\n`); + } }; -const appendSummary = (contents, environment) => - appendFileFromEnvironment('GITHUB_STEP_SUMMARY', contents, environment); - -const writeOutputs = (values, environment) => - appendFileFromEnvironment( - 'GITHUB_OUTPUT', - Object.entries(values) - .map(([key, value]) => `${key}=${value}`) - .join('\n'), - environment - ); - -const formatPackageList = packages => +const packageList = packages => packages.map(pkg => `- \`${pkg.name}\``).join('\n'); -const printNoChangesetRequired = async environment => { +const reportNotRequired = async environment => { console.log('No adaptor packages changed; a changeset is not required.'); - await appendSummary( + await appendEnvironmentFile( + environment, + 'GITHUB_STEP_SUMMARY', `## Changeset not required -No files under \`packages/*\` changed in this pull request.`, - environment +No files under \`packages/*\` changed in this pull request.` ); }; -const runDetect = async (inspection, environment) => { - const required = inspection.changedPackages.length > 0; - - await writeOutputs( - { - required: String(required), - 'changed-packages': JSON.stringify( - inspection.changedPackages.map(pkg => pkg.name) - ), - }, - environment +const detect = async (baseRef, environment) => { + const { changedPackages } = await inspectRepository({ baseRef }); + const required = changedPackages.length > 0; + + await appendEnvironmentFile( + environment, + 'GITHUB_OUTPUT', + `required=${required} +changed-packages=${JSON.stringify(changedPackages.map(pkg => pkg.name))}` ); - if (!required) { - await printNoChangesetRequired(environment); - return; - } + if (!required) return reportNotRequired(environment); console.log('Changeset coverage is required for:'); - for (const pkg of inspection.changedPackages) console.log(`- ${pkg.name}`); - - await appendSummary( + changedPackages.forEach(pkg => console.log(`- ${pkg.name}`)); + await appendEnvironmentFile( + environment, + 'GITHUB_STEP_SUMMARY', `## Changeset required -The following adaptor packages changed: - -${formatPackageList(inspection.changedPackages)}`, - environment +${packageList(changedPackages)}` ); }; -const formatFailure = (missingPackages, errors) => { - const lines = ['Changeset coverage check failed.', '']; - - if (missingPackages.length) { - lines.push('Missing changesets for:'); - for (const pkg of missingPackages) { - lines.push(`- ${pkg.name}`); - for (const file of pkg.paths) lines.push(` - ${file}`); - } - lines.push(''); - } - - if (errors.length) { - lines.push('Invalid changeset files:'); - for (const error of errors) { - lines.push(`- ${error.file}: ${error.message}`); - } - lines.push(''); +const validate = async (baseRef, environment) => { + const result = await evaluateCoverage({ baseRef }); + if (!result.changedPackages.length) { + await reportNotRequired(environment); + return true; } - lines.push("Run 'pnpm changeset' and commit the generated file."); - lines.push( - "If no release is needed, ask a maintainer to apply the exact 'no changeset required' label." - ); - lines.push(`See ${changesetsDocsUrl}.`); - - return lines.join('\n'); -}; - -const formatFailureSummary = (missingPackages, errors) => { - const lines = ['## Changeset coverage failed', '']; - - if (missingPackages.length) { - lines.push('### Missing adaptor packages', ''); - for (const pkg of missingPackages) { - lines.push(`- \`${pkg.name}\``); - for (const file of pkg.paths) lines.push(` - \`${file}\``); + if (result.missing.length || result.errors.length) { + const lines = ['Changeset coverage check failed.']; + if (result.missing.length) { + lines.push('', 'Missing changesets for:'); + for (const pkg of result.missing) { + lines.push(`- ${pkg.name}`); + pkg.paths.forEach(file => lines.push(` - ${file}`)); + } } - lines.push(''); - } - - if (errors.length) { - lines.push('### Invalid changeset files', ''); - for (const error of errors) { - lines.push(`- \`${error.file}\`: ${error.message}`); + if (result.errors.length) { + lines.push('', 'Invalid changesets:'); + result.errors.forEach(error => lines.push(`- ${error}`)); } - lines.push(''); - } - - lines.push( - 'Run `pnpm changeset`, commit the generated file, and push it.', - '', - 'If the change does not need a release, ask a maintainer to apply the exact `no changeset required` label.', - '', - `See the [Changesets documentation](${changesetsDocsUrl}) for details.` - ); - - return lines.join('\n'); -}; - -const escapeWorkflowCommand = value => - value.replace(/%/g, '%25').replace(/\r/g, '%0D').replace(/\n/g, '%0A'); - -const runValidate = async (inspection, environment) => { - if (!inspection.changedPackages.length) { - await printNoChangesetRequired(environment); - return true; - } - - const parsed = await parseChangesetTargets(inspection.changesetFiles); - const missingPackages = findMissingPackages( - inspection.changedPackages, - parsed.targets - ); - - if (missingPackages.length || parsed.errors.length) { - const failure = formatFailure(missingPackages, parsed.errors); - console.error(failure); + lines.push( + '', + "Run 'pnpm changeset' and commit the generated file.", + "If no release is needed, ask a maintainer to apply the exact 'no changeset required' label.", + `See ${docsUrl}.` + ); + const message = lines.join('\n'); + console.error(message); if (environment.GITHUB_ACTIONS === 'true') { - console.error( - `::error title=Changeset required::${escapeWorkflowCommand(failure)}` - ); + const annotation = message + .replace(/%/g, '%25') + .replace(/\r/g, '%0D') + .replace(/\n/g, '%0A'); + console.error(`::error title=Changeset required::${annotation}`); } - - await appendSummary( - formatFailureSummary(missingPackages, parsed.errors), - environment + await appendEnvironmentFile( + environment, + 'GITHUB_STEP_SUMMARY', + `## Changeset coverage failed + +\`\`\` +${message} +\`\`\`` ); return false; } console.log('Changesets cover every changed adaptor package.'); - for (const pkg of inspection.changedPackages) console.log(`- ${pkg.name}`); - - const changesetList = parsed.parsedFiles - .map(item => `- \`${item.file}\``) - .join('\n'); - await appendSummary( + result.changedPackages.forEach(pkg => console.log(`- ${pkg.name}`)); + await appendEnvironmentFile( + environment, + 'GITHUB_STEP_SUMMARY', `## Changeset coverage passed -Covered adaptor packages: - -${formatPackageList(inspection.changedPackages)} - -Changeset files: - -${changesetList}`, - environment +${packageList(result.changedPackages)}` ); return true; }; const parseArguments = argv => { - const [mode, ...args] = argv; + const mode = argv[0]; if (!['detect', 'validate'].includes(mode)) { throw new Error( 'Usage: node scripts/check-changesets.mjs [--base ]' ); } - let baseRef; - for (let index = 0; index < args.length; index += 1) { - if (args[index] !== '--base' || !args[index + 1]) { - throw new Error(`Unknown or incomplete argument: ${args[index]}`); - } - baseRef = args[++index]; + if (argv.length === 1) return { mode }; + if (argv.length === 3 && argv[1] === '--base') { + return { mode, baseRef: argv[2] }; } - - return { mode, baseRef }; + throw new Error('Expected an optional --base argument.'); }; export const main = async ( @@ -480,14 +291,11 @@ export const main = async ( environment = process.env ) => { const { mode, baseRef } = parseArguments(argv); - const inspection = await inspectRepository({ baseRef }); - if (mode === 'detect') { - await runDetect(inspection, environment); + await detect(baseRef, environment); return true; } - - return runValidate(inspection, environment); + return validate(baseRef, environment); }; const isMain = diff --git a/scripts/check-changesets.test.mjs b/scripts/check-changesets.test.mjs index a27139ab3..9ffb439c8 100644 --- a/scripts/check-changesets.test.mjs +++ b/scripts/check-changesets.test.mjs @@ -6,16 +6,16 @@ import path from 'node:path'; import test from 'node:test'; import { - findChangedAdaptorDirectories, - findChangesetFiles, - findMissingPackages, + evaluateCoverage, inspectRepository, - parseChangesetTargets, - parseNameStatus, } from './check-changesets.mjs'; -const runGit = (cwd, ...args) => - execFileSync('git', args, { cwd, encoding: 'utf8' }); +const git = (cwd, ...args) => + execFileSync('git', args, { + cwd, + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'pipe'], + }); const write = async (cwd, file, contents) => { const target = path.join(cwd, file); @@ -27,31 +27,27 @@ const packageJson = name => JSON.stringify({ name }, null, 2); const createRepository = async () => { const cwd = await fs.mkdtemp(path.join(os.tmpdir(), 'changeset-check-')); - runGit(cwd, 'init', '--initial-branch=main'); - runGit(cwd, 'config', 'user.name', 'Changeset Test'); - runGit(cwd, 'config', 'user.email', 'changeset@example.com'); + git(cwd, 'init', '--initial-branch=main'); + git(cwd, 'config', 'user.name', 'Changeset Test'); + git(cwd, 'config', 'user.email', 'changeset@example.com'); - await write( - cwd, - '.changeset/config.json', - JSON.stringify({ baseBranch: 'main' }) - ); await write( cwd, 'packages/common/package.json', packageJson('@openfn/language-common') ); + await write(cwd, 'packages/common/index.js', 'export const common = 1;\n'); await write( cwd, 'packages/gmail/package.json', packageJson('@openfn/language-gmail') ); + await write(cwd, 'packages/gmail/index.js', 'export const gmail = 1;\n'); await write(cwd, 'README.md', '# Test repository\n'); - runGit(cwd, 'add', '.'); - runGit(cwd, 'commit', '-m', 'Initial commit'); - runGit(cwd, 'switch', '-c', 'feature'); - + git(cwd, 'add', '.'); + git(cwd, 'commit', '-m', 'Initial commit'); + git(cwd, 'switch', '-c', 'feature'); return cwd; }; @@ -71,117 +67,65 @@ const changeset = releases => { return `---\n${frontmatter}\n---\n\nTest change.\n`; }; -test('parses deleted and renamed files from a name-status diff', () => { - const entries = parseNameStatus( - 'D\0packages/common/src/old.js\0R100\0packages/gmail/src/old.js\0packages/gmail/src/new.js\0' - ); - - assert.deepEqual(entries, [ - { status: 'D', paths: ['packages/common/src/old.js'] }, - { - status: 'R100', - paths: ['packages/gmail/src/old.js', 'packages/gmail/src/new.js'], - }, - ]); - - const directories = findChangedAdaptorDirectories(entries); - assert.deepEqual([...directories.keys()], ['common', 'gmail']); -}); - -test('only treats the destination of a copied file as changed', () => { - const entries = parseNameStatus( - 'C100\0packages/common/src/source.js\0packages/gmail/src/copy.js\0' - ); - - const directories = findChangedAdaptorDirectories(entries); - - assert.deepEqual([...directories.keys()], ['gmail']); -}); - -test('ignores repository files and the changeset README', () => { - const entries = [ - { status: 'M', paths: ['README.md'] }, - { status: 'M', paths: ['.changeset/README.md'] }, - { status: 'A', paths: ['.changeset/valid.md'] }, - ]; - - assert.equal(findChangedAdaptorDirectories(entries).size, 0); - assert.deepEqual(findChangesetFiles(entries), ['.changeset/valid.md']); -}); +const packageNames = result => result.changedPackages.map(pkg => pkg.name); +const missingNames = result => result.missing.map(pkg => pkg.name); -test('repository-only changes do not require a changeset', async () => +test('repository-only changes need no changeset', async () => withRepository(async cwd => { await write(cwd, 'README.md', '# Updated repository\n'); + const result = await evaluateCoverage({ cwd, baseRef: 'main' }); - const inspection = await inspectRepository({ cwd, baseRef: 'main' }); - - assert.deepEqual(inspection.changedPackages, []); + assert.deepEqual(result.changedPackages, []); + assert.deepEqual(result.missing, []); })); -test('detects one changed adaptor without a changeset', async () => +test('ignores the changeset README', async () => withRepository(async cwd => { - await write(cwd, 'packages/common/index.js', 'export const value = 1;\n'); + await write(cwd, '.changeset/README.md', 'Documentation only.\n'); + const result = await inspectRepository({ cwd, baseRef: 'main' }); - const inspection = await inspectRepository({ cwd, baseRef: 'main' }); - const parsed = await parseChangesetTargets(inspection.changesetFiles, { - cwd, - }); + assert.deepEqual(result.changesetFiles, []); + })); - assert.deepEqual( - findMissingPackages(inspection.changedPackages, parsed.targets).map( - pkg => pkg.name - ), - ['@openfn/language-common'] - ); +test('reports a changed adaptor without a changeset', async () => + withRepository(async cwd => { + await write(cwd, 'packages/common/index.js', 'export const common = 2;\n'); + const result = await evaluateCoverage({ cwd, baseRef: 'main' }); + + assert.deepEqual(missingNames(result), ['@openfn/language-common']); })); -test('passes when one changed adaptor is targeted', async () => +test('accepts a changeset targeting the changed adaptor', async () => withRepository(async cwd => { - await write(cwd, 'packages/common/index.js', 'export const value = 1;\n'); + await write(cwd, 'packages/common/index.js', 'export const common = 2;\n'); await write( cwd, '.changeset/common.md', changeset([['@openfn/language-common', 'patch']]) ); + const result = await evaluateCoverage({ cwd, baseRef: 'main' }); - const inspection = await inspectRepository({ cwd, baseRef: 'main' }); - const parsed = await parseChangesetTargets(inspection.changesetFiles, { - cwd, - }); - - assert.deepEqual( - findMissingPackages(inspection.changedPackages, parsed.targets), - [] - ); + assert.deepEqual(result.missing, []); })); -test('fails when only one of two changed adaptors is targeted', async () => +test('reports partial coverage across two adaptors', async () => withRepository(async cwd => { - await write(cwd, 'packages/common/index.js', 'export const common = 1;\n'); - await write(cwd, 'packages/gmail/index.js', 'export const gmail = 1;\n'); + await write(cwd, 'packages/common/index.js', 'export const common = 2;\n'); + await write(cwd, 'packages/gmail/index.js', 'export const gmail = 2;\n'); await write( cwd, '.changeset/gmail.md', changeset([['@openfn/language-gmail', 'minor']]) ); + const result = await evaluateCoverage({ cwd, baseRef: 'main' }); - const inspection = await inspectRepository({ cwd, baseRef: 'main' }); - const parsed = await parseChangesetTargets(inspection.changesetFiles, { - cwd, - }); - - assert.deepEqual( - findMissingPackages(inspection.changedPackages, parsed.targets).map( - pkg => pkg.name - ), - ['@openfn/language-common'] - ); + assert.deepEqual(missingNames(result), ['@openfn/language-common']); })); -test('passes when one changeset targets two changed adaptors', async () => +test('accepts one changeset targeting two adaptors', async () => withRepository(async cwd => { - await write(cwd, 'packages/common/index.js', 'export const common = 1;\n'); - await write(cwd, 'packages/gmail/index.js', 'export const gmail = 1;\n'); + await write(cwd, 'packages/common/index.js', 'export const common = 2;\n'); + await write(cwd, 'packages/gmail/index.js', 'export const gmail = 2;\n'); await write( cwd, '.changeset/both.md', @@ -190,22 +134,15 @@ test('passes when one changeset targets two changed adaptors', async () => ['@openfn/language-gmail', 'minor'], ]) ); + const result = await evaluateCoverage({ cwd, baseRef: 'main' }); - const inspection = await inspectRepository({ cwd, baseRef: 'main' }); - const parsed = await parseChangesetTargets(inspection.changesetFiles, { - cwd, - }); - - assert.deepEqual( - findMissingPackages(inspection.changedPackages, parsed.targets), - [] - ); + assert.deepEqual(result.missing, []); })); -test('passes when separate changesets target two changed adaptors', async () => +test('accepts separate changesets targeting two adaptors', async () => withRepository(async cwd => { - await write(cwd, 'packages/common/index.js', 'export const common = 1;\n'); - await write(cwd, 'packages/gmail/index.js', 'export const gmail = 1;\n'); + await write(cwd, 'packages/common/index.js', 'export const common = 2;\n'); + await write(cwd, 'packages/gmail/index.js', 'export const gmail = 2;\n'); await write( cwd, '.changeset/common.md', @@ -216,53 +153,51 @@ test('passes when separate changesets target two changed adaptors', async () => '.changeset/gmail.md', changeset([['@openfn/language-gmail', 'minor']]) ); + const result = await evaluateCoverage({ cwd, baseRef: 'main' }); - const inspection = await inspectRepository({ cwd, baseRef: 'main' }); - const parsed = await parseChangesetTargets(inspection.changesetFiles, { - cwd, - }); + assert.deepEqual(result.missing, []); + })); - assert.deepEqual( - findMissingPackages(inspection.changedPackages, parsed.targets), - [] +test('an unrelated changeset does not cover a changed adaptor', async () => + withRepository(async cwd => { + await write(cwd, 'packages/common/index.js', 'export const common = 2;\n'); + await write( + cwd, + '.changeset/gmail.md', + changeset([['@openfn/language-gmail', 'patch']]) ); - })); + const result = await evaluateCoverage({ cwd, baseRef: 'main' }); -test('an unrelated target does not cover a changed adaptor', () => { - const changedPackages = [ - { - name: '@openfn/language-common', - directories: ['common'], - paths: ['packages/common/index.js'], - }, - ]; - - assert.deepEqual( - findMissingPackages( - changedPackages, - new Set(['@openfn/language-gmail']) - ), - changedPackages - ); -}); + assert.deepEqual(missingNames(result), ['@openfn/language-common']); + })); test('reports empty and malformed changesets', async () => withRepository(async cwd => { + await write(cwd, 'packages/common/index.js', 'export const common = 2;\n'); await write(cwd, '.changeset/empty.md', '---\n---\n'); await write(cwd, '.changeset/malformed.md', '---\ninvalid: [\n---\n'); + const result = await evaluateCoverage({ cwd, baseRef: 'main' }); - const parsed = await parseChangesetTargets( - ['.changeset/empty.md', '.changeset/malformed.md'], - { cwd } - ); + assert.equal(result.errors.length, 2); + })); - assert.deepEqual( - parsed.errors.map(error => error.file), - ['.changeset/empty.md', '.changeset/malformed.md'] +test('detects both adaptors in a cross-adaptor rename', async () => + withRepository(async cwd => { + git( + cwd, + 'mv', + 'packages/common/index.js', + 'packages/gmail/from-common.js' ); + const result = await inspectRepository({ cwd, baseRef: 'main' }); + + assert.deepEqual(packageNames(result), [ + '@openfn/language-common', + '@openfn/language-gmail', + ]); })); -test('resolves added and deleted package names from the correct revision', async () => +test('resolves added and deleted package names', async () => withRepository(async cwd => { await fs.rm(path.join(cwd, 'packages/common'), { recursive: true, @@ -274,11 +209,10 @@ test('resolves added and deleted package names from the correct revision', async packageJson('@openfn/language-new') ); await write(cwd, 'packages/new/index.js', 'export const value = 1;\n'); + const result = await inspectRepository({ cwd, baseRef: 'main' }); - const inspection = await inspectRepository({ cwd, baseRef: 'main' }); - - assert.deepEqual( - inspection.changedPackages.map(pkg => pkg.name), - ['@openfn/language-common', '@openfn/language-new'] - ); + assert.deepEqual(packageNames(result), [ + '@openfn/language-common', + '@openfn/language-new', + ]); }));