diff --git a/scripts/tests/changed-yaml-files.test.ts b/scripts/tests/changed-yaml-files.test.ts new file mode 100644 index 0000000..b7dfd4d --- /dev/null +++ b/scripts/tests/changed-yaml-files.test.ts @@ -0,0 +1,58 @@ +import { describe, it, expect } from 'vitest'; +import { filterPackageYamlPaths } from '../validate.js'; + +describe('filterPackageYamlPaths', () => { + it('returns only packages/**/*.yml entries (excluding meta.yml)', () => { + const diff = [ + 'packages/acme/foo.yml', + 'packages/acme/meta.yml', + 'packages/phpvms/bar.yml', + 'README.md', + 'docs/operations.md', + 'schema/package.schema.json', + 'scripts/validate.ts', + ].join('\n'); + + expect(filterPackageYamlPaths(diff)).toEqual([ + 'packages/acme/foo.yml', + 'packages/phpvms/bar.yml', + ]); + }); + + it('handles trailing newline + blank lines + whitespace', () => { + const diff = '\n packages/acme/foo.yml \n\npackages/acme/bar.yml\n\n'; + expect(filterPackageYamlPaths(diff)).toEqual([ + 'packages/acme/foo.yml', + 'packages/acme/bar.yml', + ]); + }); + + it('returns empty array for empty input', () => { + expect(filterPackageYamlPaths('')).toEqual([]); + expect(filterPackageYamlPaths('\n\n')).toEqual([]); + }); + + it('does NOT depend on diff-filter — it trusts the caller to supply the right diff', () => { + // This test documents the contract: if a deleted path slips in, the + // filter still passes it through (because the function cannot + // distinguish A/M/D from --name-only output). The fix lives at the + // `git diff` invocation site (--diff-filter=ACMRT). Documented here + // so the responsibility is explicit. + const diff = 'packages/acme/deleted.yml\npackages/acme/added.yml'; + expect(filterPackageYamlPaths(diff)).toEqual([ + 'packages/acme/deleted.yml', + 'packages/acme/added.yml', + ]); + }); + + it('rejects non-yaml extensions and paths outside packages/', () => { + const diff = [ + 'packages/acme/foo.yaml', + 'packages/acme/foo.yml.bak', + 'tests/packages/acme/foo.yml', + '.github/workflows/validate-pr.yml', + 'packages/acme/foo.yml', + ].join('\n'); + expect(filterPackageYamlPaths(diff)).toEqual(['packages/acme/foo.yml']); + }); +}); diff --git a/scripts/validate.ts b/scripts/validate.ts index 989242d..0f9d55c 100644 --- a/scripts/validate.ts +++ b/scripts/validate.ts @@ -55,15 +55,19 @@ function readEnv(): Env { }; } -function changedYamlFiles(repoRoot: string, baseSha: string, headSha: string): string[] { - // `git diff` pathspecs do NOT support `**`; passing it makes git look for - // a literal directory of that name and silently match nothing. Use an - // unfiltered diff and filter in JS instead — same outcome, no traps. - const out = execFileSync('git', ['diff', '--name-only', `${baseSha}...${headSha}`], { - cwd: repoRoot, - encoding: 'utf8', - }); - return out +/** + * Parse `git diff --name-only` output into the subset of paths the + * validator should inspect: package YAMLs (excluding meta.yml). + * + * Exported for unit testing. Callers feed the raw stdout from + * `git diff --name-only --diff-filter=ACMRT BASE...HEAD` so deleted + * paths never reach this filter — `runPackageChecks` would otherwise + * crash with ENOENT trying to read a file that no longer exists in + * HEAD (regression discovered while exercising task 12.7 of + * bootstrap-addon-registry). + */ +export function filterPackageYamlPaths(diffOutput: string): string[] { + return diffOutput .split('\n') .map((l) => l.trim()) .filter((l) => l.length > 0) @@ -71,6 +75,23 @@ function changedYamlFiles(repoRoot: string, baseSha: string, headSha: string): s .filter((l) => path.basename(l) !== 'meta.yml'); } +function changedYamlFiles(repoRoot: string, baseSha: string, headSha: string): string[] { + // `git diff` pathspecs do NOT support `**`; passing it makes git look for + // a literal directory of that name and silently match nothing. Use an + // unfiltered diff and filter in JS instead — same outcome, no traps. + // + // `--diff-filter=ACMRT` excludes deleted (`D`) paths so the validator + // does not try to read a YAML file the PR removed. Renamed/copied + // entries surface their *new* path under these letters, which is what + // we want to validate. + const out = execFileSync( + 'git', + ['diff', '--name-only', '--diff-filter=ACMRT', `${baseSha}...${headSha}`], + { cwd: repoRoot, encoding: 'utf8' }, + ); + return filterPackageYamlPaths(out); +} + async function main(): Promise { const env = readEnv(); const repoIdent = parseRepository(env.repoSpec); @@ -116,7 +137,13 @@ async function main(): Promise { console.log('\nValidation passed.'); } -main().catch((err) => { - console.error(err); - process.exit(1); -}); +// Only auto-run when invoked as a script, not when imported (e.g. from +// unit tests). `process.argv[1]` is the entry point file path; ESM has +// no `require.main === module` equivalent so we compare paths via URL. +const entryUrl = process.argv[1] ? new URL(`file://${path.resolve(process.argv[1])}`).href : ''; +if (import.meta.url === entryUrl) { + main().catch((err) => { + console.error(err); + process.exit(1); + }); +}