Repository navigation
[lexical-y] Chore: Run the fuzz seeds and composition cases whose known failures no longer happen - #9574
Conversation
…wn failures no longer happen, and point open expected failures at their issues
## Description
Some of `@lexical/y`'s known failure lists skipped cases that pass:
- `Fuzz.test.ts` skipped seven `KNOWN_OPEN` seeds. Seeds 959, 1405 and 1856 no longer report a finding, and seeds 199 and 491 report only `loss(container)` (delete wins), which the suite already allows. These five seeds also pass on the first `@lexical/y` commit, so the entries were stale when they landed. Seeds 347 and 1045 still fail and stay listed.
- `KNOWN_WORSE_THAN_V2` skipped seeds 5789 and 7862 in the differential check, which passes on both seeds without them.
- `CompositionHoldBack.test.ts` skipped "code line line, remote insert, a delete, an insert in it, Backspace across lines" (both client id orders) as a known limit where the self-check reports a code highlight type. The case passes, with the self-check on, on main and on the commit that added the skip. `CompositionHoldBackBlockIds.test.ts` had the same skip, which could never apply (no code line, no Backspace across lines).
The three tests under "@lexical/y blocks held by a list item" never built a list item that holds a block: `$insertList` on a block in a box replaces the box with a list, and `NodeStructureExtension`, which every `@lexical/y` editor runs, keeps blocks out of list items, so the setup gives `list[bullet](listitem("abc"))` (or a bare heading) and the tests retyped the list item. They now say what they build and test.
Three expected failures said they were "in Known open issues", a list that is not in the repository (it is in #9400's description). S4-8 and ProdUndoNodeSelection now name their new issues (#9571, #9572), and R16A-2 names #9568, its cause. The S7-22 joins and the update listener expected failures now name their open issues (#9511, #9514).
## Test plan
### Before
```
$ FUZZ_SEED=<n> vitest --run --project unit packages/lexical-y/src/__tests__/unit/Fuzz.test.ts
seed 199: Tests 1 passed (1)
seed 491: Tests 1 passed (1)
seed 959: Tests 1 passed (1)
seed 1405: Tests 1 passed (1)
seed 1856: Tests 1 passed (1)
seed 347: AssertionError: expected [ 'dup' ] to deeply equal []
seed 1045: AssertionError: expected [ 'loss?' ] to deeply equal []
$ FUZZ_DIFF=1 FUZZ_SEED_START=<n> FUZZ_SEEDS=1 vitest --run --project unit packages/lexical-y/src/__tests__/unit/Fuzz.test.ts # entry removed from KNOWN_WORSE_THAN_V2
5789: Tests 1 passed (1)
7862: Tests 1 passed (1)
$ vitest --run --project unit packages/lexical-y/src/__tests__/unit/CompositionHoldBack.test.ts packages/lexical-y/src/__tests__/unit/CompositionHoldBackBlockIds.test.ts
Test Files 2 passed (2)
Tests 1931 passed | 2 skipped (1933)
$ # the skipped case run as a plain test, three times
$ vitest --run --project unit packages/lexical-y/src/__tests__/unit/CompositionHoldBack.test.ts -t "code line line, remote insert, a delete, an insert in it, Backspace across lines"
Tests 2 passed | 1762 skipped (1764)
Tests 2 passed | 1762 skipped (1764)
Tests 2 passed | 1762 skipped (1764)
```
### After
```
$ vitest --run --project unit packages/lexical-y # also matches packages/lexical-yjs
Test Files 165 passed | 2 skipped (167)
Tests 7485 passed | 46 expected fail | 2 skipped (7533)
```
Before this change the same command gave `7483 passed | 46 expected fail | 4 skipped (7533)`.
```
$ pnpm run ci-check
Changeset check passed: 31 pending changeset(s) valid.
Found 0 errors
(exit 0)
```
```
$ vitest --run --project unit packages/lexical-y/src/__tests__/unit/Fuzz.test.ts packages/lexical-y/src/__tests__/unit/CompositionHoldBack.test.ts packages/lexical-y/src/__tests__/unit/CompositionHoldBackBlockIds.test.ts packages/lexical-y/src/__tests__/unit/UpdateErrors.test.ts packages/lexical-y/src/__tests__/unit/regressions/FollowUps.test.ts packages/lexical-y/src/__tests__/unit/regressions/Joins.test.ts packages/lexical-y/src/__tests__/unit/regressions/QuotesAndSlots.test.ts packages/lexical-y/src/__tests__/unit/regressions/Undo.test.ts
Test Files 8 passed (8)
Tests 1973 passed | 11 expected fail (1984)
```
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
meta-cla
Bot
added
the
CLA Signed
lexical
@lexical/a11y
@lexical/clipboard
@lexical/code
@lexical/code-core
@lexical/code-prism
@lexical/code-shiki
@lexical/compiler
@lexical/devtools-core
@lexical/dragon
@lexical/eslint-plugin
@lexical/extension
@lexical/fast-check
@lexical/file
@lexical/hashtag
@lexical/headless
@lexical/history
@lexical/html
@lexical/internal
@lexical/link
@lexical/list
@lexical/mark
@lexical/markdown
@lexical/mdast
@lexical/offset
@lexical/overflow
@lexical/plain-text
@lexical/react
@lexical/rich-text
@lexical/selection
@lexical/table
@lexical/tailwind
@lexical/text
@lexical/utils
@lexical/y
@lexical/yjs
commit: |
…ates to it ## Description #9537 updates the comments of the update listener, R16A-2, S4-8 and ProdUndoNodeSelection expected failures, and names their issues. This change no longer edits them, so the two pull requests do not conflict. Comments only. ## Test plan ### Before ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/Fuzz.test.ts packages/lexical-y/src/__tests__/unit/CompositionHoldBack.test.ts packages/lexical-y/src/__tests__/unit/CompositionHoldBackBlockIds.test.ts packages/lexical-y/src/__tests__/unit/UpdateErrors.test.ts packages/lexical-y/src/__tests__/unit/regressions/FollowUps.test.ts packages/lexical-y/src/__tests__/unit/regressions/Joins.test.ts packages/lexical-y/src/__tests__/unit/regressions/QuotesAndSlots.test.ts packages/lexical-y/src/__tests__/unit/regressions/Undo.test.ts packages/lexical-y/src/__tests__/unit/BlocksAndLines.test.ts Test Files 9 passed (9) Tests 2025 passed | 15 expected fail (2040) ``` ### After ``` $ pnpm exec vitest run --project unit (the same files) Test Files 9 passed (9) Tests 2025 passed | 15 expected fail (2040) ```
zurfyx
changed the title
## Description `KNOWN_OPEN` said seed 1045 loses text typed into a table a peer removes. Minimized, the seed is two operations: one client makes the paragraph inside a box a check list, which replaces the box with the list, while another inserts a table into that box. The table goes with the box (delete wins), which the Yjs 13 layouts lose too. The comment now says that. Comments only. ## Test plan ### Before ``` $ FUZZ_SEED=1045 pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/Fuzz.test.ts [r0] C2 #4 list check paragraph("<0.14>") [r0] A1 #6 table table(tablerow(tablecell(paragraph("<6.0>")) tablecell(paragraph("<6.1>"))) tabl in test_box loss?: <6.0>[line,op6],<6.1>[line,op6],<6.2>[line,op6],<6.3>[line,op6] ``` (the minimized script, from the fuzz utilities' `minimize`) ### After ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/Fuzz.test.ts Test Files 1 passed (1) Tests 16 passed (16) ```
![meta-code-review[bot]](https://avatars.githubusercontent.com/in/5334748?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thanks for auditing the @lexical/y known-failure lists. Most of the removals hold up on this branch, but one doesn't.
Please keep seed 5789 in KNOWN_WORSE_THAN_V2 (packages/lexical-y/src/__tests__/unit/Fuzz.test.ts:106). The file's header (lines 29-31) says to run the relative check with FUZZ_NO_MOVE=1, and in that mode 5789 is still worse than v2:
$ FUZZ_DIFF=1 FUZZ_NO_MOVE=1 FUZZ_SEED_START=5789 FUZZ_SEEDS=1 pnpm exec vitest --run --project unit packages/lexical-y/src/__tests__/unit/Fuzz.test.ts
AssertionError: expected [ 'seed 5789: worse than v2 in loss' ] to deeply equal []
It fails the same way with FUZZ_GC=1. Over seeds 5776-5875 the same command passes on main and fails on this branch, on 5789 only. The test plan's command leaves out FUZZ_NO_MOVE=1. With moves on, genScript picks different op kinds from the same random stream (utils/fuzz.ts:183, :208-210), so that run checks a different script. That mode isn't clean anyway: seeds 5701-6000 already fail on 5859. Dropping 7862 is fine, since it passes with moves on and off, with and without FUZZ_GC=1. The description's KNOWN_WORSE_THAN_V2 bullet needs the same correction.
Nit: seed 347's comment (Fuzz.test.ts:58-60) still says "a list type change of an item that holds a paragraph". That is the shape this PR's new BlocksAndLines.test.ts comment says no @lexical/y editor stores, and the seed's trace has no list type change op. On this branch the duplicated token is <0.14>: two peers turn the same paragraph into a number list and a bullet list in the same round, and the run ends with list[number](listitem("<0.14>")) / list[bullet](listitem("<0.14>")). It would be worth re-describing it from its minimized script, as you did for 1045.
Confirmed on this branch:
- The five
KNOWN_OPENseeds pass alone, with and withoutFUZZ_GC=1; 199 and 491 report only the allowedloss(container). 347 and 1045 still fail, so they rightly stay. - The two code-line
Backspace across linescomposition cases pass in repeated runs with the self-check on, and theCompositionHoldBackBlockIdsskip could never apply. - The renamed
BlocksAndLinessetups buildlist[bullet](listitem("abc")),list[bullet](listitem("abc" br))andheading[h2]("abc"), as the new names say. - #9511 is the right issue for S7-22.
As a tests-only change, this needs no changeset (CONTRIBUTING.md).
| // text shows twice. | ||
| [7862, 'dup: indenting a list item that holds blocks'], | ||
| ]); | ||
| const KNOWN_WORSE_THAN_V2 = new Map<number, string>([]); |
There was a problem hiding this comment.
must_fix: seed 5789 still needs this entry. In the mode lines 29-31 name for the relative check, FUZZ_DIFF=1 FUZZ_NO_MOVE=1 FUZZ_SEED_START=5789 FUZZ_SEEDS=1 fails on this branch with seed 5789: worse than v2 in loss (also with FUZZ_GC=1), and it passes on main only because of this entry. The test plan's command leaves out FUZZ_NO_MOVE=1, and with moves on genScript generates a different script for the same seed. 7862 passes in every mode, so dropping that one is fine.
| const KNOWN_WORSE_THAN_V2 = new Map<number, string>([]); | |
| const KNOWN_WORSE_THAN_V2 = new Map<number, string>([ | |
| // Every token @lexical/y loses was typed into a table a peer removed | |
| // (delete wins); v2 keeps some of them and duplicates another. | |
| [5789, 'loss: delete wins in a removed container'], | |
| ]); |
There was a problem hiding this comment.
Done in ea0e554: seed 5789 is back in KNOWN_WORSE_THAN_V2 with the suggested entry and comment. Confirmed first: FUZZ_DIFF=1 FUZZ_NO_MOVE=1 FUZZ_SEED_START=5789 FUZZ_SEEDS=1 failed with seed 5789: worse than v2 in loss, also with FUZZ_GC=1; with the entry both pass, and so do seeds 5776-5875 in that mode. 7862 stays removed (passes with moves on and off). The description's KNOWN_WORSE_THAN_V2 bullet and the test plan command (now with FUZZ_NO_MOVE=1) are corrected.
| // a list type change of an item that holds a paragraph shows it twice | ||
| // (restructure written as a move; #9386 and v2 duplicate it too). | ||
| [347, 'restructure of a list item holding blocks (dup)'], |
There was a problem hiding this comment.
nit: this still describes "a list type change of an item that holds a paragraph". That is the shape the new comment in BlocksAndLines.test.ts says no @lexical/y editor stores, and seed 347's trace has no list type change (ltype) op. On this branch its duplicated token is <0.14>: two peers turn the same paragraph into a number list and a bullet list in the same round, and the run ends with list[number](listitem("<0.14>")) / list[bullet](listitem("<0.14>")). Worth re-describing from its minimized script, as you did for 1045.
There was a problem hiding this comment.
Done in ea0e554: re-described from the minimized script (three ops: a table op, then list number and list bullet on paragraph("<0.14>") by two peers in round 0). The run ends with list[number](listitem("<0.14>")) / list[bullet](listitem("<0.14>")), as you said; the comment and its label now say two peers turn the same paragraph into a number list and a bullet list at once.
… at its issue ## Description `BatchedWrites.test.ts`'s expected failure "a selection taken between reads of two changes" called itself a known limit, with no document or issue behind it. Run as a plain test, the cursor it takes resolves one character after the editor's caret, a wrong position the binding could avoid, so it is now filed as #9579 and the test names it. Comments and the test name only. ## Test plan ### Before ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/BatchedWrites.test.ts Test Files 1 passed (1) Tests 28 passed | 1 expected fail (29) ``` The expected failure as a plain test (the repro in #9579): ``` AssertionError: expected 3 to be 2 // Object.is equality ``` ### After ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/BatchedWrites.test.ts Test Files 1 passed (1) Tests 28 passed | 1 expected fail (29) ```
…own list, and say what seed 347 duplicates
## Description
Seed 5789 is still worse than v2 in the mode the file's header names for the relative check (`FUZZ_NO_MOVE=1`, with and without `FUZZ_GC=1`): every token `@lexical/y` loses was typed into a table a peer removed (delete wins), and v2 keeps some of them and duplicates another. Its `KNOWN_WORSE_THAN_V2` entry is back. Seed 7862 passes with moves on and off, so it stays removed.
Seed 347's comment described a list type change of an item that holds a paragraph, a shape no `@lexical/y` editor stores. Its minimized script is two peers turning the same paragraph into a number list and a bullet list in the same round, and the run ends with `list[number](listitem("<0.14>")) / list[bullet](listitem("<0.14>"))`; the comment now says that.
## Test plan
```
FUZZ_DIFF=1 FUZZ_NO_MOVE=1 FUZZ_SEED_START=5789 FUZZ_SEEDS=1 pnpm exec vitest --run --project unit packages/lexical-y/src/__tests__/unit/Fuzz.test.ts -t differential
```
### Before
```
AssertionError: expected [ 'seed 5789: worse than v2 in loss' ] to deeply equal []
Tests 1 failed (1)
```
(The same with `FUZZ_GC=1`.)
### After
```
FUZZ_NO_MOVE=1, seed 5789: Tests 1 passed (1)
FUZZ_NO_MOVE=1 FUZZ_GC=1, seed 5789: Tests 1 passed (1)
FUZZ_NO_MOVE=1, seeds 5776-5875: Tests 1 passed (1)
FUZZ_NO_MOVE=1, seed 7862: Tests 1 passed (1)
moves on, seed 7862: Tests 1 passed (1)
default run (Fuzz.test.ts): Tests 16 passed (16)
```
There was a problem hiding this comment.
Thanks for the follow-up: seed 5789 is back in KNOWN_WORSE_THAN_V2, and the rest of the audit holds up on ea0e554a.
Nit: seed 347's comment leaves out the box (packages/lexical-y/src/__tests__/unit/Fuzz.test.ts:58-61). The paragraph both peers convert, paragraph("<0.14>"), is the one $init puts in test_box('o') (packages/lexical-y/src/__tests__/utils/fuzz.ts:1660), and $insertList there replaces the box with each peer's list. The Limits section of packages/lexical-website/docs/collaboration/y.md counts that as a move ("two collaborators moving the same block at once can duplicate it"; "turning a paragraph into a list inside a container" behaves the same way), which is why both lists keep the text. The same two edits on a paragraph at the root converge to one bullet list with the text once, in both client id orders, so as written the comment reads as a general list-conversion duplicate. A suggested wording is inline; my earlier suggestion had the same gap.
Confirmed on this head:
- Seeds 199 and 491 report only the allowed
loss(container), and 959, 1405 and 1856 report nothing, with and withoutFUZZ_GC=1. 347 (dupof<0.14>) and 1045 (loss?of the table's tokens) still fail, and v2 duplicates or loses the same tokens. - With
FUZZ_DIFF=1 FUZZ_NO_MOVE=1, 5789 is still worse than v2 in loss (also withFUZZ_GC=1); 7862 is not worse with moves on or off. - The two code-line
Backspace across linescomposition cases pass with the self-check on, and theCompositionHoldBackBlockIdsskip could never apply. - The renamed
BlocksAndLinessetups buildlist[bullet](listitem("abc")),list[bullet](listitem("abc" br))andheading[h2]("abc"), as the new names say, and #9511 and #9579 match the expected failures that cite them.
Of the removed entries, only seed 199 and the two composition cases fall inside the default unit run; 491, 959, 1405 and 1856 run only in the longer runs the file header describes, and 7862 only under FUZZ_DIFF=1.
As a tests-only change this needs no changeset (CONTRIBUTING.md, "Changesets").
| // two peers turn the same paragraph into a number list and a bullet list | ||
| // in the same round: both lists keep it, so its text shows twice | ||
| // (#9386 and v2 duplicate it too). | ||
| [347, 'one paragraph made a list by two peers at once (dup)'], |
There was a problem hiding this comment.
nit: this leaves out the box. paragraph("<0.14>") is the paragraph $init puts in test_box('o') (utils/fuzz.ts:1660), and $insertList there replaces the box with each peer's list, which the Limits section of packages/lexical-website/docs/collaboration/y.md counts as a move ("two collaborators moving the same block at once can duplicate it"; "turning a paragraph into a list inside a container" behaves the same way). The same two edits on a paragraph at the root converge to one bullet list with the text once, in both client id orders. My earlier wording had the same gap.
| // two peers turn the same paragraph into a number list and a bullet list | |
| // in the same round: both lists keep it, so its text shows twice | |
| // (#9386 and v2 duplicate it too). | |
| [347, 'one paragraph made a list by two peers at once (dup)'], | |
| // two peers turn the paragraph in the box into a number list and a bullet | |
| // list in the same round: each list replaces the box (a move), so both | |
| // keep the paragraph and its text shows twice (#9386 and v2 duplicate it | |
| // too). The same race on a paragraph at the root keeps one list. | |
| [347, 'a box paragraph made a list by two peers at once (dup)'], |
Description
Some of
@lexical/y's known failure lists skipped cases that pass:Fuzz.test.tsskipped sevenKNOWN_OPENseeds. Seeds 959, 1405 and 1856 no longer report a finding, and seeds 199 and 491 report onlyloss(container)(delete wins), which the suite already allows. These five seeds also pass on the first@lexical/ycommit, so the entries were stale when they landed. Seeds 347 and 1045 still fail and stay listed; their comments now say what they lose or duplicate, from their minimized scripts (1045: a table inserted into a box while a peer's list conversion replaces the box, delete wins; 347: two peers turn the same paragraph into a number list and a bullet list in the same round, and both lists keep it).KNOWN_WORSE_THAN_V2skipped seeds 5789 and 7862 in the differential check. 7862 passes without its entry with moves on and off (and withFUZZ_GC=1), so it is removed. 5789 stays: in the mode the file's header names for the relative check (FUZZ_NO_MOVE=1), it is still worse than v2 in loss (every token@lexical/yloses was typed into a table a peer removed, delete wins; v2 keeps some of them and duplicates another).CompositionHoldBack.test.tsskipped "code line line, remote insert, a delete, an insert in it, Backspace across lines" (both client id orders) as a known limit where the self-check reports a code highlight type. The case passes, with the self-check on, on main and on the commit that added the skip.CompositionHoldBackBlockIds.test.tshad the same skip, which could never apply (no code line, no Backspace across lines).The three tests under "@lexical/y blocks held by a list item" never built a list item that holds a block:
$insertListon a block in a box replaces the box with a list, andNodeStructureExtension, which every@lexical/yeditor runs, keeps blocks out of list items, so the setup giveslist[bullet](listitem("abc"))(or a bare heading) and the tests retyped the list item. They now say what they build and test.The S7-22 joins expected failure now names its open issue (#9511), and
BatchedWrites' cursor expected failure, which called itself a known limit with no document behind it, names the issue filed for it (#9579: the cursor resolves one character off). The other expected failures' comments are updated in #9537.Test plan
Before
After
Before this change the same command gave
7483 passed | 46 expected fail | 4 skipped (7533).