chore(CI): speedup gh workflows, reduce E2E flake #8842
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Skipped Deployments
|
|
No changes to documentation |
Coverage Report
File CoverageNo changed files found. |
⚡️ Editor Performance ReportUpdated Tue, 11 Mar 2025 23:19:33 GMT
Detailed information🏠 Reference resultThe performance result of
🧪 Experiment resultThe performance result of this branch
📚 Glossary
|
stipsan
force-pushed
the
e2e-flake-(again)
branch
from
7acb2ed to
0fd7c7f
Compare
stipsan
force-pushed
the
e2e-flake-(again)
branch
from
7fdd3b7 to
e67fec7
Compare
stipsan
force-pushed
the
e2e-flake-(again)
branch
from
e67fec7 to
9648b79
Compare
stipsan
force-pushed
the
e2e-flake-(again)
branch
from
9a87d73 to
bceb701
Compare
stipsan
force-pushed
the
e2e-flake-(again)
branch
from
bceb701 to
78db2a8
Compare
stipsan
force-pushed
the
e2e-flake-(again)
branch
from
78db2a8 to
a4d9aa4
Compare
stipsan
force-pushed
the
e2e-flake-(again)
branch
from
a4d9aa4 to
7c4190d
Compare
stipsan
force-pushed
the
e2e-flake-(again)
branch
from
7c4190d to
616611b
Compare
stipsan
force-pushed
the
e2e-flake-(again)
branch
from
616611b to
317d16c
Compare
| await page.getByTestId('action-publish').click() | ||
| expect(await paneFooter.textContent()).toMatch(/published/i) | ||
| await publishButton.click() | ||
| await expect(paneFooter).toContainText(/published/i, {useInnerText: true, timeout: 30_000}) |
There was a problem hiding this comment.
I think this is all great Cody, and really appreciate you taking the time to solve this.
I wonder something in this extended timeouts, have we seen them resolve after 30 seconds?
In the failing tests I'm seeing the published is not showing and it is just blocked in saving forever, delaying the whole test suite, and having to wait 30seconds for an actions to succeed doesn't feel right, if this is the experience in user land we should get it fixed.
There was a problem hiding this comment.
I've seen some tests take over 1 minute and still showing the saving spinner. I don't yet know exactly when it happens, it seems to be happening when the entire e2e suite is running and there's a lot of mutations in the e2e dataset so it's difficult to reproduce locally.
I'll look into this condition after this PR is merged :)
There was a problem hiding this comment.
And I agree, delaying the whole suite up to 30s is bad but it's better than 5s but then have a suite that will retry up to 5 times.
My goal is to find a better and more reliable approach to waiting for the saving spinner to complete and to understand the conditions causing it to take so long.
There was a problem hiding this comment.
To add more on this, I agree will longer timeouts rather than multiple retries. We are adding some telemetry to test the speed of this across studios, maybe it will point us in the right direction.
There was a problem hiding this comment.
Yeah, I've also seen it taking long, but I wonder, do they ever resolve after this long or are they just blocked in this state? In the ones I've found I've never seen it resolve, even after 30secs
There was a problem hiding this comment.
@pedrobonamin yeah, I've seen cases beyond the default 5s, usually around 10. Not in chromium but in firefox. It seems like firefox generally needs longer timeouts, not sure why
There was a problem hiding this comment.
@pedrobonamin for example, if you download the full-html-report--attempt-1 artifact from the build that ran just after this PR merged, you'll see that it waited 5.1 seconds:
There was a problem hiding this comment.
This is a great way to see it, thank you!
Description
run_install: falseas this is the default option ofpnpm/action-setup@v4.uses
cache: pnpmonactions/setup-node@v4, which replaces customcache-node-modulescommands, and leaves it to github to know the best way to cache pnpm.cache: pnpmwe need to runpnpm/action-setupbeforeactions/setup-node.node-version: lts/*so we automatically are on the last stable LTS release of Node.js, so we don't have to manually bump majors every year. A lot of jobs are on Node.js v18, instead of the faster v22.e2e-ct.yml, uses turborepo to cache build output, which is much faster than the customactions/cachesteps, and doesn't require cleanup steps or garbage collection.e2e-ct.ymlnow caches install for chromium and firefox, webkit still needs to run install on every run for it to be successful, although it's still unclear why.e2e:buildcommand now uses turborepo, significantly speeding up the e2e suite as it no longer needs to runsanity buildon every single git push, only on changes that might affect the suite.playwright-ct.config.tsis adjusted to make flake easier to detect, and tests fail faster instead of spending up to 30 mins on the CI before failing. Retries is set to just 1, as our options fortraceandvideo, assume at least 1 retry on errors (for exampleon-first-retry).TestForm.tsx, used bye2e-ct, is updated with the same fixes related to unstableuseRefusage as in the production document providers/What to review
Hopefully the changes makes sense and have helpful inline code comments.
In general the idea of setting retries to just
1, instead of4or6, is that global limits like these makes it hard to track down flake, since the CI might regularly retry flaky tests 4+ times before they pass. It's better to set higher retry limits on specific flaky tests, as it also makes them easier to find and to keep track of.Testing
If existing tests pass we're good 🤞
Notes for release
N/A