E2E Test Utils: Add new fixtures for performance metrics #52993
Conversation
swissspidy
added
[Type] Automated Testing
swissspidy
removed
the
[Status] Blocked
|
Size Change: +79 B (0%) Total Size: 1.62 MB
ℹ️ View Unchanged
|
| @@ -4,6 +4,8 @@ | |||
| "incremental": false, | |||
| "composite": false, | |||
| "module": "CommonJS", | |||
| "moduleResolution": "node16", | |||
There was a problem hiding this comment.
Ensures dynamic imports are left untouched by TypeScript, otherwise it would transform them into require() calls.
Dynamic import can be used to import ESM files in CJS environments.
There was a problem hiding this comment.
Maybe this should be the default now in tsconfig.base.json?
|
Flaky tests detected in ded25ba. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6234819966
|
This reverts commit 068e63a.
There was a problem hiding this comment.
@swissspidy This looks great to me so far! One concern regarding code organization.
There was a problem hiding this comment.
The code looks good to me, but I'm going to leave the final verdict to @kevin940726. Thanks for working on this! 🙌
|
@kevin940726 friendly ping :) |
|
Just a quick note that this change, in combination with recent changes in core trunk in https://core.trac.wordpress.org/changeset/56647 broke my (and likely others) local develoopment environment. When the gutenberg repo is cloned into the Honestly, spending a few hours to understand while suddenly the gutenberg build fails is not the best development experience. I'm not blaming this change, I'd just like to see more focus from projects leads and developers to this kind of problems and how to better coordinate, to avoid extremely time consuming debuggint to contributors. Man contributors work on both core and gutenberg, Whatever their local environment setup is, there should bever be such a bild failure. We should always keep in mind that contribution is open to everyone and any change should be made with more coordination between core and gutenberg, This morning, the gutenberg build suddenly fails with errors like: Screenshot: 
To my understanding, I think this comes from recent changes in the As you may notice, the conflicting package is 4 directories up from the gutemberg directory, that is a core package. Running git bisect brought me to this PR's commit. I do relize this happens in combination with core changes, but testing this PR only with the Gutenberg wp-env seems less than ideal. So far, the only quick workaround for me is to delete (or move) the core |
|
Thanks for flagging this, @afercia. I am able to reproduce this and can confirm your workaround also fixes the issue for me. I'm looking to understand this better so we can decide between a rollback or patch fix. |
|
This PR didn't touch anything with Puppeteer, so not sure how that could happen with these changes here 🤔 |
|
I was able to reproduce at first, but after removing the @afercia could you try these steps and see if you're still able to reproduce? |
|
I believe |
|
I can still reproduce the issue mentioned in #52993 (comment) using the latest trunk versions of both WordPress and Gutenberg. |
|
@desrosj I can still reproduce the issue after deleting both
As I mentioned in my previous comment, when building Gutenberg npm appears to use a
It's a package versions conflict. I think that when npm finds the same package in one of the ancestors directories, it picks up the most recent version. Is that correct? This can easily lead to conflicts when packages in Core and Gutenberg are not aligned. For this reason I mentioned that I would like to see more coordination between the two projects. I find it even strange calling them Worth noting that at the end of June we had a similar case. Conflicting packages between Core and Gutenberg. The only workaround was to manually downgrade the |
|
Thanks all for circling back! I've been looking at this a bit today, but let's move all discussion to the Trac ticket (58671). |
|
One last connection to make in case someone stumbles upon this in the future. The offending update was identified and fixed in Core changeset 56944. I've created Core-59632 to explore adding a workflow to hopefully catch future instances where issues like this occur. |
What?
Adds new fixtures to
@wordpress/e2e-test-utils-playwrightto make it easier to get various performance metrics, either fromServer-Timing,PerformanceObserver, or Lighthouse.Combines some existing code in the repo with some of the utils from https://github.com/swissspidy/wp-i18n-benchmarks
Blockers
get-portv6+ is ESM-only, so right now this uses an older version.*
lightouseis ESM-only, though it has a CJS file that could be imported. I had trouble getting it to work properly either with a dynamic import or fromlighthouse/core/index.cjs. That's why I tried tweaking thetsconfig.jsonfile to get that to work.lighthousev11 requires Node 18+, so we can't upgrade in the foreseeable future since we just upgraded to Node 16 todayWhy?
Reduces duplicated code and makes it easier for anyone writing performance tests with Playwright to measure all sorts of metrics.
In the future, core tests will migrate to Playwright and want to use these new utils to collect all the metrics.
How?
New
Metricsfixture class with the following methods:getServerTiming()getTimeToFirstByte()getLargestContentfulPaint()(viaPerformanceObserver)getCumulativeLayoutShift()(viaPerformanceObserver)getLighthouseReport()There is some overlap between
getLighthouseReport()and the others, but since Lighthouse is slower to run and collects a lot of other things as well, it makes sense to have both.Testing Instructions