Rework some editor tests to use visual cursor and snapshots #186
Conversation
Codecov Report
@@ Coverage Diff @@
## main #186 +/- ##
==========================================
+ Coverage 65.83% 66.53% +0.70%
==========================================
Files 87 87
Lines 4338 4429 +91
Branches 322 331 +9
==========================================
+ Hits 2856 2947 +91
Misses 1465 1465
Partials 17 17
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
|
||
| const positions = []; | ||
| for (let i = minPos; i <= maxPos; i++) { | ||
| const text = editor.view.state.doc.textBetween(i, i + 1); |
There was a problem hiding this comment.
This probably isn't the most efficient way to do this, but it works!
| expect(getText(content.child(0))).toEqual('der'); | ||
| expect(getText(content.child(1))).toEqual('Content Line 1'); | ||
| expect(getText(content.child(2))).toEqual('Content Line 2'); | ||
| expect(getPrettyHTML(editor)).toMatchInlineSnapshot(` |
There was a problem hiding this comment.
This is an example of a test that reads much more clearly after this change.
| expect(getText(content.child(1))).toEqual('Content Line 1'); | ||
| expect(getText(content.child(2))).toEqual('Content Line 2'); | ||
| expect(getPrettyHTML(editor)).toMatchInlineSnapshot(` | ||
| "<collapsible-block folded=\\"false\\"> |
There was a problem hiding this comment.
@danvk @sehyod wondering if we can fix/improve these ' vs '' in the snapshot (would read better without the \\") and also, would be great to match the setUpEditorWithSelection.
Not sure we can change this: vitest-dev/vitest#856
There was a problem hiding this comment.
Yes, I 100% agree. I have pushed a change to use single quotes in getPrettyHtml
src/editor/test-utils.ts
Outdated
| * @returns The text positions of the one or two "|" characters in the editor. | ||
| */ | ||
| export function setUpEditorWithSelection(editor: Editor, contentHTML: string) { | ||
| editor.chain().setContent(contentHTML).run(); |
There was a problem hiding this comment.
nit: No need to chain to run only one command. You can write editor.commands.setContent(contentHTML);
| `<collapsible-block folded='false'> | ||
| <collapsible-summary></collapsible-summary> | ||
| <collapsible-content> | ||
| <p></p>| |
There was a problem hiding this comment.
This should be <p>|</p> to correspond to a real situation: users can only place the cursor in valid positions, ie in Text nodes.
What happens here is that setContent command will surround | with a paragraph and a draggableBlock, because that is the only way, given the schema, to have text after the paragraph, this could lead to unexpected behaviours in the tests. In this case it does work because when setUpEditorWithSelection deletes the | character, it does remove the blocks surrounding it too because these blocks cannot be empty
This reworks some of the new editor tests from #154 to be more legible by making them more "visual". So rather than:
We have:
The
|here indicates where the cursor should be. With two|characters you get a selection. This is somewhat similar in spirit to how TypeScript tests editor interactions (google "typescript fourslash").This has a few advantages:
On the assertion side, I set up snapshot testing. This tends to make it much clearer what the resulting state of the document after an interaction is than a series of assertions.
Closes #169