* fix(#1660): fail-closed frontmatter set of object-list fields instead of silent no-op cmdFrontmatterSet reported {updated:true} even when spliceFrontmatter returned the content unchanged, which happened whenever the new value's extractFrontmatter projection equalled the original's — notably for object-list fields like must_haves, whose {path,provides} items flatten to scalar strings under the lossy parser. Detect a no-op (newContent === content) for a dict-valued field and surface an error directing the user to edit the file directly, instead of silently accepting a no-op set. Scalars and scalar arrays round-trip faithfully, so idempotent sets of those are intentionally NOT flagged (two precision regression tests lock this). Folded into frontmatter-cli.test.cjs. * chore(#1660): backfill changeset pr ref to 1664 * refactor(#1660): extract noOpObjectListSetError as pure tested helper (Stryker coverage) cmdFrontmatterSet is not in Stryker's property/unit test set, so the inline no-op detection added survivors that dropped the frontmatter module below its 62% mutation threshold. Extract the detection into a pure exported helper noOpObjectListSetError and unit-test every branch directly (changed content, scalar, scalar-array, null, dict no-op). cmdFrontmatterSet now calls the helper. Same pattern as the #1572 spliceFrontmatter coverage fix.
This commit is contained in:
@@ -402,3 +402,42 @@ describe('frontmatter set/merge preserves must_haves object-lists (#1572)', () =
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
// Bug #1660 — frontmatter set of an object-list field (e.g. must_haves) is a silent no-op
|
||||
// when the new value's lossy parse projection equals the original's. Folded into the owning
|
||||
// frontmatter-cli test (no new top-level bug-NNNN file).
|
||||
describe('Bug #1660: frontmatter set of an object-list field fails closed instead of a silent no-op', () => {
|
||||
const PLAN_WITH_MUST_HAVES = [
|
||||
'---', 'phase: 1', 'wave: 1',
|
||||
'must_haves:', ' artifacts:', ' - path: src/foo.ts', ' provides: the foo',
|
||||
'---', '# body', '',
|
||||
].join('\n');
|
||||
|
||||
test('setting must_haves to a value that flattens to the original projection fails closed (no silent no-op)', () => {
|
||||
const file = writeTempFile(PLAN_WITH_MUST_HAVES);
|
||||
const before = fs.readFileSync(file, 'utf-8');
|
||||
// New value {artifacts:["path: src/foo.ts"]} — its extractFrontmatter projection equals
|
||||
// the original's flattened projection, so the set would otherwise be a silent no-op.
|
||||
const result = runGsdTools(['frontmatter', 'set', file, '--field', 'must_haves', '--value', JSON.stringify({ artifacts: ['path: src/foo.ts'] })]);
|
||||
const parsed = JSON.parse(result.output);
|
||||
assert.ok(parsed.error, 'a no-op set of an object-list field must surface an error, not silent {updated:true}');
|
||||
const after = fs.readFileSync(file, 'utf-8');
|
||||
assert.equal(after, before, 'the file must be unchanged when the set is refused (no silent partial write)');
|
||||
});
|
||||
|
||||
test('an idempotent set of a scalar (wave, same value) still reports updated (no false positive)', () => {
|
||||
const file = writeTempFile('---\nphase: 1\nwave: 1\n---\n# body\n');
|
||||
const result = runGsdTools(['frontmatter', 'set', file, '--field', 'wave', '--value', '1']);
|
||||
const parsed = JSON.parse(result.output);
|
||||
assert.equal(parsed.updated, true, 'an idempotent SCALAR set must still report {updated:true} (not fail-closed)');
|
||||
assert.ok(!parsed.error, 'an idempotent scalar set must not produce an error');
|
||||
});
|
||||
|
||||
test('an idempotent set of a scalar array (tags, same value) still reports updated (no false positive)', () => {
|
||||
const file = writeTempFile('---\nphase: 1\ntags: ["a","b"]\n---\n# body\n');
|
||||
const result = runGsdTools(['frontmatter', 'set', file, '--field', 'tags', '--value', '["a","b"]']);
|
||||
const parsed = JSON.parse(result.output);
|
||||
assert.equal(parsed.updated, true, 'an idempotent scalar-ARRAY set must still report {updated:true} (arrays round-trip; not fail-closed)');
|
||||
assert.ok(!parsed.error, 'an idempotent scalar-array set must not produce an error');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -20,6 +20,7 @@ const {
|
||||
extractFrontmatter,
|
||||
reconstructFrontmatter,
|
||||
spliceFrontmatter,
|
||||
noOpObjectListSetError,
|
||||
parseMustHavesBlock,
|
||||
FRONTMATTER_SCHEMAS,
|
||||
} = require('../gsd-core/bin/lib/frontmatter.cjs');
|
||||
@@ -1202,3 +1203,29 @@ describe('reconstructFrontmatter: nested subval plain string', () => {
|
||||
assert.equal(result, 'meta:\n tag: "issue#42"');
|
||||
});
|
||||
});
|
||||
|
||||
// noOpObjectListSetError (#1660) — pure detection helper, unit-tested directly because the
|
||||
// cmdFrontmatterSet path is not in Stryker's property/unit set.
|
||||
describe('noOpObjectListSetError (#1660)', () => {
|
||||
const ORIG = '---\nphase: 1\n---\n';
|
||||
test('changed content (real update) → null', () => {
|
||||
assert.equal(noOpObjectListSetError(ORIG, ORIG + 'x', { must_haves: 1 }), null);
|
||||
});
|
||||
test('scalar value no-op → null (idempotent scalar sets are fine)', () => {
|
||||
for (const v of [1, 'str', true, 0, '']) assert.equal(noOpObjectListSetError(ORIG, ORIG, v), null, `scalar ${JSON.stringify(v)}`);
|
||||
});
|
||||
test('scalar-array value no-op → null (scalar arrays round-trip faithfully)', () => {
|
||||
assert.equal(noOpObjectListSetError(ORIG, ORIG, ['a', 'b']), null);
|
||||
assert.equal(noOpObjectListSetError(ORIG, ORIG, []), null);
|
||||
});
|
||||
test('null value no-op → null', () => {
|
||||
assert.equal(noOpObjectListSetError(ORIG, ORIG, null), null);
|
||||
});
|
||||
test('dict value no-op → error message naming the object-list round-trip limit', () => {
|
||||
const msg = noOpObjectListSetError(ORIG, ORIG, { artifacts: [{ path: 'p' }] });
|
||||
assert.equal(typeof msg, 'string');
|
||||
assert.ok(msg.includes('had no effect'), msg);
|
||||
assert.ok(msg.includes('object-list'), msg);
|
||||
assert.ok(msg.includes('Edit the file directly'), msg);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user