test: regression suite for #437/#440 markdown editor round-trip (stacked on #442)#444
test: regression suite for #437/#440 markdown editor round-trip (stacked on #442)#444wonderwhy-er wants to merge 1 commit into
Conversation
Mounts the new CodeMirror-based mountMarkdownEditor (this PR) with 14 fixtures covering every observed corruption mode in #437 and #440, and asserts that getValue() returns the input unchanged. All 14 cases passed on this branch and ALL 14 fail on origin/main (where the Tiptap+tiptap-markdown editor is still the implementation), so the suite directly attests that this PR fixes the bug it's claiming to fix. Cases: - Pipe table not collapsed to 'AB12' (#437) - '~' not escaped to '\~' (#437, #440) - Adjacent headings keep original spacing (#437) - Wikilink + heading round-trips (#437) - Trailing newline preserved - Combined #437 fixture - YAML frontmatter not parsed as Setext heading (#437 LevionLaurion) - '[x]' task list brackets not escaped (#440) - Underscore identifiers not escaped (#440) - '~/path' not escaped (#440) - Loose lists keep blank lines (#440) - CRLF documented behaviour (CodeMirror normalises to LF; benign — #438 fixes the upstream read path) - README-style file (200+ lines, mixed structure) doesn't collapse — captures a live in-the-wild corruption where a real README went from 209 lines to 22 lines after a single edit_block call - Realistic doc with embedded table doesn't lose adjacent prose Adds jsdom@^24 as a devDependency so the test can mount CodeMirror in node. Auto-discovered by test/run-all-tests.js (matches test*.js).
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
codeant-ai
Bot
added
the
size:XL
|
CodeAnt AI finished reviewing your PR. |
|
CodeAnt AI is running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
size:XL
Sequence DiagramThis PR adds a regression suite that mounts the built markdown editor in a jsdom DOM, feeds it various markdown fixtures, and asserts that getValue returns the original input unchanged, proving the new CodeMirror-based editor avoids the prior silent corruption. sequenceDiagram
participant TestRunner
participant Jsdom
participant MarkdownEditor
TestRunner->>Jsdom: Initialize DOM environment
TestRunner->>MarkdownEditor: Import mount and shell from dist bundle
TestRunner->>MarkdownEditor: Mount editor with markdown input
MarkdownEditor->>MarkdownEditor: Load and display markdown content
TestRunner->>MarkdownEditor: Call getValue for current document
MarkdownEditor-->>TestRunner: Return serialized markdown
TestRunner->>TestRunner: Compare output to input and record test result
Generated by CodeAnt AI |
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| const editor = mountFor(input, view); | ||
| const out = editor.getValue(); | ||
| editor.destroy(); | ||
| return out; |
There was a problem hiding this comment.
Suggestion: editor.destroy() is not guaranteed to run if editor.getValue() throws, which can leave CodeMirror event handlers and DOM state alive and contaminate later cases in the same test file. Wrap value retrieval in a try/finally so cleanup always happens. [resource leak]
Severity Level: Major ⚠️
- ⚠️ Markdown editor roundtrip tests share leaked editor state.
- ⚠️ Later fixtures may fail due to previous editor instance.Steps of Reproduction ✅
1. Run the main test runner `test/run-all-tests.js` (lines 98–119, 139–144) which
discovers `test/test-markdown-editor-roundtrip.js` and spawns `node
./test-markdown-editor-roundtrip.js`.
2. Inside `test/test-markdown-editor-roundtrip.js`, the `runAllTests()` function (lines
234–270) builds the `tests` array (lines 235–249) and, in the loop at lines 253–256, calls
each test via `await t();` within a `try`/`catch`.
3. Each test (e.g. `testPipeTableSurvivesRoundTrip` at lines 70–74) calls `roundTrip()`
(lines 59–64), which creates an editor via `mountFor` (lines 46–57), then immediately
calls `editor.getValue()` and finally `editor.destroy()` at lines 60–62.
4. If any future change in `../dist/ui/file-preview/src/markdown/editor.js` (imported at
lines 42–44) causes `mountMarkdownEditor` or the returned `editor.getValue()` to throw
before `roundTrip` reaches line 62, control flow will exit `roundTrip()` without executing
`editor.destroy()`.
5. The thrown error is caught by the `catch (err)` block in `runAllTests()` at lines
258–262, which increments `failed` and logs the error, but does not and cannot trigger
`editor.destroy()` for that test, leaving the underlying CodeMirror instance and its
DOM/event handlers attached to the JSDOM-backed `#root` element (lines 23–40, 46–49).
6. Subsequent tests in the same process reuse the same global DOM and will mount new
editors on top of potentially leaked state, so a single thrown error during
`editor.getValue()` can contaminate later fixtures, making round-trip regressions harder
to interpret; wrapping lines 60–63 in a `try`/`finally` would ensure cleanup always runs.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** test/test-markdown-editor-roundtrip.js
**Line:** 60:63
**Comment:**
*Resource Leak: `editor.destroy()` is not guaranteed to run if `editor.getValue()` throws, which can leave CodeMirror event handlers and DOM state alive and contaminate later cases in the same test file. Wrap value retrieval in a `try/finally` so cleanup always happens.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
|
CodeAnt AI finished running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
CodeAnt AI is running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
size:XL
Sequence DiagramThis PR adds a jsdom-based regression suite that mounts the CodeMirror-backed markdown editor with various markdown inputs and verifies that the editor returns the same text, guarding against the silent markdown corruption previously seen. sequenceDiagram
participant TestSuite
participant DOM
participant MarkdownEditor
TestSuite->>DOM: Initialize jsdom environment with root element
TestSuite->>MarkdownEditor: Render shell and mount with markdown input
MarkdownEditor->>MarkdownEditor: Initialize editor state from input text
TestSuite->>MarkdownEditor: Call getValue for current content
MarkdownEditor-->>TestSuite: Return markdown text
TestSuite->>TestSuite: Compare output to input and assert round trip
Generated by CodeAnt AI |
|
CodeAnt AI finished running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
1 participant
User description
Stacks on #442. Adds a 14-case regression suite that proves #442 fixes the silent .md corruption reported in #437 and #440.
Differential
origin/main(Tiptap editor in 0.2.39)feat/codemirror-markdown-editor(this PR's base)The same test file produces both results — it is a clean before/after.
What it covers
Each case mounts
mountMarkdownEditorfrom the builtdist/, callsgetValue()immediately, and asserts equality with the input. Cases:>70%-changedthreshold incomputeEditBlocksthen emits oneedit_blockreplacing the whole fileread_filepath that's what actually feeds the editorWhy this is useful for #442
#442 is a large rewrite (-Tiptap +CodeMirror, ~600 lines moved). This suite gives anyone reviewing it a concrete way to verify the corruption is gone, and serves as a regression guard if anyone later considers adding a parse-and-reserialize step back to the editor.
Setup
Adds
jsdom@^24as a devDependency to mount CodeMirror in node. The test is auto-discovered bytest/run-all-tests.js(matchestest*.js).Sibling PR against
mainA companion branch
test/issue-437-markdown-roundtripexists with the Tiptap version of this test — it fails onmaintoday. Either land #442 first and rebase that branch onto the newmain, or merge this stacked PR into #442 and skip the sibling. Either path works.CodeAnt-AI Description
Add regression coverage for markdown editor round-trip stability
What Changed
Impact
✅ Fewer markdown corruption regressions✅ Safer edits for README-style files✅ Clearer protection for table and list round-trips🔄 Retrigger CodeAnt AI Review
Details
💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.