Improve test coverage for our use of CodeMirror - #1284
Merged
Conversation
Build confidence for CodeMirror upgrades by covering upgrade-sensitive extension code headlessly (EditorState + transactions, no EditorView): - signatureHelp: StateField lifecycle via the public showTooltip facet, including automatic triggering, position mapping and close semantics. - autocompletion: the completion source via constructed CompletionContexts with a fake language server client, including trigger kinds, filtering, ordering, boosts and apply behaviour. Extracted createCompletionSource from the extension to enable this. - diagnostics: LSP mapping including V1 board gating of V2-only API warnings. - editingLine: field and plugin set/clear/timeout behaviour. Named and exported the plugin class, following the dnd-decorations pattern. - names/regexp-util: pure function coverage.
Extract the syntax tree walk from readBlocks into codeBlocks in blocks.ts, separating it from the geometry so it can be tested. grammarInfo moves alongside it. Behaviour-preserving: emission order (innermost first, relied on for cursor attribution) and the positionsForNode call sequence are unchanged, verified pixel-identical in the running app. The grammar contract tests pin the @lezer/python node names and compound statement shapes the highlighting relies on, so a grammar upgrade that renames or restructures them fails tests instead of silently removing the highlighting. Also cover doc-util's body trailer skipping and unnecessary-code overlap checks.
- editingLine: treat cursor offset 0 as a valid position so the typing indicator works at the start of the document. Fix the same falsy-zero pattern for skipBodyTrailers' result in the structure highlighting. - skipBodyTrailers: drop the default min of 0, which is out of range for CodeMirror's 1-based line numbers and threw if used. - autocompletion: don't assume completion items carry LSP data; Pyright always sends it but the protocol doesn't require it. - signatureHelp: handle activeSignature being omitted (LSP allows this as well as null) and skip the tooltip for empty signature lists, rather than throwing while rendering the tooltip. - grammar tests: fail loudly if the document exceeds the initial headless parse budget, as codeBlocks reads syntaxTree(state) and would otherwise silently walk a partial tree.
Cover the CodeMirror extension behaviour that unit tests can't reach, for confidence in CodeMirror upgrades: - diagnostics: first coverage of our lint fork, including the typing indicator it exists for, plus the language server glue end to end. - structure highlighting: block geometry, cursor-driven active block and the full/simple/none setting. - dnd: preview during drag, exact revert on drag out, suppression of the native drop cursor for snippet drags, drop highlighting and single-step undo of a drop. The preview lifecycle helpers synthesise drag events against the app's handlers as Playwright's mouse API can't hold an HTML5 drag mid-flight for assertions; the real browser drag pipeline is still covered by the dragTo-based drop test.
Forked from @codemirror/view 0.19.44 per the commit that introduced it (ef53e3e) and not synced since. Note the behavioural change, matching the provenance header style of our lint fork.
|
Preview build will be at |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivated by de-risking upgrades.
There's one non-trivial change to the code structure highlighting to better share between the test/implementation. Seems OK from manual testing.