Merge pull request #35 from kristianwind/assert-structure-not-text
CI / Static checks (push) Successful in 48s
CI / WordPress latest + WooCommerce latest on PHP 8.3 (push) Successful in 1m1s

Assert structure rather than text, and never leave a half-built zip
This commit is contained in:
Kristian Wind
2026-09-20 10:21:49 +02:00
committed by GitHub
3 changed files with 54 additions and 19 deletions
+9
View File
@@ -53,6 +53,15 @@ zip_entries() {
ci_step "what the zip contains" ci_step "what the zip contains"
zip_entries | sed 's/^/ /' zip_entries | sed 's/^/ /'
# 🔴 A failed build must not leave a zip behind. Every exit below this point
# happens AFTER the archive is written, so without this the tree keeps a
# half-built artefact that looks exactly like a good one — and the release step
# takes `build/<slug>.zip` by name, as does anyone publishing by hand. The
# check that caught it: stubbing ci/dist-files.sh empty makes this script exit
# 1 with "The main plugin file is not in the zip", and the zip was still there
# afterwards.
trap 'code=$?; [ "$code" = 0 ] || { rm -f "build/$PLUGIN_SLUG.zip"; echo "Removed the incomplete build/$PLUGIN_SLUG.zip." >&2; }; exit $code' EXIT
ci_step "checks" ci_step "checks"
zip_entries | grep -qx "$PLUGIN_SLUG/$PLUGIN_MAIN" \ zip_entries | grep -qx "$PLUGIN_SLUG/$PLUGIN_MAIN" \
|| { echo "The main plugin file is not in the zip." >&2; exit 1; } || { echo "The main plugin file is not in the zip." >&2; exit 1; }
+22 -8
View File
@@ -278,13 +278,16 @@ wc_edm_ok(
// The other half: the by-hand require above would pass even if the plugin // The other half: the by-hand require above would pass even if the plugin
// never loaded the file in a real admin request. This is the assertion that // never loaded the file in a real admin request. This is the assertion that
// covers that, and it is the half a reduced repro always omits. // covers that, and it is the half a reduced repro always omits.
//
// Read from the list the plugin actually loads from, NOT searched for in this
// file's source text. The previous version did the latter, and a comment
// mentioning that path would have satisfied it — the same hollow shape as
// grepping a file for the words of the guard it is supposed to have, which is
// a bug this kit has already had once today.
wc_edm_ok( wc_edm_ok(
'and the plugin itself loads the shared menu file in admin', 'and the plugin itself loads the shared menu file in admin',
false !== strpos( in_array( 'includes/shared-menu.php', WC_Email_Discount_Manager::admin_includes(), true ),
file_get_contents( WC_EDM_PLUGIN_FILE ), 'admin includes: ' . implode( ', ', WC_Email_Discount_Manager::admin_includes() )
"'includes/shared-menu.php',"
),
'it is not in the is_admin() branch of includes(), so nothing would define wpmgr_ensure_menu()'
); );
// The card on the shared dashboard. Absent from it is a different failure from // The card on the shared dashboard. Absent from it is a different failure from
@@ -390,10 +393,21 @@ wc_edm_is(
$update_uri . '/releases/download/v1.4.0/woocommerce-email-discount-manager.zip', $update_uri . '/releases/download/v1.4.0/woocommerce-email-discount-manager.zip',
$parsed['package'] $parsed['package']
); );
wc_edm_ok( // Asserted as the host, not as the absence of one particular IP address.
// Gitea's ROOT_URL has been a raw address before, and the package URL it
// reports carries whatever that is — so a check that names today's IP is blind
// to tomorrow's. What matters is that the URL points at the host we publish
// from, however the forge chose to describe itself.
$wc_edm_host = wp_parse_url( $parsed['package'], PHP_URL_HOST );
wc_edm_is(
'so it is never the raw address the forge happened to report', 'so it is never the raw address the forge happened to report',
false === strpos( $parsed['package'], '57.129.89.117' ) && 0 === strpos( $parsed['package'], 'https://' ), 'gitea.nolimit.dk',
'got ' . $parsed['package'] $wc_edm_host
);
wc_edm_is(
'and it is fetched over https',
'https',
wp_parse_url( $parsed['package'], PHP_URL_SCHEME )
); );
wc_edm_is( 'and the tested-up-to line is read out of the release notes', '7.1', $parsed['tested'] ); wc_edm_is( 'and the tested-up-to line is read out of the release notes', '7.1', $parsed['tested'] );
wc_edm_ok( 'a release with no zip attached is not offered at all', null === $parse->invoke( new WC_EDM_Updater(), array( 'tag_name' => 'v9.9.9', 'assets' => array() ) ) ); wc_edm_ok( 'a release with no zip attached is not offered at all', null === $parse->invoke( new WC_EDM_Updater(), array( 'tag_name' => 'v9.9.9', 'assets' => array() ) ) );
+23 -11
View File
@@ -220,6 +220,28 @@ final class WC_Email_Discount_Manager {
/** /**
* Load class files. * Load class files.
*/ */
/**
* The files loaded only in an admin request.
*
* A method rather than a literal inside includes() so a test can read the
* actual list. The assertion it replaces searched this file's SOURCE TEXT
* for the shared menu's path, which a comment mentioning that path would
* have satisfied just as well — the same hollow shape as grepping a file
* for the words of the guard it is supposed to have.
*
* @return string[]
*/
public static function admin_includes() {
return array(
'includes/shared-menu.php',
'includes/class-admin.php',
'includes/class-admin-groups.php',
'includes/class-admin-emails.php',
'includes/class-admin-shipping.php',
'includes/class-admin-audit.php',
);
}
private function includes() { private function includes() {
$files = array( $files = array(
'includes/class-db.php', 'includes/class-db.php',
@@ -231,17 +253,7 @@ final class WC_Email_Discount_Manager {
); );
if ( is_admin() ) { if ( is_admin() ) {
$files = array_merge( $files = array_merge( $files, self::admin_includes() );
$files,
array(
'includes/shared-menu.php',
'includes/class-admin.php',
'includes/class-admin-groups.php',
'includes/class-admin-emails.php',
'includes/class-admin-shipping.php',
'includes/class-admin-audit.php',
)
);
} }
foreach ( $files as $file ) { foreach ( $files as $file ) {