Skip to content

Build/Test Tools: Smoke test the upgraded site over HTTP - #13355

Closed
adimoldovan wants to merge 13 commits into
WordPress:trunkfrom
adimoldovan:smoke-tests-after-upgrade
Closed

Build/Test Tools: Smoke test the upgraded site over HTTP#13355
adimoldovan wants to merge 13 commits into
WordPress:trunkfrom
adimoldovan:smoke-tests-after-upgrade

Conversation

@adimoldovan

@adimoldovan adimoldovan commented Sep 1, 2026

Copy link
Copy Markdown

Trac ticket: https://core.trac.wordpress.org/ticket/66036

Add a light smoke test after the upgrade via cli completed.

Upgrade Tests currently stop at wp core version, which reads a string off the filesystem then report success.

This PR adds one step at the end of the job that moves the site to port 8889, serves it with PHP's built-in server, and makes a few curl requests to validate the site works.

Request Must contain
GET / content="WordPress <version>", matched against wp core version, Hello world!, and </html>
GET /?rest_route=/ a whole JSON body whose name is Upgrade Test
GET /wp-admin/upgrade.php No Update Required and </html>
GET /?p=99999999 HTTP 404
POST /wp-login.php wordpress_logged_in_ in the cookie jar
GET /wp-admin/ id="wpadminbar" and </html>

The step then fails if PHP wrote a fatal error to the server log. That catches a fatal during shutdown, which leaves every response whole.

Each check asserts a marker that a working install must produce, and also </html>.

On a failure the step prints the response body and the server log.

The server starts with -d opcache.jit=disable. Without that it crashes.

The step skips multisite. A network keeps its domain in wp-config.php and in the site and blogs tables, and WordPress strips only :80 and :443 from the host before it matches a network, so a move to another port needs more than an option update.

It sets DISABLE_WP_CRON and WP_HTTP_BLOCK_EXTERNAL. A cron spawn is a request no check asked for, and the update checks on /wp-admin/ leave the runner for api.wordpress.org. WP_Http::block_request() exempts the site's own host, so the site can still talk to itself.

Why not the existing Playwright tests

They need Node, npm ci, a browser download, npm run build, and a Docker environment. This job has none of those. Their global setup also switches the theme to Twenty Twenty-One and deletes every post, so it would wipe the upgraded site before it tested it.

Effect on other branches

Branches 6.6 to 7.1 call reusable-upgrade-testing.yml@trunk, so this step runs on their upgrade jobs too. Branches 6.4 and 6.5 call the older upgrade-testing-run.yml and do not change. upgrade-develop-testing.yml calls this workflow too, and runs on any change to src/**.php.

All the markers exist in the source of every branch from 6.6 to trunk, and every caller runs Ubuntu.

Follow-up

A follow-up should add the same check to install-testing.yml, and moves this block into a composite action that both workflows can call.

Testing Instructions

The workflow runs on any pull request that touches upgrade-testing.yml or reusable-upgrade-testing.yml, so this one exercises it.

To run locally:

  1. Start a database:

    docker run -d --name wp-smoke-db -e MYSQL_ROOT_PASSWORD=root \
      -e MYSQL_DATABASE=test_db -p 13306:3306 mysql:8.4
    
  2. In an empty directory, install an older version and upgrade it:

    wp core download --version=7.0
    wp config create --dbname=test_db --dbuser=root --dbpass=root --dbhost=127.0.0.1:13306
    wp core install --url=http://localhost/ --title="Upgrade Test" --admin_user=admin \
      --admin_password=password --admin_email=me@example.org --skip-email
    wp core update --minor && wp core update-db
    wp core update && wp core update-db
    
  3. Copy the run: block of the new "Post-upgrade smoke check" step into smoke.sh and run it with
    RUNNER_TEMP=/tmp SITE_PORT=8889 WP_ADMIN_USER=admin WP_ADMIN_PASSWORD=password bash -e smoke.sh.
    It prints seven ok lines and exits 0.

  4. Confirm it catches a broken site. Each case below is one file in wp-content/mu-plugins/.
    Write it, run the script again, then delete it. The WP_CLI guard keeps WP-CLI working, so the
    break reaches the step over HTTP rather than stopping the first wp command.

    File content Expected failure
    <?php if ( ! defined( "WP_CLI" ) ) { boom_undefined(); } ::error::/ returned HTTP 500
    <?php add_action( "wp_footer", function () { boom_undefined(); } ); ::error::/ did not contain: </html>
    <?php if ( ! defined( "WP_CLI" ) ) { register_shutdown_function( function () { boom_undefined(); } ); } ::error::the server logged a fatal error
    <?php add_filter( "template_redirect", function () { if ( is_404() ) { status_header( 200 ); } }, 1 ); ::error::a missing post returned 200, expected 404

    The third case leaves every response whole and is caught only by the log check.

  5. Confirm it catches missing content: wp post delete 1 --force, then run the script again. It
    fails with ::error::/ did not contain: Hello world!.

Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: Drafting the workflow step and tracing the opcache.jit crash mentioned above. I ran the local verification described here and reviewed the result.

Upgrade Tests end at `wp core version`, which reads a string off the filesystem.
No step in the job makes an HTTP request. A site that upgrades to the right
version and then fatals on every page still reports green.

Serve the upgraded site with `wp server` and check what a working install must
produce: the version in the generator tag, the default post, a 404 for a missing
one, the REST index, the login form, an authentication cookie from a real login,
and the admin bar on the dashboard. A check for a missing error string breaks as
soon as someone rewords the string.

This skips multisite. The network domain lives in `wp-config.php` and in the
`site` and `blogs` tables. WordPress strips only `:80` and `:443` from the host
before it matches a network, so a move to another port needs more than an option
update.
@adimoldovan adimoldovan self-assigned this Sep 1, 2026
Drop the block above the step. Keep one line on the `if`, which says why the
step skips multisite.
Six of 264 upgrade jobs failed. A request hung for 30 seconds and died:

    Fatal error: Maximum execution time of 30+2 seconds exceeded (terminated)
    in wp-includes/rest-api.php on line 2820

WordPress cron spawns a request back to the same server, and PHP's built-in
server has nothing left to serve it. Four worker processes did not prevent
this, and that mode is experimental: two jobs also died with a segmentation
fault. Turn cron off instead, and drop the workers.

Give every request a 30 second limit, so a hang that slips through fails the
step instead of spending the 20 minute job timeout.

Let the outer shell expand the URL in the wait loop. actionlint reads the
single-quoted form as shellcheck SC2016.
Stop the background server when the step ends, and keep the step's exit code.
Print the response body when a check finds the wrong content. Read the port
from the site URL. Share the admin credentials with the install step. Match
the generator tag without its closing slash. Stop the wait loop when the
server dies.
Every marker sat near the top of the response. A fatal in the second half of a
page leaves an HTTP 200 with half a document, and the check passed. Each HTML
check now also looks for the closing tag, and the REST index must parse as
whole JSON. Neither marker depends on the wording of an error message.

A failed request printed almost nothing. All requests now go through one helper
that keeps the body and the status code. A response that fails prints what the
site returned. The login POST fails the same way as the other checks.

An empty reply came back as a bare curl error. The step now checks whether the
server still runs. A page that kills the PHP process says so and prints the
exit status.

The step reads the server log at the end. A fatal during shutdown leaves every
response whole and the site broken.
The step reported an empty reply as a bare curl error. It now reports two
facts: whether the server process is still up, and whether anything still
listens on the port. The process that holds the port is a grandchild of the
step, so one fact alone cannot tell the two causes apart.

The wait for the server to start had no message. A server that never binds now
says so, instead of leaving a bare exit 124 in the log.

The REST index check read a scratch file that the previous request filled. It
now makes its own request and asserts the parsed value, so line order no longer
decides what it reads.

The 404 page must arrive whole, as every other page must. A missing server log
can no longer replace the exit status the step meant to report.
`wp server` wraps `php -S` in a router and puts the server two processes below
the step. Every path the step asks for resolves without that router: `/` and
`/wp-admin/` reach their `index.php` as directory indexes, and the rest are
files on disk.

Dropping it removes a layer. The server is now the step's own child, so when a
request gets no reply the step reports the status the server exited with,
rather than the status of the process that started it.
PHP's built-in server answered the connection and then closed it with no
reply, on about four jobs in every run. One process accepts the connection
and runs WordPress, so when that process stalls, nothing answers.

nginx now owns the listening socket, and PHP-FPM runs the site behind it.
A worker that dies mid-request becomes an HTTP 502 the step reports, and
the pool starts another. Both programs are on the runner image, and the
step runs them as the runner user with its own config in RUNNER_TEMP.

The step also blocks external HTTP. The update checks on /wp-admin/ left
the runner for api.wordpress.org, which no check asked for.
PHP workers crashed with SIGSEGV on about five percent of cold pools, on
PHP 8.4 and 8.5, while they served the first request. A run of 1200 pools
on the runner image found the cause: the runner sets opcache.jit to
tracing with a 256M buffer. Stock PHP ships the JIT off, so the crash
needs a setup no site runs.

With the JIT off and opcache still on, none of 400 pools crashed. With
the JIT on, 19 of 400 crashed.
A core dump named the crash: every frame sat in opcache.so, and the server
that produced it was php -S. opcache counts the built-in server as a web
SAPI, not as the CLI, so opcache.enable_cli never kept it out and it ran
the JIT the runner turns on. One bug hit both servers.

With the JIT off, php -S crashed on none of 1600 cold starts, against 38
with it on. It serves every path the check asks for, so the pool, the
server block, the socket and the second process all go.
The login page GET repeated the login POST. The POST asks for the same URL
and already fails when that page is broken.

The single post view repeated the front page, which prints post titles. The
front page check now asserts the title, so a site that renders but lost its
content still fails.
@adimoldovan
adimoldovan marked this pull request as ready for review September 3, 2026 12:00
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

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 props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props adrianmoldovanwp, lancewillett.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@adimoldovan
adimoldovan marked this pull request as draft September 3, 2026 12:01
Keep only what the code cannot say: why the step skips multisite, why it runs
last, why the JIT flag is there, and the false passes that each check rules out.
@adimoldovan
adimoldovan marked this pull request as ready for review September 3, 2026 12:36
@adimoldovan adimoldovan changed the title Build/Test Tools: Check the upgraded site over HTTP. Build/Test Tools: Check the upgraded site over HTTP Sep 3, 2026
@adimoldovan adimoldovan changed the title Build/Test Tools: Check the upgraded site over HTTP Build/Test Tools: Smoke test the upgraded site over HTTP Sep 3, 2026
pento pushed a commit that referenced this pull request Sep 3, 2026
Upgrade tests previously stopped after checking the WordPress version on disk. Serve upgraded single-site installations with PHP's built-in server and verify the front end, REST API, database upgrade page, 404 handling, login, dashboard, and server log.

Disable cron and external HTTP requests during the check, and disable the runner's PHP JIT for the temporary server. Multisite remains excluded because moving a network to a temporary port requires coordinated configuration and database changes.

Developed in: #13355

Props adrianmoldovanwp.
See #66036.


git-svn-id: https://develop.svn.wordpress.org/trunk@63447 602fd350-edb4-49c9-b593-d223f7449a82
@lancewillett

Copy link
Copy Markdown
Member

The upgrade workflow smoke check landed in https://core.trac.wordpress.org/changeset/63447. The install workflow follow-up remains as next step.

A follow-up should add the same check to install-testing.yml, and moves this block into a composite action that both workflows can call.

@adimoldovan Do you already have a PR for that?

markjaquith pushed a commit to markjaquith/WordPress that referenced this pull request Sep 3, 2026
Upgrade tests previously stopped after checking the WordPress version on disk. Serve upgraded single-site installations with PHP's built-in server and verify the front end, REST API, database upgrade page, 404 handling, login, dashboard, and server log.

Disable cron and external HTTP requests during the check, and disable the runner's PHP JIT for the temporary server. Multisite remains excluded because moving a network to a temporary port requires coordinated configuration and database changes.

Developed in: WordPress/wordpress-develop#13355

Props adrianmoldovanwp.
See #66036.

Built from https://develop.svn.wordpress.org/trunk@63447


git-svn-id: http://core.svn.wordpress.org/trunk@62628 1a063a9b-81f0-0310-95a4-ce76da25c4cd

@lucatume lucatume left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks correct and I've left a comment that is not a blocker.

Leaving a note here to pick your brain: this script is 100+ lines of bash code to be read inline into a script that is not syntax highlighted. Moving this to a dedicated .sh script used by the workflow would (pro) make it read like a bash file (and allow to add self-tests) but (con) require the actions/checkout step to pull it in (likely from trunk).

version="$(wp core version)"

# A fatal mid-page still returns 200, so every check also asks for the closing tag.
check '/' "content=\"WordPress ${version}\"" 'Hello world!' '</html>'

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The option to show posts on front is implied and currently correct and aligned with defaults. What would make this deterministic is to set it explicitly with wp option update show_on_front posts in the script setup phase.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will also address this in the follow-up PR.

@adimoldovan

Copy link
Copy Markdown
Author

The code looks correct and I've left a comment that is not a blocker.

Leaving a note here to pick your brain: this script is 100+ lines of bash code to be read inline into a script that is not syntax highlighted. Moving this to a dedicated .sh script used by the workflow would (pro) make it read like a bash file (and allow to add self-tests) but (con) require the actions/checkout step to pull it in (likely from trunk).

Valid concern, and my plan was to do that in the follow-up that adds this same check in the install workflow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants