* fix(#1006): harden render --preview against fragment parse failures `render --preview` wrote `report.preview` unconditionally. When a `.changeset` fragment fails to parse, `cmdRender` early-returns with `{exitCode:1, report: {failures}}` and NO `preview` key, so `process.stdout.write(undefined)` threw ERR_INVALID_ARG_TYPE and the rc release job's "Preview CHANGELOG" step died with a cryptic TypeError that masked the real cause. Guard the preview write on `typeof report.preview === 'string'` (ADR-227: shape, not just type); when absent, fall through to the existing failure reporter that names the offending fragment and exits non-zero — identical to a non-preview render. Also backfills the stray placeholder `pr: 0` -> `pr: 939` in .changeset/936-convergence-inline-plan-phase.md that triggered the live failure. Regression test (red-then-green verified) added at the render --preview seam. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#1006): validate changeset fragment content at the Changeset Required gate The `Changeset Required` gate (scripts/changeset/lint.cjs) only checked that a `.changeset/*.md` fragment EXISTS in the PR diff; it never validated the fragment's contents. So a malformed fragment (e.g. an un-backfilled `pr: 0` placeholder) silently merged to `next` and only detonated later in the rc release job. This is the upstream prevention for #1006 — the crash hardening turns the failure into a clear message, this stops the bad fragment ever reaching the release path. evaluateLint now accepts `fragmentFailures` and fails with the typed reason `fail_invalid_fragment` (naming each offending file) before the existence/ opt-out checks — a malformed fragment beats `no-changelog`, since it will break the render regardless. main() reads + parseFragment()s every changed fragment: a deleted fragment (not on disk) is skipped, a present-but-unreadable one fails closed. Tests assert on the typed LINT_REASON enum (no raw-text matching), a precedence case over the opt-out label, and an end-to-end suite that drives the real main() against a temp git repo (malformed -> fail, valid -> pass, deleted -> skipped) so the wiring is regression-proof. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(#1006): assert the typed --json report in the preview regression test Code review flagged the preview parse-failure regression test for positive raw-text matching on CLI output (`combined.includes('bad-fragment.md')` / `'invalid_pr'`), which this repo's testing standards forbid. Keep the non-json `runRenderRaw` call for the negative crash proof (the ERR_INVALID_ARG_TYPE crash lives only on the non-json stdout.write path), and add a `--json` invocation that asserts the offending fragment + typed `invalid_pr` reason via the structured `report.failures[]` surface instead of rendered prose. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
268 lines
10 KiB
JavaScript
268 lines
10 KiB
JavaScript
'use strict';
|
|
process.env.GSD_TEST_MODE = '1';
|
|
|
|
const { test, describe } = require('node:test');
|
|
const assert = require('node:assert/strict');
|
|
const path = require('node:path');
|
|
const fs = require('node:fs');
|
|
const os = require('node:os');
|
|
const cp = require('node:child_process');
|
|
|
|
const { evaluateLint, LINT_REASON } = require(path.join(__dirname, '..', 'scripts', 'changeset', 'lint.cjs'));
|
|
|
|
const ROOT = path.join(__dirname, '..');
|
|
const LINT_SCRIPT = path.join(ROOT, 'scripts', 'changeset', 'lint.cjs');
|
|
const { cleanup } = require('./helpers.cjs');
|
|
|
|
/**
|
|
* Build a minimal temp git repo shaped like a PR branch:
|
|
* origin/main = base commit (README.md)
|
|
* pr = PR branch with caller-supplied files committed on top
|
|
*
|
|
* @param {string} tmpDir - pre-created temp directory (mkdtempSync result)
|
|
* @param {Array<{file: string, content: string}>} prFiles - files to create on the PR branch
|
|
* @param {Array<{file: string, content: string}>} [baseFiles] - extra files to create on base commit
|
|
* @returns {string} path to the temp repo (same as tmpDir)
|
|
*/
|
|
function buildTempRepo(tmpDir, prFiles, baseFiles = []) {
|
|
const git = (...args) => cp.execFileSync('git', args, { cwd: tmpDir, encoding: 'utf8' });
|
|
|
|
git('init', '-q', '-b', 'main');
|
|
git('config', 'user.email', 'test@example.com');
|
|
git('config', 'user.name', 'Test');
|
|
|
|
// Base commit: README + any caller-supplied base files
|
|
fs.writeFileSync(path.join(tmpDir, 'README.md'), '# test\n');
|
|
for (const { file, content } of baseFiles) {
|
|
const abs = path.join(tmpDir, file);
|
|
fs.mkdirSync(path.dirname(abs), { recursive: true });
|
|
fs.writeFileSync(abs, content);
|
|
}
|
|
git('add', '-A');
|
|
git('commit', '-q', '-m', 'base');
|
|
|
|
// Fake origin/main so `git diff origin/main...HEAD` works without a real remote
|
|
git('update-ref', 'refs/remotes/origin/main', 'HEAD');
|
|
|
|
// PR branch
|
|
git('checkout', '-q', '-b', 'pr');
|
|
|
|
// Create or delete the PR files
|
|
for (const { file, content } of prFiles) {
|
|
const abs = path.join(tmpDir, file);
|
|
if (content === null) {
|
|
// null content = delete the file (unlinkSync, not rmSync, to stay within
|
|
// the no-raw-rmsync-in-tests rule — we are removing a single file, not a tree)
|
|
try { fs.unlinkSync(abs); } catch { /* already absent — ok */ }
|
|
} else {
|
|
fs.mkdirSync(path.dirname(abs), { recursive: true });
|
|
fs.writeFileSync(abs, content);
|
|
}
|
|
}
|
|
git('add', '-A');
|
|
git('commit', '-q', '-m', 'pr changes');
|
|
|
|
return tmpDir;
|
|
}
|
|
|
|
/**
|
|
* Invoke lint.cjs --json in the given repo directory and return the parsed report.
|
|
* @param {string} repoDir
|
|
* @returns {{ status: number, report: object }}
|
|
*/
|
|
function runLint(repoDir) {
|
|
const result = cp.spawnSync(
|
|
process.execPath,
|
|
[LINT_SCRIPT, '--json'],
|
|
{
|
|
cwd: repoDir,
|
|
env: { ...process.env, GITHUB_BASE_REF: 'main', GITHUB_EVENT_PATH: '' },
|
|
encoding: 'utf8',
|
|
},
|
|
);
|
|
let report = {};
|
|
try { report = JSON.parse(result.stdout); } catch { /* leave as empty object */ }
|
|
return { status: result.status, report };
|
|
}
|
|
|
|
// evaluateLint is a pure function over file lists + label list — no fs, no git.
|
|
// Tests assert on the structured verdict: { ok: bool, reason: LINT_REASON.X }.
|
|
|
|
describe('changeset lint: pure verdict (#2975)', () => {
|
|
test('LINT_REASON enum exposes the documented codes', () => {
|
|
assert.deepEqual(
|
|
Object.keys(LINT_REASON).sort(),
|
|
['OK_FRAGMENT_PRESENT', 'OK_NO_USER_FACING_CHANGES', 'OK_OPT_OUT_LABEL', 'FAIL_MISSING_FRAGMENT', 'FAIL_INVALID_FRAGMENT'].sort(),
|
|
);
|
|
});
|
|
|
|
test('OK_FRAGMENT_PRESENT when the diff includes a new .changeset/*.md', () => {
|
|
const verdict = evaluateLint({
|
|
changedFiles: ['bin/install.js', '.changeset/silly-bears-dance.md'],
|
|
labels: [],
|
|
});
|
|
assert.deepEqual(verdict, { ok: true, reason: LINT_REASON.OK_FRAGMENT_PRESENT });
|
|
});
|
|
|
|
test('FAIL_MISSING_FRAGMENT when user-facing files change without a fragment', () => {
|
|
const verdict = evaluateLint({
|
|
changedFiles: ['bin/install.js', 'tests/foo.test.cjs'],
|
|
labels: [],
|
|
});
|
|
assert.deepEqual(verdict, { ok: false, reason: LINT_REASON.FAIL_MISSING_FRAGMENT });
|
|
});
|
|
|
|
test('OK_OPT_OUT_LABEL when no-changelog label present, even with user-facing changes', () => {
|
|
const verdict = evaluateLint({
|
|
changedFiles: ['bin/install.js'],
|
|
labels: ['no-changelog'],
|
|
});
|
|
assert.deepEqual(verdict, { ok: true, reason: LINT_REASON.OK_OPT_OUT_LABEL });
|
|
});
|
|
|
|
test('OK_NO_USER_FACING_CHANGES when only test/ci/doc files change', () => {
|
|
const verdict = evaluateLint({
|
|
changedFiles: ['tests/foo.test.cjs', '.github/workflows/x.yml', 'docs/x.md'],
|
|
labels: [],
|
|
});
|
|
assert.deepEqual(verdict, { ok: true, reason: LINT_REASON.OK_NO_USER_FACING_CHANGES });
|
|
});
|
|
|
|
test('FAIL_MISSING_FRAGMENT when CHANGELOG.md is edited directly (closes the workflow bypass)', () => {
|
|
const verdict = evaluateLint({
|
|
changedFiles: ['CHANGELOG.md'],
|
|
labels: [],
|
|
});
|
|
assert.deepEqual(verdict, { ok: false, reason: LINT_REASON.FAIL_MISSING_FRAGMENT });
|
|
});
|
|
|
|
test('a fragment alone (no source change) is OK_FRAGMENT_PRESENT — fragment-only PR is allowed', () => {
|
|
const verdict = evaluateLint({
|
|
changedFiles: ['.changeset/silly-bears-dance.md'],
|
|
labels: [],
|
|
});
|
|
assert.deepEqual(verdict, { ok: true, reason: LINT_REASON.OK_FRAGMENT_PRESENT });
|
|
});
|
|
|
|
test('FAIL_INVALID_FRAGMENT when fragmentFailures is non-empty', () => {
|
|
const verdict = evaluateLint({
|
|
changedFiles: ['.changeset/bad.md'],
|
|
labels: [],
|
|
fragmentFailures: [{ file: '.changeset/bad.md', reason: 'invalid_pr', detail: '0' }],
|
|
});
|
|
assert.equal(verdict.ok, false);
|
|
assert.equal(verdict.reason, LINT_REASON.FAIL_INVALID_FRAGMENT);
|
|
assert.deepEqual(verdict.failures, [{ file: '.changeset/bad.md', reason: 'invalid_pr', detail: '0' }]);
|
|
});
|
|
|
|
test('OK_FRAGMENT_PRESENT when fragment is present and fragmentFailures is empty (regression guard)', () => {
|
|
const verdict = evaluateLint({
|
|
changedFiles: ['bin/install.js', '.changeset/good.md'],
|
|
labels: [],
|
|
fragmentFailures: [],
|
|
});
|
|
assert.deepEqual(verdict, { ok: true, reason: LINT_REASON.OK_FRAGMENT_PRESENT });
|
|
});
|
|
|
|
test('FAIL_INVALID_FRAGMENT beats no-changelog opt-out label', () => {
|
|
const verdict = evaluateLint({
|
|
changedFiles: ['.changeset/bad.md'],
|
|
labels: ['no-changelog'],
|
|
fragmentFailures: [{ file: '.changeset/bad.md', reason: 'invalid_pr', detail: '0' }],
|
|
});
|
|
assert.equal(verdict.ok, false);
|
|
assert.equal(verdict.reason, LINT_REASON.FAIL_INVALID_FRAGMENT);
|
|
});
|
|
});
|
|
|
|
// ---------------------------------------------------------------------------
|
|
// End-to-end integration: real main() wiring via temp git repo (#1006)
|
|
// ---------------------------------------------------------------------------
|
|
describe('changeset lint: main() end-to-end wiring (#1006)', () => {
|
|
// Each test allocates its own tmpDir so cases run independently.
|
|
|
|
test('malformed fragment (pr: 0) fails the gate end-to-end', (t) => {
|
|
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-lint-e2e-'));
|
|
t.after(() => cleanup(tmpDir));
|
|
|
|
buildTempRepo(tmpDir, [
|
|
// User-facing source change that requires a fragment
|
|
{ file: 'bin/thing.js', content: '// placeholder\n' },
|
|
// Malformed fragment: pr: 0 is rejected by parseFragment (pr must be > 0)
|
|
{
|
|
file: '.changeset/bad.md',
|
|
content: '---\ntype: Fixed\npr: 0\n---\n**Bad** placeholder. (#1)\n',
|
|
},
|
|
]);
|
|
|
|
const { status, report } = runLint(tmpDir);
|
|
|
|
assert.equal(status, 1, `expected exit 1, got ${status}`);
|
|
assert.equal(
|
|
report.reason,
|
|
LINT_REASON.FAIL_INVALID_FRAGMENT,
|
|
`expected FAIL_INVALID_FRAGMENT, got ${report.reason}`,
|
|
);
|
|
assert.ok(Array.isArray(report.failures), 'failures must be an array');
|
|
const badEntry = report.failures.find((f) => f.file.endsWith('.changeset/bad.md'));
|
|
assert.ok(badEntry, 'failures must contain the bad fragment file');
|
|
assert.equal(badEntry.reason, 'invalid_pr', `expected reason invalid_pr, got ${badEntry.reason}`);
|
|
});
|
|
|
|
test('valid fragment (pr: 1) passes the gate end-to-end', (t) => {
|
|
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-lint-e2e-'));
|
|
t.after(() => cleanup(tmpDir));
|
|
|
|
buildTempRepo(tmpDir, [
|
|
{ file: 'bin/thing.js', content: '// placeholder\n' },
|
|
{
|
|
file: '.changeset/good.md',
|
|
content: '---\ntype: Fixed\npr: 1\n---\n**Good** fix. (#1)\n',
|
|
},
|
|
]);
|
|
|
|
const { status, report } = runLint(tmpDir);
|
|
|
|
assert.equal(status, 0, `expected exit 0, got ${status}`);
|
|
assert.equal(
|
|
report.reason,
|
|
LINT_REASON.OK_FRAGMENT_PRESENT,
|
|
`expected OK_FRAGMENT_PRESENT, got ${report.reason}`,
|
|
);
|
|
});
|
|
|
|
test('deleted fragment is skipped and does not produce fail_invalid_fragment', (t) => {
|
|
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-lint-e2e-'));
|
|
t.after(() => cleanup(tmpDir));
|
|
|
|
// Base commit includes a valid fragment (pr: 5) that the PR will delete.
|
|
buildTempRepo(
|
|
tmpDir,
|
|
[
|
|
// PR deletes the old fragment (null = delete)
|
|
{ file: '.changeset/old.md', content: null },
|
|
// PR adds a new valid fragment
|
|
{
|
|
file: '.changeset/new.md',
|
|
content: '---\ntype: Fixed\npr: 6\n---\n**New** fix. (#6)\n',
|
|
},
|
|
],
|
|
// base files: the old fragment exists before the PR
|
|
[{ file: '.changeset/old.md', content: '---\ntype: Fixed\npr: 5\n---\n**Old** fix. (#5)\n' }],
|
|
);
|
|
|
|
const { status, report } = runLint(tmpDir);
|
|
|
|
assert.equal(status, 0, `expected exit 0, got ${status}`);
|
|
assert.equal(
|
|
report.reason,
|
|
LINT_REASON.OK_FRAGMENT_PRESENT,
|
|
`expected OK_FRAGMENT_PRESENT, got ${report.reason}`,
|
|
);
|
|
// The deleted fragment must NOT appear in failures
|
|
const failures = report.failures ?? [];
|
|
const deletedEntry = failures.find((f) => f.file.endsWith('.changeset/old.md'));
|
|
assert.ok(!deletedEntry, `deleted fragment must not appear in failures, got: ${JSON.stringify(failures)}`);
|
|
});
|
|
});
|