Repository navigation
Repair wp/6.4 branch Actions: Retry Puppeteer plugin activation/deactivation on timeout - #83690
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Size Change: 0 B Total Size: 1.63 MB ℹ️ View Unchanged
|
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
e1c6ca2 to
fe5cdc6
Compare
fe5cdc6 to
9e88777
Compare
9e88777 to
c2696e9
Compare
c2696e9 to
1b2d390
Compare
1b2d390 to
c06b3f6
Compare
wp/6.4 branch Actions: retry Puppeteer plugin activation/deactivation on timeoutwp/6.4 branch Actions: Retry Puppeteer plugin activation/deactivation on timeout
c06b3f6 to
6abd11f
Compare
There was a problem hiding this comment.
The logic looks good to me here. A couple of non-blocking notes:
- I'd probably skip adding the changelog entry unless we're intending to publish an update to the package? (I assume not)
- While this adds additional attempts and ups the timeout on each to
60000, the overall test run might still reach the runner's timeout? E.g. on this branch I think it has a cap at100000if I'm reading this right:gutenberg/packages/e2e-tests/config/setup-test-framework.js
Lines 68 to 69 in 40a9d7c
For that second point above, I very much relied on Claude to help me locate the line and I haven't verified that this actual run really uses that timeout value, but it looks legit enough.
The consequence would be that we mightn't ever get the full 3 retries that this PR attempts. Not a reason to block this PR (it can still manage a second attempt), but just noting in case you were expecting the full 3 retries and it doesn't quite make it.
In any case, LGTM (I wouldn't worry about tweaking this PR further, especially given its place in the stack)
15d9714 to
874a225
Compare
Part of #82929.
What?
Fixes Puppeteer end-to-end tests failing on timeouts in
activatePlugin/deactivatePlugin(Puppeteer - 1,- 2and- 3), in two steps:A 60-second wait alone isn't enough; for example,
allowed-patterns.test.js'sbeforeAllhook still timed out callingactivatePlugin( 'gutenberg-test-allowed-patterns' ):Why?
Unlike an
it()-level failure, abeforeAll/afterAllhook failure doesn't get a second chance from Jest's ownjest.retryTimes(2)safety net (packages/e2e-tests/config/setup-test-framework.js) — that only re-runs test bodies, not hooks. So when the one-shot 60-second wait inactivatePlugin/deactivatePlugintimes out under CI resource contention, the entire describe block fails at once with no fallback, which is exactly what's observed (a singlebeforeAlltimeout cascades into everyit()in the file being reported as failed).This PR wraps the click-and-wait sequence in both helpers in a bounded retry loop (2 attempts): on a timeout it re-visits
plugins.phpand tries again, rather than failing immediately. The retry first checks whether the previous attempt actually succeeded server-side and only the render wait timed out (in which case the "Activate"/"Deactivate" link is already gone), so it never tries to click a link that no longer exists.The loop stops at 2 attempts because these helpers usually run in a hook, and Jest stops a hook after 100 seconds on this branch (
jest.setTimeout( PUPPETEER_TIMEOUT || 100000 )insetup-test-framework.js; CI does not setPUPPETEER_TIMEOUT). A first attempt that times out uses about 60 seconds, which leaves roughly 40 seconds for the second, so a third attempt would never start. That second attempt still covers the most common case: the plugin was activated or deactivated server-side and only the page was slow to render, which the reload shows right away.Upstream
There is no trunk change to port. Trunk never changed these helpers; it moved its end-to-end tests from Puppeteer to Playwright instead (the allowed patterns tests in #57399) and later removed
@wordpress/e2e-test-utilsentirely in #72880. Playwright'srequestUtils.activatePlugin()activates plugins through the REST API rather than the plugins screen, so it has no equivalent wait.wp/6.5and newer have no Puppeteer tests that call these helpers.wp/6.3and older have the same helpers, but none of their recent CI runs hit this timeout.Use of AI Tools
This PR was created by Claude Code under my supervision. All code should be treated as AI-produced and not yet reviewed by a human until this PR is marked Ready for Review.