test(cli): pin toCanonical failure modes for malformed frontmatter#81
Conversation
The single existing test asserted toCanonical returns null for "no frontmatter", codifying a silent null without distinguishing failure modes. A caller reading that test could assume every null means "no frontmatter", when other malformed inputs behave differently. Add three companion tests pinning the behaviour observed today (Option A; the discriminated-union refactor is left for a separate issue, toCanonical's signature is unchanged): - malformed YAML with bad indentation: throws YAMLParseError - frontmatter using tabs for indentation: throws YAMLParseError - unbalanced delimiters (opening --- with no closing ---): returns null toCanonical does not wrap parseYaml in a try/catch, so a malformed body surfaces the parser's throw rather than a null. The two throwing shapes both surface as YAMLParseError (distinct parser causes, same error class), and the unbalanced-delimiters shape collapses with the no-frontmatter case into null because the frontmatter regex requires a closing delimiter. That collapse is pinned explicitly and is worth tightening later via a discriminated return. A future refactor that swallows the parser throw and returns null on malformed YAML now breaks a test instead of passing silently. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 29 minutes and 48 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
1 participant
Summary
packages/cli/tests/fmt.test.tshad a single test assertingtoCanonicalreturnsnullfor "no frontmatter", which codifies a silent null without distinguishing failure modes. A caller reading it could treat everynullas "no frontmatter" when other malformed inputs behave differently.This PR takes Option A: add companion tests pinning the behaviour observed today.
toCanonical's signature is left unchanged (the discriminated-union refactor is Option B, a separate issue).Observed behaviour (determined empirically, not assumed)
I ran each input through the real
toCanonical(via a throwaway probe importing the function the same way the test does) and read the implementation, which callsparseYamlwith no surroundingtry/catch:key: valuethenbad: indent)YAMLParseError: Nested mappings are not allowed in compact mappings---, no closing---)nullYAMLParseError: Tabs are not allowed as indentationSo there are two distinct observable outcomes across the three shapes:
YAMLParseError(distinct underlying causes, same error class). The function does not swallow the throw, so these are clearly distinct from the null path.null, because the frontmatter regex (/^---\r?\n([\s\S]*?)\r?\n---/) requires a closing delimiter and simply does not match.Tests added
Three companion tests in
fmt.test.ts:throws (not returns null) on malformed YAML frontmatter with bad indentation→toThrow(YAMLParseError)throws (not returns null) on frontmatter using tabs for indentation→toThrow(YAMLParseError)returns null on unbalanced delimiters, identical to missing frontmatter today→toBeNull()Each pins the distinct outcome, so a future refactor that collapses a throw into a silent null breaks a test instead of passing.
Behaviour worth tightening later
Two collapses are pinned explicitly and noted as candidates for the Option B discriminated-union return (
{ ok: false; reason: "no-frontmatter" | "malformed-yaml" | "unbalanced-delimiters" }):null. A caller cannot tell "you forgot the closing---" from "there is no frontmatter".YAMLParseErrorclass (different messages). They are distinguishable by message but not by type.Both would be resolved by the discriminated-union signature; that is a separate refactor and out of scope here.
Gates (all pass, node v22.22.2)
npm run build:coreand fullnpm run build: successnpm test: 476 passed, 0 failed (473 + 3 new)npm run typecheck: cleannpm run lint: no new findings. fmt.test.ts carries one pre-existingnoUnusedImportswarning on its line 1node:fsimport, unrelated to this change and left untouched; the repo lint baseline is unchanged (5 errors / 22 warnings).Closes #69
Summary by cubic
Add tests in
packages/cli/tests/fmt.test.tsto pintoCanonicalfailure modes for malformed frontmatter. Tests assertYAMLParseErrorfor malformed YAML (bad indent or tabs) andnullfor unbalanced delimiters, so future changes don’t collapse errors into null.Written for commit 53aeb5f. Summary will update on new commits. Review in cubic