test(git-sync): exhaustive converter coverage + fix 3 round-trip data-loss bugs
Coder↔reviewer design loop (9 rounds, reviewer verdict: exhaustive) produced 92 specs; implemented +123 tests (465 -> 588 passing). The new round-trip coverage exposed three genuine data-loss bugs in the Markdown<->ProseMirror converter, all now FIXED (round-trip is lossless for these): 1. pageBreak was lost on export (no converter case -> rendered to "" and the node vanished). Now emits <div data-type="pageBreak"></div>, which the schema parses back -> round-trips. 2. A block image between blocks left an empty <p> artifact after import-hoisting, producing a phantom blank-gap diff on every sync. markdownToProseMirror now strips content-less paragraphs after generateJSON — with a schema-validity guard that keeps the obligatory single empty paragraph in `content: "block+"` containers (tableCell/tableHeader/blockquote/column/callout/doc), so empty cells/quotes never become an invalid `content: []`. 3. The `code` mark combined with another mark was not byte-stable (emitted nested HTML that the schema's `code` `excludes:"_"` collapsed on import). The converter now emits code-only when `code` co-occurs, matching the editor. New coverage spans media/diagram/details/columns/math/mention attribute round-trips, converter emission branches, git error paths, and engine decision branches. A dedicated test pins the empty-container schema validity (the review catch on the bug-2 fix). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -77,6 +77,79 @@ describe('sanitizeTitle', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('sanitizeTitle — boundary trim and nullish input', () => {
|
||||
// Spec case 1: the length-cap branch (sanitize.ts lines ~79-81) does
|
||||
// `slice(0, MAX_LENGTH).trim()`. The inner `.trim()` after the cap only
|
||||
// does observable work when the 120-char slice boundary lands on whitespace.
|
||||
// Existing length tests use all-'x' input where that trim is a no-op, so the
|
||||
// "trim after cap" sub-branch is otherwise unexercised.
|
||||
//
|
||||
// NOTE(review): The spec's literal example input
|
||||
// 'x'.repeat(118) + ' ' + 'yyyyyyyyyy'
|
||||
// does NOT yield the spec's stated expected output 'x'.repeat(118). Whitespace
|
||||
// runs are collapsed (`/\s+/g` -> single space) BEFORE the length cap, so the
|
||||
// three spaces fold to one: the collapsed string is
|
||||
// 'x'.repeat(118) + ' ' + 'y'.repeat(10) (length 129)
|
||||
// and the char at the slice boundary (index 119) is a 'y', not whitespace.
|
||||
// The actual result is 'x'.repeat(118) + ' y' (length 120) — the inner trim is
|
||||
// a no-op for that exact input. We assert that ACTUAL behavior first (so the
|
||||
// discrepancy is documented and locked down), then use a corrected input that
|
||||
// genuinely lands the cut inside whitespace to exercise the intended sub-branch.
|
||||
it('collapses the spec literal before capping, so its inner trim is a no-op', () => {
|
||||
const input = 'x'.repeat(118) + ' ' + 'y'.repeat(10);
|
||||
const out = sanitizeTitle(input);
|
||||
// Whitespace-run collapse happens before the cap, so the boundary is a 'y'.
|
||||
expect(out).toBe('x'.repeat(118) + ' y');
|
||||
expect(out.length).toBe(120);
|
||||
});
|
||||
|
||||
it('drops a boundary space via the post-cap trim (lines ~79-81)', () => {
|
||||
// To genuinely land the slice(0,120) boundary ON whitespace AFTER collapse,
|
||||
// put a single token boundary at index 119: 119 non-space chars, then a run
|
||||
// of spaces (collapsed to one surviving space at index 119), then more text.
|
||||
// slice(0,120) === 'x'.repeat(119) + ' ', and the post-cap .trim() removes
|
||||
// that trailing space -> 'x'.repeat(119) (length 119, no trailing space).
|
||||
const input = 'x'.repeat(119) + ' '.repeat(5) + 'y'.repeat(10);
|
||||
const out = sanitizeTitle(input);
|
||||
expect(out).toBe('x'.repeat(119));
|
||||
expect(out.length).toBe(119);
|
||||
expect(out.endsWith(' ')).toBe(false);
|
||||
// The inner trim genuinely fired: without it the result would be
|
||||
// 'x'.repeat(119) + ' ' (length 120, trailing space).
|
||||
expect(out).not.toBe('x'.repeat(119) + ' ');
|
||||
});
|
||||
|
||||
// Spec case 2: the function guards input with `(title ?? '')` (line ~74). The
|
||||
// nullish-coalescing branch — title being null/undefined rather than '' — is
|
||||
// not exercised by the existing tests (which pass '' and ' '). This is the
|
||||
// path that protects against a missing page title.
|
||||
it('returns "_" for null input without throwing', () => {
|
||||
let out!: string;
|
||||
expect(() => {
|
||||
out = sanitizeTitle(null as any);
|
||||
}).not.toThrow();
|
||||
expect(out).toBe('_');
|
||||
// No path separators in the produced name.
|
||||
expect(out).not.toContain('/');
|
||||
expect(out).not.toContain('\\');
|
||||
});
|
||||
|
||||
it('returns "_" for undefined input without throwing', () => {
|
||||
let out!: string;
|
||||
expect(() => {
|
||||
out = sanitizeTitle(undefined as any);
|
||||
}).not.toThrow();
|
||||
expect(out).toBe('_');
|
||||
expect(out).not.toContain('/');
|
||||
expect(out).not.toContain('\\');
|
||||
});
|
||||
|
||||
it('null and undefined inputs collapse to the same empty-name guard result', () => {
|
||||
expect(sanitizeTitle(null as any)).toBe(sanitizeTitle(undefined as any));
|
||||
expect(sanitizeTitle(null as any)).toBe(sanitizeTitle(''));
|
||||
});
|
||||
});
|
||||
|
||||
describe('disambiguate', () => {
|
||||
it('appends a stable ~slugId suffix', () => {
|
||||
expect(disambiguate('Notes', 'abc123')).toBe('Notes ~abc123');
|
||||
|
||||
Reference in New Issue
Block a user