[lexical-playground] CI: fix flaky MaxLength emoji e2e test #6257

potatowagon · 2024-06-05T10:33:04Z

Description

  appears randomly thus making test flaky, add keypress delays when typing to prevent that

Test plan

add --reporter=html --repeat-each=10 to "test-e2e-collab-firefox": "cross-env E2E_BROWSER=firefox E2E_EDITOR_MODE=rich-text-with-collab playwright test --project="firefox"", in build script

then run npm run start & npm run test-e2e-collab-firefox

Before

After

npm run start & npm run cross-env E2E_BROWSER=firefox E2E_EDITOR_MODE=rich-text-with-collab playwright test --project="firefox" --reporter=html --repeat-each=10


> @lexical/monorepo@0.16.0 test-e2e-collab-firefox
> cross-env E2E_BROWSER=firefox E2E_EDITOR_MODE=rich-text-with-collab playwright test --project="firefox" --reporter=html --repeat-each=10


> @lexical/monorepo@0.16.0 start
> cross-env NODE_ENV=development concurrently "npm:collab" "npm run dev --prefix packages/lexical-playground"

[collab] 
[collab] > @lexical/monorepo@0.16.0 collab
[collab] > cross-env HOST=localhost PORT=1234 npx y-websocket-server
[collab] 
[1] 
[1] > lexical-playground@0.16.0 dev
[1] > vite --host --port 3000
[1] 

Running 10 tests using 4 workers
[4/10] [firefox] › packages/lexical-playground/__tests__/e2e/MaxLength.spec.mjs:77:8 › MaxLength › can restrict emojis on boundaries
[1] 
[1]   VITE v5.2.11  ready in 620 ms
[1] 
[1]   ➜  Local:   http://localhost:3000/
[1]   ➜  Network: http://172.19.82.250:3000/
  Slow test file: [firefox] › packages/lexical-playground/__tests__/e2e/MaxLength.spec.mjs (53.0s)
  Consider splitting slow test files to speed up parallel execution
  10 passed (19.9s)

vercel · 2024-06-05T10:33:07Z

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	Jun 7, 2024 10:03am
lexical-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jun 7, 2024 10:03am

github-actions · 2024-06-05T10:34:47Z

size-limit report 📦

Path	Size
lexical - cjs	28.33 KB (0%)
lexical - esm	28.14 KB (0%)
@lexical/rich-text - cjs	36.75 KB (0%)
@lexical/rich-text - esm	28.09 KB (0%)
@lexical/plain-text - cjs	35.33 KB (0%)
@lexical/plain-text - esm	25.31 KB (0%)
@lexical/react - cjs	38.54 KB (0%)
@lexical/react - esm	29.17 KB (0%)

Sahejkm · 2024-06-05T10:59:56Z

packages/lexical-playground/__tests__/e2e/MaxLength.spec.mjs

@@ -75,7 +79,7 @@ test.describe('MaxLength', () => {
    await pasteFromClipboard(page, {
      'text/plain': 'lorem ipsum dolor sit amet, consectetur adipiscing elit',
    });
-    await page.keyboard.press('Backspace');
+    await pressBackspace(page, 1, STANDARD_KEYPRESS_DELAY_MS);


do we need delay at 3 places ? or just one where the flakyness appears and adds additional   ?

strangely today i wasnt able to repro the failures on my machine

ah ok if i run the whole test suit, able to repro it again

Sahejkm · 2024-06-05T11:00:32Z

packages/lexical-playground/__tests__/e2e/MaxLength.spec.mjs

@@ -88,7 +92,7 @@ test.describe('MaxLength', () => {
      `,
    );

-    await page.keyboard.type('💏');
+    await page.keyboard.type('💏', {delay: STANDARD_KEYPRESS_DELAY_MS});


also is this test failing in CI in past runs for all environments ? or we wanna add delays based on environments where it fails like firefox or iscollab etc

feels abit overkill to guard the delays based on enviroment given at most a total of 300ms (1 third of a second) is added.

the benefit is the test is stable across all enviroments

i just feel our e2e test suite will grow and scale overtime with more tests added, we should try to add whats needed, overusing sleeps is also not great experience, making e2e test suite slow. unless this test is flaky over all environments and needs sleeps

also has this test failed CI in the past ? as priority should be stabilising CI and fix highly flaky tests 1st for improving on DevEx, having past failure references would be good to track in some comment or PR or issue.

Also, if we see the run https://github.com/facebook/lexical/actions/runs/9364683650/job/25778198911?pr=6246 for [lexical-playground] CI: standardise key press delay #6246 it actually shows Heading test is still flaky after the fix. How are we verifying the fix ? As its not consistent and still appears after this change

That's why i prefer fixing the tests causing CI failures 1st to verify the flakyness is actually resolved

thanks for pointing out a still flaky test result. ill investigate more tmr. today i discovered the repeat param which allows me to stress test to make sure the test is no longer flaky.

also, u have a valid point to reduce unnecessary sleeps so that devx and cost is not affected. i plan to experiment how much total delay would be added for x amount of default delay, and see if such a value is an acceptable value interms of devx and cost. and also, the projected value if our test suite grows

for this test, it is consistenly flaky on my machine when i run the suite of firefox collab test. when ran isolated, it is also consistently flaky. after this fix and stress testing it 10 times, it passes consistently on all 10 runs

potatowagon · 2024-06-05T11:01:50Z

followup thoughts:
to scale solving flakiness due to lack of keypress delays

could look into setting the standard kepress delay as the default delay after a set of consecutive keypresses
as long as the total delay added per test on avg does not cross 2 s
seems feasible since it is unlikely a tests reaches 10 typing actions
adding a default delay after a set of consecutive key presses wouldnt add more then 1s per test given a standard delay of 100ms, and worse case of 10 typing actions

would experiment this in a seperate pr

etrepum · 2024-06-05T15:22:29Z

I wonder if there is an approach we could use where we attach an updateListener to the editor under test and wait for the update to be reconciled (with a timeout of 100ms or whatever the constant should be) after a keypress instead of always sleeping 100ms. Of course it wouldn't generalize to all keypresses but it might help remove a lot of sleeps and let us both speed up the tests and make them more reliable.

Sahejkm · 2024-06-06T00:40:22Z

+1 to Bob's point, if we could build a waitFor() kinda API which waits for the necessary step to complete with some Max timeout to Wait, it would be more reliable over sleeps which add constant delays without any underlying reason for the delays , was reading this yesterday https://shilpa-goley.medium.com/how-to-handle-flaky-tests-e738e5d22156

potatowagon · 2024-06-06T00:41:22Z

I wonder if there is an approach we could use where we attach an updateListener to the editor under test and wait for the update to be reconciled (with a timeout of 100ms or whatever the constant should be) after a keypress instead of always sleeping 100ms. Of course it wouldn't generalize to all keypresses but it might help remove a lot of sleeps and let us both speed up the tests and make them more reliable.

thanks @etrepum , that sounds like a good idea! will give it a try

Sahejkm · 2024-06-07T09:07:03Z

packages/lexical-playground/__tests__/e2e/MaxLength.spec.mjs

@@ -75,7 +79,7 @@ test.describe('MaxLength', () => {
    await pasteFromClipboard(page, {
      'text/plain': 'lorem ipsum dolor sit amet, consectetur adipiscing elit',
    });
-    await page.keyboard.press('Backspace');
+    await pressBackspace(page, 1);


default value is 1 right, adding this again gives any additional value :) ?

potatowagon · 2024-06-07T10:42:12Z

tests failing due to flaky test

potatowagon requested review from zurfyx, fantactuka, acywatson, Fetz, ivailop7 and Sahejkm as code owners June 5, 2024 10:33

facebook-github-bot added the CLA Signed label Jun 5, 2024

potatowagon linked an issue Jun 5, 2024 that may be closed by this pull request

Stabilise CI: resolve flaky e2e tests #6241

Closed

Sahejkm reviewed Jun 5, 2024

View reviewed changes

potatowagon added 2 commits June 7, 2024 12:00

fix test

Loading
Loading status checks…

b18f98f

remove other delays

ef69f9b

potatowagon force-pushed the fix-test-emoji branch from d997ce8 to ef69f9b Compare June 7, 2024 06:00

vercel bot deployed to Preview – lexical June 7, 2024 06:01 View deployment

vercel bot deployed to Preview – lexical-playground June 7, 2024 06:01 View deployment

Sahejkm reviewed Jun 7, 2024

View reviewed changes

remove num chars and use default

db82b92

vercel bot deployed to Preview – lexical June 7, 2024 10:03 View deployment

vercel bot deployed to Preview – lexical-playground June 7, 2024 10:03 View deployment

Sahejkm approved these changes Jun 7, 2024

View reviewed changes

potatowagon added this pull request to the merge queue Jun 7, 2024

Merged via the queue into main with commit b4281f7 Jun 7, 2024
38 of 39 checks passed

phazingazrael mentioned this pull request Jul 23, 2024

[Snyk] Upgrade lexical from 0.15.0 to 0.16.1 phazingazrael/Terra-Logger#32

Open

phazingazrael mentioned this pull request Jul 23, 2024

[Snyk] Upgrade @lexical/react from 0.15.0 to 0.16.1 phazingazrael/Terra-Logger#33

Open

potatowagon deleted the fix-test-emoji branch July 26, 2024 05:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[lexical-playground] CI: fix flaky MaxLength emoji e2e test #6257

[lexical-playground] CI: fix flaky MaxLength emoji e2e test #6257

potatowagon commented Jun 5, 2024 •

edited

Loading

vercel bot commented Jun 5, 2024 •

edited

Loading

github-actions bot commented Jun 5, 2024 •

edited

Loading

Sahejkm Jun 5, 2024 •

edited

Loading

potatowagon Jun 7, 2024

potatowagon Jun 7, 2024

Sahejkm Jun 5, 2024

potatowagon Jun 5, 2024 •

edited

Loading

Sahejkm Jun 5, 2024 •

edited

Loading

Sahejkm Jun 5, 2024 •

edited

Loading

potatowagon Jun 5, 2024 •

edited

Loading

potatowagon Jun 5, 2024 •

edited

Loading

potatowagon commented Jun 5, 2024 •

edited

Loading

etrepum commented Jun 5, 2024

Sahejkm commented Jun 6, 2024 •

edited

Loading

potatowagon commented Jun 6, 2024

Sahejkm Jun 7, 2024 •

edited

Loading

potatowagon commented Jun 7, 2024 •

edited

Loading

[lexical-playground] CI: fix flaky MaxLength emoji e2e test #6257

[lexical-playground] CI: fix flaky MaxLength emoji e2e test #6257

Conversation

potatowagon commented Jun 5, 2024 • edited Loading

Description

Test plan

Before

After

vercel bot commented Jun 5, 2024 • edited Loading

github-actions bot commented Jun 5, 2024 • edited Loading

size-limit report 📦

Sahejkm Jun 5, 2024 • edited Loading

Choose a reason for hiding this comment

potatowagon Jun 7, 2024

Choose a reason for hiding this comment

potatowagon Jun 7, 2024

Choose a reason for hiding this comment

Sahejkm Jun 5, 2024

Choose a reason for hiding this comment

potatowagon Jun 5, 2024 • edited Loading

Choose a reason for hiding this comment

Sahejkm Jun 5, 2024 • edited Loading

Choose a reason for hiding this comment

Sahejkm Jun 5, 2024 • edited Loading

Choose a reason for hiding this comment

potatowagon Jun 5, 2024 • edited Loading

Choose a reason for hiding this comment

potatowagon Jun 5, 2024 • edited Loading

Choose a reason for hiding this comment

potatowagon commented Jun 5, 2024 • edited Loading

etrepum commented Jun 5, 2024

Sahejkm commented Jun 6, 2024 • edited Loading

potatowagon commented Jun 6, 2024

Sahejkm Jun 7, 2024 • edited Loading

Choose a reason for hiding this comment

potatowagon commented Jun 7, 2024 • edited Loading

potatowagon commented Jun 5, 2024 •

edited

Loading

vercel bot commented Jun 5, 2024 •

edited

Loading

github-actions bot commented Jun 5, 2024 •

edited

Loading

Sahejkm Jun 5, 2024 •

edited

Loading

potatowagon Jun 5, 2024 •

edited

Loading

Sahejkm Jun 5, 2024 •

edited

Loading

Sahejkm Jun 5, 2024 •

edited

Loading

potatowagon Jun 5, 2024 •

edited

Loading

potatowagon Jun 5, 2024 •

edited

Loading

potatowagon commented Jun 5, 2024 •

edited

Loading

Sahejkm commented Jun 6, 2024 •

edited

Loading

Sahejkm Jun 7, 2024 •

edited

Loading

potatowagon commented Jun 7, 2024 •

edited

Loading