Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Address more flaky collab tests #5788

Merged
merged 2 commits into from
Apr 1, 2024

Conversation

etrepum
Copy link
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

Copy link

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
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
45 checks passed
@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