Flaky 'useMultiRootEditor' slow editor test

[f1ames]

The should assign properly data property to editor even if it is still mounting (see here) test fails randomly with:

- Expected
+ Received

  Object {
    "content": "",
-   "intro": "Hello World!",
+   "intro": "",
  }

 ❯ tests/useMultiRootEditor.test.tsx:951:57
    949|     await waitFor( () => {
    950|      expect( result.current.editor ).to.be.instanceof( SlowEditor );
    951|      expect( result.current.editor!.data.get() ).to.deep.equal( {
       |                                                         ^
    952|       intro: 'Hello World!',
    953|       content: ''

See on CI.

ckeditor5-react/tests/useMultiRootEditor.test.tsx

Line 903 in 8886ca3

it( 'should assign properly `data` property to editor even if it is still mounting', async () => {

It might be related to using timeouts:

ckeditor5-react/tests/useMultiRootEditor.test.tsx

Lines 938 to 955 in 8886ca3

	await timeout( 100 );
	result.current.setData( {
	intro: 'Hello World!',
	content: ''
	} );
	await timeout( 200 );
	deferInitialization.resolve();
	await waitFor( () => {
	expect( result.current.editor ).to.be.instanceof( SlowEditor );
	expect( result.current.editor!.data.get() ).to.deep.equal( {
	intro: 'Hello World!',
	content: ''
	} );
	} );

What's worth mentioning is that it seems it got more unstable after #534 PR got merged.

[f1ames]

After #543, it seems this is not flaky test in fact, but really a regression from #534. Test is failing randomly still after improvements from #543 so there might be race condition or something similar going on.

[f1ames]

Ok, some initial observations. I'll put my console dump first (you can skip it first and go to conclusions first) which might not be really readable but still (and at least useful for me) and then refer to it.

You can find a diff of the above here - https://www.diffchecker.com/H91iddhW/.

See dumps

Successful run

----- START -----
useMultiRootEditor:return - editorRefs.instance.current?.id undefined
useMultiRootEditor:return - editorRefs.instance.current?.id undefined
useLifeCycleSemaphoreSyncRef:runAfterMount - revision 1728908958656
useMultiRootEditor:_initializeEditor - totalRestartsRef {
  "current": 0,
}
useMultiRootEditor:_initializeEditor:watchdog.setCreator - totalRestartsRef.current +0
useMultiRootEditor:_externalSetData - newData {
  "content": "",
  "intro": "Hello World!",
}
useLifeCycleSemaphoreSyncRef:runAfterMount - revision 1728908958656
----- ASSERT -----
useMultiRootEditor:_externalSetData:semaphore.runAfterMount - newData, houldUpdateEditor.current {
  "content": "",
  "intro": "Hello World!",
} true
useMultiRootEditor:useEffect:semaphore.replace:afterMount
useMultiRootEditor:useInstantEditorEffect - shouldUpdateEditor.current true
useMultiRootEditor:useInstantEditorEffect - data {
  "content": "",
  "intro": "Hello World!",
}
useMultiRootEditor:useInstantEditorEffect - instance.data.get(), instance.id {} eab447c86dc59195b19d8fefbfd95f8e7
useMultiRootEditor:useInstantEditorEffect - editorRefs.instance.current?.data.get(), editorRefs.instance.current?.id {} eab447c86dc59195b19d8fefbfd95f8e7
useMultiRootEditor:useInstantEditorEffect - roots, instance.model.document.getRoots() [
  "intro",
  "content",
  "footer",
] [
  "intro",
  "content",
  "footer",
]
useMultiRootEditor:useInstantEditorEffect - newRoots []
useMultiRootEditor:useInstantEditorEffect - removedRoots [
  "footer",
]
useMultiRootEditor:return - editorRefs.instance.current?.id eab447c86dc59195b19d8fefbfd95f8e7
useMultiRootEditor:return - editorRefs.instance.current?.id eab447c86dc59195b19d8fefbfd95f8e7
useMultiRootEditor:useInstantEditorEffect:_updateEditorData - dataToUpdate {
  "content": "",
  "intro": "Hello World!",
}
useMultiRootEditor:onDetachRoot - rootName footer
useMultiRootEditor:onChangeData - newData {}
useMultiRootEditor:useInstantEditorEffect - shouldUpdateEditor.current false
useMultiRootEditor:useInstantEditorEffect - data {
  "content": "",
  "intro": "Hello World!",
}
useMultiRootEditor:useInstantEditorEffect - instance.data.get(), instance.id {
  "content": "",
  "intro": "Hello World!",
} eab447c86dc59195b19d8fefbfd95f8e7
useMultiRootEditor:useInstantEditorEffect - editorRefs.instance.current?.data.get(), editorRefs.instance.current?.id {
  "content": "",
  "intro": "Hello World!",
} eab447c86dc59195b19d8fefbfd95f8e7
useMultiRootEditor:useInstantEditorEffect - roots, instance.model.document.getRoots() [
  "intro",
  "content",
] [
  "intro",
  "content",
]
useMultiRootEditor:return - editorRefs.instance.current?.id eab447c86dc59195b19d8fefbfd95f8e7
useMultiRootEditor:return - editorRefs.instance.current?.id eab447c86dc59195b19d8fefbfd95f8e7
----- ASSERT -----
{
  "content": "",
  "intro": "Hello World!",
} eab447c86dc59195b19d8fefbfd95f8e7

Failing run

----- START -----
useMultiRootEditor:return - editorRefs.instance.current?.id undefined
useMultiRootEditor:return - editorRefs.instance.current?.id undefined
useLifeCycleSemaphoreSyncRef:runAfterMount - revision 1728909075935
useMultiRootEditor:return - editorRefs.instance.current?.id undefined
useMultiRootEditor:return - editorRefs.instance.current?.id undefined
useMultiRootEditor:_initializeEditor - totalRestartsRef {
  "current": 0,
}
useMultiRootEditor:_initializeEditor:watchdog.setCreator - totalRestartsRef.current +0
useMultiRootEditor:_externalSetData - newData {
  "content": "",
  "intro": "Hello World!",
}
useLifeCycleSemaphoreSyncRef:runAfterMount - revision 1728909075935
----- ASSERT -----
useMultiRootEditor:useInstantEditorEffect - shouldUpdateEditor.current true
useMultiRootEditor:useInstantEditorEffect - data {}
useMultiRootEditor:useInstantEditorEffect - instance.data.get(), instance.id {} ee9e8e2bde0ca16fea929a15e86555d27
useMultiRootEditor:useInstantEditorEffect - editorRefs.instance.current?.data.get(), editorRefs.instance.current?.id {} ee9e8e2bde0ca16fea929a15e86555d27
useMultiRootEditor:useInstantEditorEffect - roots, instance.model.document.getRoots() [
  "intro",
  "content",
  "footer",
] [
  "intro",
  "content",
  "footer",
]
useMultiRootEditor:useInstantEditorEffect - newRoots []
useMultiRootEditor:useInstantEditorEffect - removedRoots [
  "intro",
  "content",
  "footer",
]
useMultiRootEditor:_externalSetData:semaphore.runAfterMount - newData, houldUpdateEditor.current {
  "content": "",
  "intro": "Hello World!",
} false
useMultiRootEditor:useEffect:semaphore.replace:afterMount
useMultiRootEditor:useInstantEditorEffect - shouldUpdateEditor.current true
useMultiRootEditor:useInstantEditorEffect - data {
  "content": "",
  "intro": "Hello World!",
}
useMultiRootEditor:useInstantEditorEffect - instance.data.get(), instance.id {} ee9e8e2bde0ca16fea929a15e86555d27
useMultiRootEditor:useInstantEditorEffect - editorRefs.instance.current?.data.get(), editorRefs.instance.current?.id {} ee9e8e2bde0ca16fea929a15e86555d27
useMultiRootEditor:useInstantEditorEffect - roots, instance.model.document.getRoots() [
  "intro",
  "content",
  "footer",
] [
  "intro",
  "content",
  "footer",
]
useMultiRootEditor:useInstantEditorEffect - newRoots []
useMultiRootEditor:useInstantEditorEffect - removedRoots [
  "footer",
]
useMultiRootEditor:return - editorRefs.instance.current?.id ee9e8e2bde0ca16fea929a15e86555d27
useMultiRootEditor:return - editorRefs.instance.current?.id ee9e8e2bde0ca16fea929a15e86555d27
useMultiRootEditor:useInstantEditorEffect:_updateEditorData - dataToUpdate {
  "content": "",
  "intro": "Hello World!",
}
useMultiRootEditor:onDetachRoot - rootName footer
useMultiRootEditor:onChangeData - newData {}
useMultiRootEditor:useInstantEditorEffect - shouldUpdateEditor.current false
useMultiRootEditor:useInstantEditorEffect - data {
  "content": "",
  "intro": "Hello World!",
}
useMultiRootEditor:useInstantEditorEffect - instance.data.get(), instance.id {
  "content": "",
  "intro": "Hello World!",
} ee9e8e2bde0ca16fea929a15e86555d27
useMultiRootEditor:useInstantEditorEffect - editorRefs.instance.current?.data.get(), editorRefs.instance.current?.id {
  "content": "",
  "intro": "Hello World!",
} ee9e8e2bde0ca16fea929a15e86555d27
useMultiRootEditor:useInstantEditorEffect - roots, instance.model.document.getRoots() [
  "intro",
  "content",
] [
  "intro",
  "content",
]
useMultiRootEditor:return - editorRefs.instance.current?.id ee9e8e2bde0ca16fea929a15e86555d27
useMultiRootEditor:return - editorRefs.instance.current?.id ee9e8e2bde0ca16fea929a15e86555d27
useMultiRootEditor:_initializeEditor:watchdog.on_error [CKEditorError: writer-detachroot-no-root
Read more: https://ckeditor.com/docs/ckeditor5/latest/support/error-codes.html#error-writer-detachroot-no-root]
useMultiRootEditor:_initializeEditor:watchdog.setCreator - totalRestartsRef.current 1
useMultiRootEditor:return - editorRefs.instance.current?.id ead7e49e7b97f24d64f968654f434db89
----- ASSERT -----
{
  "content": "",
  "intro": "",
} ead7e49e7b97f24d64f968654f434db89
----- ASSERT -----
{
  "content": "",
  "intro": "",
} ead7e49e7b97f24d64f968654f434db89

Some conclusions

1. Fix: Update data of roots with modified content only #535 PR changed the content of those tests slightly by adding additional footer root (see https://github.com/ckeditor/ckeditor5-react/pull/535/files#diff-5b81880d0441ee0751e0a44635f33052c1176a93a78f4ef4914b85e42adc265fR23).
2. The test fails because of watchdog restarting the editor after setting data. Watchdog restart is caused by the editor crash because of #error-writer-detachroot-no-root when footer root is about to be detached. And so assert gets data from new editor instance - see diff screenshot below (important - I would assume after editor is restarted by watchdog it should have the same contents than previous instance, either it's broken too or race condition here so data is not yet set on new instance).

3. The failing run starts by some empty set data (see diff screenshot below) which detaches all the roots, and so next footer detach fails (here I'm still looking into why it happens, looks like trying to detach the same root multiple times is also some outdated state issue).

4. When I run this test in separation (both watchdog [true, false]) it fails like 30%. When I run the test only with watchdog (by modifying this for) it seems to fail always. Running it with "npx vitest run -t 'should assign properly data property to editor even if it is still mounting'".
5. It is not a regression from Fix: Update data of roots with modified content only #535. This issue can be observed before those changes by adding additional root to these tests. I created a branch from v9.3.0 and modified the test and it fails the same way. See the branch - https://github.com/ckeditor/ckeditor5-react/tree/t/542-test (you should run only this test, others will fail since I did not adjusted assertions there - "npx vitest run -t 'should assign properly data property to editor even if it is still mounting'").
6. This tests mocks console.warn and console.error to empty functions, which makes noticing errors (like the one from this issue) really time consuming.

[f1ames]

@Mati365 I guess it was closed incidentally, right? We concluded that #543 does not fix it.

[Mati365]

Yep, sorry.