Tests: Add end-to-end coverage for scheduled post publishing. - #12668
Tests: Add end-to-end coverage for scheduled post publishing.#12668csmcneill wants to merge 9 commits into
Conversation
Exercises a true future-to-publish transition through WP-Cron: the test schedules a post by the server's clock, waits out the scheduled time without issuing requests (an ordinary request that observes the due event would take the doing_cron lock, and the local Docker environment cannot perform the loopback spawn that releases useful work from it), then drives wp-cron.php explicitly and asserts the transition. Also covers the Scheduled posts list view. See #52895. Co-Authored-By: Craft Agent <agents-noreply@craft.do>
|
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 Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
There was a problem hiding this comment.
Pull request overview
Adds new Playwright end-to-end coverage to verify WordPress scheduled posts both (a) transition from future → publish via a real WP-Cron run and (b) appear in the admin posts list “Scheduled” view.
Changes:
- Introduces an e2e spec that schedules a post based on the server’s HTTP
Dateheader to avoid host/container clock skew. - Adds a deterministic “future-to-publish” transition check by driving
wp-cron.phpdirectly after the scheduled time elapses. - Adds coverage for the admin “Scheduled” status view showing the scheduled post and its scheduled date.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
- Send date_gmt with an explicit UTC designator. - Fail clearly when the server response has no Date header. - Drive wp-cron.php once, before the status poll, so polling cannot spawn cron and re-arm the doing_cron lock against a later drive. Co-Authored-By: Craft Agent <agents-noreply@craft.do>
The spec spans real time by design (a 65-second quiet wait plus up to 60 seconds of polling), which can exceed the default 100-second per-test timeout on the failure path. test.slow() keeps a genuine failure surfacing as the poll's message instead of a test timeout. Co-Authored-By: Craft Agent <agents-noreply@craft.do>
There was a problem hiding this comment.
🟡 Not ready to approve
The “Scheduled view” test can currently pass without confirming the view filter/navigation actually changed, so it may not provide the intended coverage.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Comments suppressed due to low confidence (1)
tests/e2e/specs/scheduled-posts.test.js:123
- After clicking the “Scheduled” status view, the test only asserts that the future post appears and that a “Scheduled” cell exists. However, scheduled posts are included in the default “All” posts list (post status
futureis not internal, so it shows in the admin all list), so these assertions can pass even if the view switch didn’t happen. Add an explicit assertion that navigation/filtering occurred (e.g., URL containspost_status=future).
// Switch to the Scheduled status view.
await page
.locator( '.subsubsub' )
.getByRole( 'link', { name: /^Scheduled/ } )
.click();
- Files reviewed: 1/1 changed files
- Comments generated: 0 new
- Review effort level: Low
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
…ndary. wp_insert_post() coerces a post submitted as `future` into `publish` unless its date is at least MINUTE_IN_SECONDS ahead of the server clock, compared at whole-second resolution. Scheduling exactly a minute out sits on that boundary and only holds when the insert lands inside the same clock second as the Date header it was derived from, which a loaded machine does not guarantee: on one CI attempt the reading was 23:03:37, the insert landed in 23:03:38, and the stored date of 23:04:37 was fifty-nine seconds out and published immediately. Schedule ninety seconds out instead, and derive the wait from the date_gmt the server actually stored, measured against the server clock after the insert, so no pre-insert estimate is carried into it. Co-Authored-By: Craft Agent <agents-noreply@craft.do>
wp-cron.php runs every due event in timestamp order, and an install has a queue of unrelated core events already due ahead of anything a test creates. If one of those is slow, the drive is still working through them when its own lock expires, and it stops there. Sixty seconds after it started, the next request to reach the site reclaims that lock and finishes the queue, so the transition lands at exactly the sixty second mark — which made a sixty second poll budget a dead heat rather than a limit. Raise the budget past WP_CRON_LOCK_TIMEOUT and reissue the drive on a cadence just past it, so the recovery is deliberate rather than dependent on an unrelated request happening along. Do not wait on the drive itself: under PHP-FPM wp-cron.php calls fastcgi_finish_request() and answers immediately, while under mod_php the response waits for every due event, so awaiting it stalls the caller for as long as the slow event runs. A short timeout is enough, and its expiry can be ignored, because wp-cron.php sets ignore_user_abort( true ) and the run continues either way. Co-Authored-By: Craft Agent <agents-noreply@craft.do>
There was a problem hiding this comment.
🟡 Not ready to approve
The status-polling and time-sync requests can still trigger spawn_cron() and take the doing_cron lock, which can interfere with the explicit wp-cron.php drive and make the transition test less deterministic in non-loopback environments.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Comments suppressed due to low confidence (2)
tests/e2e/specs/scheduled-posts.test.js:83
- The REST status reads in
readPostStatus()run through the normal WP bootstrap, which means_wp_cron()can still callspawn_cron()on shutdown and take thedoing_cronlock. That can interfere with the explicit/wp-cron.phpdrives (especially in non-loopback environments) and make the publish transition less deterministic. Pass adoing_wp_cronquery arg on these REST polls to suppressspawn_cron()for the polling requests.
params: { context: 'edit' },
tests/e2e/specs/scheduled-posts.test.js:16
getServerNow()makes a normal front-end request, which runswp_cron()oninit(and_wp_cron()onshutdown) and can therefore callspawn_cron()and take thedoing_cronlock. In environments where loopback requests can’t run, this can unnecessarily block the later explicit/wp-cron.phpdrive and add flakiness. Add adoing_wp_cronquery arg to disable cron spawning for this time-sync request.
This issue also appears on line 83 of the same file.
const response = await requestUtils.request.get(
new URL( '/', requestUtils.baseURL ).toString()
);
- Files reviewed: 1/1 changed files
- Comments generated: 0 new
- Review effort level: Low
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Every request the scheduled posts spec makes now carries `doing_wp_cron` except the drives themselves. spawn_cron() returns early when that argument is present, so only a drive can take the doing_cron lock. This matters more than it looks. An ordinary request that takes the lock holds it for WP_CRON_LOCK_TIMEOUT and then delegates the run to a loopback request, so where loopback cannot be made, a one-second status poll takes the lock every time it expires, delivers nothing, and leaves every later drive to find a lock it cannot claim. The transition then never happens, whatever the budget. Confirmed against a reproduction that starves the first drive with a slow event scheduled ahead of the post: the transition used to land at exactly sixty seconds, from the poll's own spawned run, and now lands with the drive on its cadence. Follow-up to [62838]. Co-Authored-By: Craft Agent <agents-noreply@craft.do>
There was a problem hiding this comment.
🟡 Not ready to approve
The new spec builds URLs with leading slashes (e.g. new URL('/wp-cron.php', baseURL)), which breaks when WP_BASE_URL includes a path (subdirectory installs), so the test can hit the wrong endpoint.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Comments suppressed due to low confidence (2)
tests/e2e/specs/scheduled-posts.test.js:76
new URL( '/wp-cron.php', requestUtils.baseURL )ignores any path inWP_BASE_URLand will hit/wp-cron.phpat the web root instead of (for example)/wp/wp-cron.phpfor subdirectory installs. Use a relative URL against a normalized base URL so cron is driven on the correct site path.
new URL( '/wp-cron.php', requestUtils.baseURL ).toString(),
tests/e2e/specs/scheduled-posts.test.js:16
new URL( '/', requestUtils.baseURL )discards any path component inWP_BASE_URL(e.g.http://host/wp), so this can request the wrong origin/path and read a Date header unrelated to the WordPress install under test. Build the URL from the configuredbaseURL(and normalize a trailing slash) instead of forcing/.
This issue also appears on line 76 of the same file.
const url = new URL( '/', requestUtils.baseURL );
- Files reviewed: 1/1 changed files
- Comments generated: 0 new
- Review effort level: Low
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Adopted in 0b28beb —more details below. Three commits on this branch since the last review: 89034a8 — 5fa66af — 0b28beb — the point quoted above. Every request the spec makes now carries On that last one: the effect is stronger than "less deterministic." A request that takes the The transition never happens and no amount of budget rescues it. Worth saying plainly that the previous revision passed only because loopback does work in the environment I validated it in — the poll's own spawned run was quietly doing the drive's job, which is exactly the dependency the explicit drive exists to remove. I checked the switch rather than assuming it. Against a reproduction that starves the first drive with a slow event scheduled ahead of the post, the transition used to land at exactly +60s, from the poll's spawned run; it now lands at +66s, from the drive on its cadence. Same pass, different mechanism, and the mechanism was the point. Also 21/21 locally on 7.0.2 and 7.1-beta4, plus lint. |
There was a problem hiding this comment.
🟡 Not ready to approve
The new spec relies on less-established RequestUtils surfaces (baseURL and a rest(..., { params }) shape) that can be made more robust/consistent with small in-file adjustments.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Comments suppressed due to low confidence (3)
tests/e2e/specs/scheduled-posts.test.js:78
- Same as above: building an absolute URL from
requestUtils.baseURLties this helper to an internal fixture detail. Using a relative path keeps this working as long as the request context hasbaseURLconfigured.
await requestUtils.request.get(
new URL( '/wp-cron.php', requestUtils.baseURL ).toString(),
{ timeout: 5_000 }
);
tests/e2e/specs/scheduled-posts.test.js:19
requestUtils.baseURLisn’t used anywhere else in the e2e suite and may not be a stable/public surface of theRequestUtilsfixture. Since the underlying Playwright request context is already created with abaseURL(see tests/e2e/config/global-setup.js), you can avoid coupling to abaseURLproperty here by requesting a relative URL directly.
This issue also appears on line 75 of the same file.
const url = new URL( '/', requestUtils.baseURL );
url.searchParams.set( 'doing_wp_cron', '1' );
const response = await requestUtils.request.get( url.toString() );
tests/e2e/specs/scheduled-posts.test.js:97
- Other tests in this repo pass query arguments by including them in the
pathstring forrequestUtils.rest(...), and there are no other examples of aparamsoption. To avoid relying on an undocumented/unsupported option (and to ensurecontext=edit/doing_wp_cron=1are definitely sent), encode them into thepathdirectly.
const post = await requestUtils.rest( {
path: `/wp/v2/posts/${ postId }`,
// `doing_wp_cron` makes spawn_cron() return early, so polling cannot
// take the doing_cron lock. See driveCron() for why that matters.
params: { context: 'edit', doing_wp_cron: '1' },
} );
- Files reviewed: 1/1 changed files
- Comments generated: 0 new
- Review effort level: Low
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Root-relative URLs resolve against the origin alone, so on a subdirectory install — a WP_BASE_URL like http://localhost/wp — the spec's clock reads and cron drives would escape the install and address the server root. The e2e tooling already guards navigations against exactly this: the page fixture resolves root-relative paths against the full baseURL so tests work on subdirectory installs. That handling covers page.goto() alone, not the request context these helpers drive cron through. Resolve every URL the spec builds through one helper that normalizes the base URL's trailing slash and resolves paths relative to it, the same way the page fixture resolves navigations. Co-Authored-By: Craft Agent <agents-noreply@craft.do>
There was a problem hiding this comment.
🟢 Ready to approve
The change set is isolated to a new e2e spec and the cron-lock/skew-handling approach matches WordPress core cron behavior without introducing production-code risk.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0 new
- Review effort level: Low
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Adds end-to-end coverage for scheduled post publishing: a true future-to-publish transition through WP-Cron, plus the Scheduled view of the posts list.
As far as I can tell, no e2e test currently exercises the actual cron transition — existing coverage stops at "the post is scheduled". The transition test schedules a post 60 seconds out by the server's own clock (read from an HTTP Date header, so host/container clock skew doesn't matter), lets the scheduled time pass, then drives wp-cron.php explicitly and asserts the status flip.
Two environment realities shaped the design and are documented in the spec:
Trac ticket: https://core.trac.wordpress.org/ticket/52895
Verification
Use of AI Tools
AI assistance: Yes
Tool(s): Claude (Claude Code via Craft Agents)
Agent(s): Fable 5
Used for: Test authoring and cross-environment verification, with human review; reviewed and submitted by the author.
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.