Switch performance tests to Playwright: Take 2 #53768
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit 5014a65.
WunderBart
force-pushed
the
refactor/switch-perf-specs-to-playwright-attempt-2
branch
from
bb062fb
to
6436f14
Compare
|
Size Change: 0 B Total Size: 1.51 MB ℹ️ View Unchanged
|
|
Flaky tests detected in df9eba5. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5892385264
|
|
Running a perf job against the base branch to ensure the legacy canvas is now handled as expected: https://github.com/WordPress/gutenberg/actions/runs/5892879390/job/15985139816 /cc @swissspidy |
WunderBart
force-pushed
the
refactor/switch-perf-specs-to-playwright-attempt-2
branch
from
df9eba5
to
b5f9b21
Compare
WunderBart
force-pushed
the
refactor/switch-perf-specs-to-playwright-attempt-2
branch
from
2a911c6
to
77bbc1f
Compare
WunderBart
requested review from
kevin940726,
ntwb,
nerrad,
ajitbohra,
ryanwelcher,
juanmaguitar,
fabiankaegy and
ndiego
as code owners
|
@kevin940726 @youknowriad, any chance you could take a look? I'm AFK next week, so it would be great to land it today! |
swissspidy
approved these changes
Aug 18, 2023
|
@Mamaduka @kevin940726 not sure what to do with that flaky E2E - I've re-run the job several times already, but it keeps failing and blocking this PR. I can also see it failing on |
…-to-playwright-attempt-2
|
@WunderBart, I tried but failed to debug that one as well :( I wonder if it's related to change #53744 or its combination with other PRs. I started noticing this failure more often after it got merged. |
I think it actually started to be flakier after the PR right above mine: #53736. I don't think the changes from #53744 could have caused it. I've only slightly changed how we check for the legacy canvas presence. I will double check though and see if I can fix that spec. I'd really love to land this one today! 🙌 /cc @jsnajdr, do you think anything from #53736 could make that spec flakier? |
|
@Mamaduka, as they say, the seventh time's a charm, amirite? 😅 All E2Es now passed, but the JS unit ones didn't. Hope to get a pass after a (single) rerun! 🤞 |
WunderBart
deleted the
refactor/switch-perf-specs-to-playwright-attempt-2
branch
|
This switch made the "first block load" metric very unstable. Also it increased the numbers for "typing" and "select block" metrics. |
FBL does look particularly unstable. I'd try waiting for the block to be attached to DOM instead of becoming visible for starters. Playwright waits for the element to be visible by default, which is opposite to what Puppeteer does. The massive post content that we're testing against halts the UI for a bit, which could be making the visibility callback timing so random. I'm going to see if that helps and make a PR. Regarding the Typing and Block Select metrics, it looks like they went back down (and stabilized?) after the recent base branch update.
|
|
I've downloaded raw metrics for a few recent jobs and noticed huge spikes there (10-13 seconds), usually happening for 1-3 samples and then returning to stable-ish ~2.5 seconds. Interestingly, it seems to be happening for the latest ref only, not the base one, which is why we see such big spikes, as the normalization factor gets heavily distorted. Unfortunately, so far, I haven't been able to reproduce this behavior locally + I'm getting much lower values (stable ~1.1s). Any idea what might be causing those delays? cc @dmsnell. Anyway, one immediate solution for this would be calculating the median, not the average, as it would filter out those outliers. Here are the aforementioned raw metrics:
Another thing I've noticed is that the codevitals value seems to be 10x'd for some reason. The first block definitely does not take 29 seconds to load. 🤔 Is this intended, @youknowriad? 
|
I'm definitely not 10xing on purpose but maybe it's the normalization. In other words, the first time we computed the values, the environment might have been slower than the environment we use today (like the CI runner for instance) |
|
@youknowriad |
This was referenced Oct 17, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
A second (and hopefully last) attempt to switch the perf specs to Playwright. 🤞
The original PR was reverted because the tests couldn't handle the environment in which the editor canvas is not iframed (legacy). I've implemented that partially in #53744, and the rest should be in this PR.
edit:
Tested via a custom workflow, running against the base commit (legacy editor) now passes: https://github.com/WordPress/gutenberg/actions/runs/5893776939/job/15986051163
Testing Instructions
See #52022