The smoke test could not fail, and other people's eyes
Four fixes, two of them serious, three of them found by the session porting
this into subscriptions.
1. THE SMOKE TEST COULD NOT FAIL. `wp eval-file` runs the file through eval()
INSIDE A METHOD, so the top level is function scope. `$wc_edm_failures = 0`
at the top and `global $wc_edm_failures` in the helper were two different
variables: the helpers incremented the real global, the verdict read the
local, and the local was always zero.
Measured rather than reasoned. A deliberately false assertion added to the
unmodified file:
FAIL DELIBERATE FAILURE — this run must go red
All assertions passed on WordPress 7.1 with WooCommerce 11.1.0.
EXIT=0
and with the fix, the same forced failure:
FAIL DELIBERATE FAILURE — this run must go red
1 assertion(s) failed on WordPress 7.1.
EXIT=1
The FAIL lines always printed, so failures found by reading the output were
real finds. What was broken is the CI gate: a regression on either forge
would have gone green. Four lines at the top now prove the counter reaches
the verdict before anything relies on it — this is the one file whose whole
job is to be able to go red, so it checks that it can.
2. `git ls-files` QUOTES NON-ASCII PATHS. "TESTMILJØ.md" comes back as the
literal string "TESTMILJ\303\230.md", which matches no pattern in
.distignore, so the file is kept — an internal document walking into the
release zip with nothing said. Fixed in ci/dist-files.sh and in
ci/static.sh's trailing-?> check, which had the same bug and would have
silently skipped the file it exists to check. Verified by adding such a
filename and watching it be excluded. `done < <(...)` rather than a pipe, or
the loop runs in a subshell and the failure counter stops incrementing.
3. THE FEED CACHE WAS SIX HOURS, and Kristian hit it: a release published and
his shop shown nothing. The reasoning ("WordPress checks twice a day") was
wrong. WordPress gates its own check by screen in wp_update_plugins() — one
minute on Dashboard → Updates, one hour on Plugins, 2–12 hours in the
background — so WordPress is already the rate limiter and a longer cache
only hides releases underneath it. Fifteen minutes now, pinned by an
assertion.
4. The wp_update_plugins() assertion made a real HTTP request, because the
force-check tests had just deleted the transient. It passed only because
this feed repo is public, which made it measure the environment rather than
the code. Re-primed first.
ci_in_container() replaces the inline docker plumbing in ci_php(), taken from
the subscriptions port: the script goes in as a file rather than interpolated
into `sh -c`, and --entrypoint sh matters for images whose entrypoint swallows
its arguments.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
kristianwind
co-authored by
Claude Opus 5
Claude Opus 5
parent
48119f2034
commit
a1f6e54f9d
+8
-1
@@ -18,7 +18,14 @@ while IFS= read -r line; do
|
||||
[ -n "$line" ] && patterns+=("${line%/}")
|
||||
done < .distignore
|
||||
|
||||
git ls-files | while IFS= read -r file; do
|
||||
# -z, not plain `git ls-files`: git QUOTES any path containing a non-ASCII
|
||||
# byte, so "TESTMILJØ.md" comes out as the literal 14-character string
|
||||
# "TESTMILJ\303\230.md". That form matches no pattern in .distignore, so the
|
||||
# file is kept — an internal document walking straight into the release zip
|
||||
# with nothing said. (-z also covers paths with spaces.) Found in a sibling
|
||||
# plugin that has exactly such a filename; this repo has none today, which is
|
||||
# precisely why it would have sat here until someone added one.
|
||||
git ls-files -z | while IFS= read -r -d '' file; do
|
||||
keep=1
|
||||
for p in "${patterns[@]}"; do
|
||||
# A bare name excludes the file or directory wherever it appears, which is
|
||||
|
||||
@@ -46,33 +46,51 @@ ci_plugin_header() {
|
||||
"$(ci_repo_root)/$PLUGIN_MAIN" | head -1 | sed -E 's/[[:space:]]+$//'
|
||||
}
|
||||
|
||||
# Run a shell script against a checkout of this repository, inside a container.
|
||||
#
|
||||
# The repository goes in over a tarball and `docker cp`, never a bind mount: a
|
||||
# Gitea Actions job runs inside a container talking to the HOST's docker
|
||||
# daemon, so a host path here lands on an empty directory nobody created and
|
||||
# the check passes by checking nothing. The symptom reads like a typo in a path.
|
||||
#
|
||||
# Two details are load-bearing. The script is copied in as a FILE rather than
|
||||
# interpolated into `sh -c`, so it can contain $, quotes and newlines without
|
||||
# being mangled — which matters the moment it is more than one command. And
|
||||
# --entrypoint sh is not decoration: some images (composer:2, for one) have an
|
||||
# entrypoint that swallows its arguments.
|
||||
#
|
||||
# Taken from the subscriptions port, which needed it for composer.
|
||||
ci_in_container() { # image script-text
|
||||
local image="$1" script="$2"
|
||||
local dir cid code=0
|
||||
|
||||
dir=$(mktemp -d -t wpci-XXXXXX)
|
||||
( cd "$(ci_repo_root)" && git ls-files -z | tar --null -T - -cf "$dir/src.tar" )
|
||||
printf '%s\n' "$script" > "$dir/run.sh"
|
||||
|
||||
cid=$(docker create --entrypoint sh -w /src "$image" \
|
||||
-c 'mkdir -p /src && tar -xf /tmp/src.tar -C /src && exec sh /tmp/run.sh')
|
||||
docker cp "$dir/src.tar" "$cid:/tmp/src.tar" >/dev/null
|
||||
docker cp "$dir/run.sh" "$cid:/tmp/run.sh" >/dev/null
|
||||
docker start -a "$cid" || code=$?
|
||||
docker rm -f "$cid" >/dev/null
|
||||
rm -rf "$dir"
|
||||
return "$code"
|
||||
}
|
||||
|
||||
# Run one of this repository's PHP scripts.
|
||||
#
|
||||
# Uses the local php when there is one — a laptop, GitHub's hosted runners —
|
||||
# and a container otherwise. The Gitea runner image has no php, and losing a
|
||||
# build to "php: command not found" after everything else passed is a silly way
|
||||
# to spend an afternoon. Measured: it happened on the very first Gitea run.
|
||||
#
|
||||
# The repository goes in over a tarball and `docker cp`, never a bind mount: a
|
||||
# Gitea Actions job runs inside a container talking to the HOST's docker
|
||||
# daemon, so a host path here lands on an empty directory nobody created.
|
||||
ci_php() {
|
||||
if command -v php >/dev/null 2>&1; then
|
||||
php "$@"
|
||||
return
|
||||
fi
|
||||
|
||||
local tarball cid code=0
|
||||
tarball=$(mktemp -t wpci-php-XXXXXX.tar)
|
||||
( cd "$(ci_repo_root)" && git ls-files -z | tar --null -T - -cf "$tarball" )
|
||||
|
||||
cid=$(docker create "php:8.4-cli" sh -c \
|
||||
"mkdir -p /src && tar -xf /tmp/src.tar -C /src && cd /src && exec php $(printf '%q ' "$@")")
|
||||
docker cp "$tarball" "$cid:/tmp/src.tar" >/dev/null
|
||||
docker start -a "$cid" || code=$?
|
||||
docker rm -f "$cid" >/dev/null
|
||||
rm -f "$tarball"
|
||||
return "$code"
|
||||
ci_in_container "php:8.4-cli" "exec php $(printf '%q ' "$@")"
|
||||
}
|
||||
|
||||
ci_step() { printf '\n\033[1m→ %s\033[0m\n' "$*"; }
|
||||
|
||||
+4
-2
@@ -76,11 +76,13 @@ before=$fails
|
||||
if git ls-files -z '*.php' | xargs -0 grep -nP '<\?(?!php|=|xml)' ; then
|
||||
note_fail "Use <?php, not short open tags."
|
||||
fi
|
||||
while read -r f; do
|
||||
# -z: git quotes non-ASCII paths, and the quoted form is not a path that
|
||||
# exists, so this loop would silently skip the file it was meant to check.
|
||||
while IFS= read -r -d '' f; do
|
||||
[ -n "$f" ] || continue
|
||||
[ "$(tail -c 3 "$f" | tr -d '[:space:]')" = "?>" ] \
|
||||
&& { ci_annotate error "$f" 1 "Remove the trailing ?> at the end of the file."; fails=$((fails + 1)); }
|
||||
done <<< "$(git ls-files '*.php')"
|
||||
done < <(git ls-files -z '*.php')
|
||||
[ "$fails" = "$before" ] && echo " clean"
|
||||
|
||||
ci_step "text domain"
|
||||
|
||||
+26
-5
@@ -71,11 +71,14 @@ Three things in that file are load-bearing and none of them look it:
|
||||
WordPress 5.8. Remove the header and a .org plugin that happens to share the
|
||||
slug could be pushed onto a shop running yours.
|
||||
|
||||
The answer is cached for six hours, which is right for the check WordPress runs
|
||||
twice a day and wrong for the **Check again** link — so `forget_on_force_check()`
|
||||
drops the cache on `force-check`. Without it, someone who has just published a
|
||||
release clicks that link, sees nothing, and concludes the whole mechanism is
|
||||
broken.
|
||||
🔴 **Keep the feed cache short — WordPress is already the rate limiter.** It
|
||||
gates its own check by screen: one minute on Dashboard → Updates, one hour on
|
||||
the Plugins screen, 2–12 hours in the background. This filter only runs when
|
||||
that check runs, so any cache longer than that cadence simply hides new
|
||||
releases underneath it. Ours was six hours "because WordPress checks twice a
|
||||
day anyway", and the symptom was a release going out and the shop being shown
|
||||
nothing. It is fifteen minutes now, and `forget_on_force_check()` drops it
|
||||
entirely on the **Check again** link.
|
||||
|
||||
**6. Make the Gitea repository public**, or point `Update URI` at a separate
|
||||
public releases-only repository. Every shop reads that feed without
|
||||
@@ -105,6 +108,24 @@ it cannot.
|
||||
- `ci/workflows-in-sync.sh` — fails when the two workflow roots drift, which is
|
||||
the only way to notice that a workflow stopped running on Gitea.
|
||||
|
||||
## Running these locally: `git add` first
|
||||
|
||||
Every one of these scripts derives "what ships" from `git ls-files`, which
|
||||
reads the **index**. An uncommitted file is therefore invisible to
|
||||
`ci/dist-files.sh`, to the release zip, and to the copy that goes into the test
|
||||
site.
|
||||
|
||||
That is deliberate — it is what catches a file that was written, referenced and
|
||||
never committed, which is a release that is missing a file while every local
|
||||
test passes because the file is sitting right there in the working tree. But it
|
||||
also means running any of this before `git add` tests a release without your
|
||||
newest file, and the failure is silent: the check does not complain, it just
|
||||
measures something else. It cost two rounds here, once on
|
||||
`includes/class-updater.php` and once on `assets/icon.svg`.
|
||||
|
||||
If a script reports something you just fixed as still missing, `git status`
|
||||
before you debug the script.
|
||||
|
||||
## Two guards worth understanding before you move them
|
||||
|
||||
**`.gitea/workflows/` hides `.github/workflows/` completely.** Gitea uses the
|
||||
|
||||
@@ -185,11 +185,21 @@ the real list table and asserts the flag rather than trusting the reasoning.
|
||||
|
||||
### "Check again" really checks again
|
||||
|
||||
The feed is cached for six hours. That is right for the check WordPress runs on
|
||||
its own twice a day, and wrong for a link whose entire promise is that it looks
|
||||
*now* — so the cache is dropped when wp-admin is loaded with `force-check`.
|
||||
Without that, publishing a release and then clicking **Check again** shows
|
||||
nothing for up to six hours, which reads exactly like a broken updater.
|
||||
The feed is cached for fifteen minutes, and the cache is dropped entirely when
|
||||
wp-admin is loaded with `force-check`.
|
||||
|
||||
It was six hours, on the reasoning that WordPress checks twice a day anyway.
|
||||
That reasoning was wrong. WordPress gates its own check **by screen**:
|
||||
|
||||
| Screen | How often WordPress re-checks |
|
||||
| --- | --- |
|
||||
| Dashboard → Updates | every minute |
|
||||
| Plugins | every hour |
|
||||
| Background / cron | every 2–12 hours |
|
||||
|
||||
This filter only runs when that check runs, so WordPress is already the rate
|
||||
limiter. A cache longer than its cadence does nothing but hide new releases
|
||||
underneath it — a release published, and a shop shown nothing.
|
||||
|
||||
From the command line, `wp transient delete wc_edm_release_feed` does the same.
|
||||
|
||||
|
||||
@@ -34,10 +34,24 @@ class WC_EDM_Updater {
|
||||
const TRANSIENT = 'wc_edm_release_feed';
|
||||
|
||||
/**
|
||||
* How long that cache lives. WordPress checks for updates twice a day; a
|
||||
* shorter window would only add requests, a longer one would delay a fix.
|
||||
* How long that cache lives.
|
||||
*
|
||||
* Six hours, originally, on the reasoning that WordPress checks twice a
|
||||
* day anyway. That reasoning was wrong, and the symptom was a shop that
|
||||
* had just been told a new version existed and was shown nothing.
|
||||
*
|
||||
* WordPress does not check twice a day when somebody is looking. It gates
|
||||
* its own check by screen — one minute on Dashboard → Updates, one hour on
|
||||
* the Plugins screen, twelve hours in the background (wp_update_plugins(),
|
||||
* switch on current_filter()). This filter only runs when that check runs,
|
||||
* so WordPress is already the rate limiter, and a cache longer than its
|
||||
* cadence does nothing but hide new releases underneath it.
|
||||
*
|
||||
* Fifteen minutes is left only to absorb a burst — several calls in one
|
||||
* request, or other code calling wp_update_plugins() with extra stats,
|
||||
* which bypasses the gate above.
|
||||
*/
|
||||
const TTL = 6 * HOUR_IN_SECONDS;
|
||||
const TTL = 15 * MINUTE_IN_SECONDS;
|
||||
|
||||
/**
|
||||
* Hook into the update check for our own host only.
|
||||
|
||||
+53
-7
@@ -14,7 +14,29 @@ if ( ! defined( 'ABSPATH' ) ) {
|
||||
exit;
|
||||
}
|
||||
|
||||
$wc_edm_failures = 0;
|
||||
/*
|
||||
* The counter lives in $GLOBALS on purpose, and the four lines below prove it
|
||||
* reaches the verdict before anything relies on it.
|
||||
*
|
||||
* `wp eval-file` runs this file through eval() INSIDE A METHOD, so the top
|
||||
* level here is function scope, not global scope. A plain `$failures = 0` at
|
||||
* the top and a `global $failures` in a helper below are therefore two
|
||||
* different variables: the helpers increment the real global, the verdict at
|
||||
* the bottom reads the local, and the local is always zero. Every assertion
|
||||
* fails, every FAIL line prints, the file ends with "All assertions passed"
|
||||
* and exits 0 — and CI goes green. That is not reasoning, it is what this file
|
||||
* did until it was measured.
|
||||
*
|
||||
* This is the one file whose entire job is to be able to go red, so it checks
|
||||
* that it can.
|
||||
*/
|
||||
$GLOBALS['wc_edm_failures'] = 0;
|
||||
( static function () { ++$GLOBALS['wc_edm_failures']; } )();
|
||||
if ( 1 !== $GLOBALS['wc_edm_failures'] ) {
|
||||
echo "The failure counter does not reach the verdict: this file cannot go red.\n";
|
||||
exit( 1 );
|
||||
}
|
||||
$GLOBALS['wc_edm_failures'] = 0;
|
||||
|
||||
/**
|
||||
* Assert a condition and keep going, so one run reports every failure.
|
||||
@@ -24,14 +46,12 @@ $wc_edm_failures = 0;
|
||||
* @param string $detail Shown when it fails.
|
||||
*/
|
||||
function wc_edm_ok( $what, $condition, $detail = '' ) {
|
||||
global $wc_edm_failures;
|
||||
|
||||
if ( $condition ) {
|
||||
printf( " ok %s\n", $what );
|
||||
return;
|
||||
}
|
||||
|
||||
++$wc_edm_failures;
|
||||
++$GLOBALS['wc_edm_failures'];
|
||||
printf( " FAIL %s%s\n", $what, '' !== $detail ? " — {$detail}" : '' );
|
||||
}
|
||||
|
||||
@@ -345,7 +365,18 @@ wc_edm_is( 'the current version is answered too, not passed over', WC_EDM_VERSIO
|
||||
* WordPress does with our answer, and a test that needs the internet fails for
|
||||
* reasons that have nothing to do with it.
|
||||
*/
|
||||
// "Check again" must discard the six-hour cache, or the button lies: someone
|
||||
// The cache must not outlive WordPress's own check cadence. It gates by screen
|
||||
// — one minute on Dashboard → Updates, one hour on the Plugins screen — and
|
||||
// this filter only runs when that check runs, so anything longer here hides a
|
||||
// new release underneath a cache the shop owner cannot see. This was six hours
|
||||
// and produced exactly that: a release published, and a shop shown nothing.
|
||||
wc_edm_ok(
|
||||
'the cache is shorter than the interval WordPress re-checks on',
|
||||
WC_EDM_Updater::TTL <= 15 * MINUTE_IN_SECONDS,
|
||||
'TTL is ' . WC_EDM_Updater::TTL . 's; WordPress re-checks every 60s on Dashboard → Updates'
|
||||
);
|
||||
|
||||
// "Check again" must discard the cache, or the button lies: someone
|
||||
// who has just published a release clicks it, sees nothing, and concludes the
|
||||
// whole mechanism is broken.
|
||||
set_transient( WC_EDM_Updater::TRANSIENT, array( 'version' => '1.0.0', 'package' => 'x', 'url' => 'x', 'tested' => '' ), 600 );
|
||||
@@ -368,6 +399,21 @@ delete_transient( WC_EDM_Updater::TRANSIENT );
|
||||
// strlen(null) deprecation from class-wp-plugins-list-table.php. It is
|
||||
// WordPress's, not ours, which is why the deprecation check filters on this
|
||||
// plugin's directory rather than failing on everything it sees.
|
||||
// Primed again deliberately. The force-check assertions above deleted it, and
|
||||
// without this offer_update() makes a real HTTP request — which passes only
|
||||
// because this plugin's feed repo happens to be public. That would make the
|
||||
// assertion measure the environment rather than the code under test.
|
||||
set_transient(
|
||||
WC_EDM_Updater::TRANSIENT,
|
||||
array(
|
||||
'version' => WC_EDM_VERSION,
|
||||
'package' => 'https://gitea.nolimit.dk/kw/demo/releases/download/v1/demo.zip',
|
||||
'url' => 'https://gitea.nolimit.dk/kw/demo',
|
||||
'tested' => '7.1',
|
||||
),
|
||||
600
|
||||
);
|
||||
|
||||
delete_site_transient( 'update_plugins' );
|
||||
wp_update_plugins();
|
||||
$transient = get_site_transient( 'update_plugins' );
|
||||
@@ -407,8 +453,8 @@ delete_transient( WC_EDM_Updater::TRANSIENT );
|
||||
/* --------------------------------------------------------------- verdict -- */
|
||||
|
||||
echo "\n";
|
||||
if ( $wc_edm_failures > 0 ) {
|
||||
printf( "%d assertion(s) failed on WordPress %s.\n", $wc_edm_failures, get_bloginfo( 'version' ) );
|
||||
if ( $GLOBALS['wc_edm_failures'] > 0 ) {
|
||||
printf( "%d assertion(s) failed on WordPress %s.\n", $GLOBALS['wc_edm_failures'], get_bloginfo( 'version' ) );
|
||||
exit( 1 );
|
||||
}
|
||||
printf(
|
||||
|
||||
Reference in New Issue
Block a user