Skip to content

fix: Address more flaky collab tests#5788

Merged
acywatson merged 2 commits intofacebook:mainfrom
etrepum:flaky-collab-tests
Apr 1, 2024
Merged

fix: Address more flaky collab tests#5788
acywatson merged 2 commits intofacebook:mainfrom
etrepum:flaky-collab-tests

Conversation

@etrepum
Copy link
Copy Markdown
Collaborator

@etrepum etrepum commented Mar 30, 2024

General improvements to the reliability of collab tests

  • Fix a few statements where promises were not returned or awaited (particularly in beforeEach)
  • Moved the yjs top-level paragraph conflict logic outside of the selection recovery condition
  • Used more of the playwright locator methods that retry automatically (may not be strictly necessary but it was part of my exploration)
  • Wait for both frames to be connected to collab during initialization
  • Clean up a few instances where await and async functions were used for no reason
  • Use a bit more of the helper functions to clean things up

@vercel
Copy link
Copy Markdown

vercel bot commented Mar 30, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
lexical ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 31, 2024 2:33am
lexical-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 31, 2024 2:33am

Comment on lines +106 to +110
// If there was a collision on the top level paragraph
// we need to re-add a paragraph
if ($getRoot().getChildrenSize() === 0) {
$getRoot().append($createParagraphNode());
}
Copy link
Copy Markdown
Collaborator Author

@etrepum etrepum Mar 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to bang my head against the wall for a very long time to figure out that this would fix the issue. I haven't read enough yjs and syncEvent code to figure out why it's doing the wrong thing (and that it's timing dependent!), but I don't see the harm in making sure that the editor doesn't become completely empty (since this code ran anyway under a more limited set of conditions).

@acywatson acywatson merged commit 5d61ab2 into facebook:main Apr 1, 2024
@etrepum etrepum deleted the flaky-collab-tests branch May 11, 2024 06:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants