From 02ebe05ecdf267d7f18cec84ab16ca6f155ca205 Mon Sep 17 00:00:00 2001 From: Ryan Leeson Date: Tue, 4 Aug 2026 11:06:36 -0400 Subject: [PATCH] fail the build when the class cache is not written; resolve review nits Build command (the one path that could still resurrect #30): - generate_cache() now explicitly verifies a non-empty cache file exists as Spatie will provide false positive cache builds on un-writeable locations. - Explicitly clear existing cache files for in place rebuilds to avoid stale files. - Tests cover both paths: an unwritable target fails with nothing written, and a regenerate that cannot replace an existing cache fails the build. Test isolation (correcting 90a3956): - Retesting showed @runTestsInSeparateProcesses was inherited, fixed annotations and added the trait to test_disable_constant_forces_live_discovery to avoid confusion - Dropped the ReflectionProperty/Method::setAccessible() calls, which have had no effect since PHP 8.1 and are deprecated in 8.5. Nits from review: - Updated all i18n text domains to tenup-plugin, matching the rest of the plugin - Trimmed the ModuleInitialization cache/live fallback documentation to concise statements - Revise Upgrade Guide documentation to list the five current cache badges/messages - Debugging: note that a tenup_framework_cache_load_failed listener has to be registered before the failing loader runs, so full coverage needs an mu-plugin. phpcs and phpstan (level 10) clean; phpunit green on PHP 8.3 (65 tests / 181 assertions). On local PHP 8.5 the three process-isolated tests error on a patchwork deprecation flood, unrelated to these changes. Refs #30 Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 1 + docs/Debugging.md | 4 ++ docs/Upgrade-Guide.md | 15 +++-- src/Debug/LoaderDebug.php | 86 ++++++++++++++-------------- src/ModuleInitialization.php | 40 +++++++++---- tests/Bin/GenerateClassCacheTest.php | 57 ++++++++++++++++++ tests/Debug/LoaderDebugTest.php | 2 - tests/FrameworkTestSetup.php | 19 +++--- tests/ModuleInitializationTest.php | 8 +++ 9 files changed, 159 insertions(+), 73 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 23dc70e..8b388a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ All notable changes to this project will be documented in this file, per [the Ke - The class-loader cache is now **read-only at runtime** and opt-in. The framework reads a pre-built cache if present and discovers live otherwise, but never writes one on the server — fixing stale caches that could only be cleared by hand ([#30](https://github.com/10up/wp-framework/issues/30)). - A corrupt or truncated shipped cache is caught at runtime and the request falls back to a live scan instead of fataling, so a bad cache degrades performance rather than taking the site down. The fallback fires a `tenup_framework_cache_load_failed` action (for logging or alerting) and the loader debug page flags that loader red as "Cache failed to load — running live" instead of reporting it as in use. - Bumped the cache filename so a cache written by an older version is ignored after upgrade rather than served stale. +- `generate_cache()` now verifies the cache file was actually written and fails with a `RuntimeException` (a non-zero exit from the build command) when it was not. An un-writable target or a full disk previously produced only PHP warnings, so the command reported success and the build stayed green having shipped no cache — or, on a deploy that does not clean, the previous build's cache. Any writable cache location from an earlier build is cleared before the write so a failure cannot leave a stale file behind. - `TENUP_FRAMEWORK_DISABLE_CLASS_CACHE` now forces live discovery (ignores any shipped cache). ### Removed diff --git a/docs/Debugging.md b/docs/Debugging.md index 18de671..cd3ca88 100644 --- a/docs/Debugging.md +++ b/docs/Debugging.md @@ -74,6 +74,10 @@ add_action( 'tenup_framework_cache_load_failed', function ( $dir, $error ) { }, 10, 2 ); ``` +The listener has to be registered *before* the loader that fails runs. Plugins typically call +`init_classes()` as their main file loads, which is earlier than another plugin can hook, so to +catch every loader reliably register this from an mu-plugin. + The fix is to rebuild the cache in your pipeline and redeploy. ## Known limitations diff --git a/docs/Upgrade-Guide.md b/docs/Upgrade-Guide.md index d22fcbd..7489a2b 100644 --- a/docs/Upgrade-Guide.md +++ b/docs/Upgrade-Guide.md @@ -95,11 +95,16 @@ Because nothing in the runtime announces whether caching is on, confirm it expli /wp-admin/admin.php?page=tenup-framework-loaders ``` -For each loader it shows a **Cache status** line: - -- *"discovering live on every request"* — no cache is in use (expected if you didn't add the build - step). -- *"Cache in use …"* — a pre-built cache is being read (expected after wiring in the build step). +Each loader carries a status badge: + +- **Uncached — live discovery** — no cache is in use, so classes are discovered on every request + (expected if you didn't add the build step). +- **Cache in use** — a pre-built cache is being read (expected after wiring in the build step). +- **Caching disabled** — `TENUP_FRAMEWORK_DISABLE_CLASS_CACHE` is set, so any shipped cache is + ignored. +- **Cache failed to load — running live** — a cache file is present but could not be read; rebuild + it in your pipeline and redeploy. +- **Cache present but not used** — a cache file exists that the loader did not read. Use the per-loader **staleness check** to confirm a shipped cache matches what's on disk. diff --git a/src/Debug/LoaderDebug.php b/src/Debug/LoaderDebug.php index 0bf7a97..8e32a5b 100644 --- a/src/Debug/LoaderDebug.php +++ b/src/Debug/LoaderDebug.php @@ -170,7 +170,7 @@ static function ( $loaders ) { * @return void */ public static function register_page() { - $title = __( 'WP Framework Loaders', 'tenup-framework' ); + $title = __( 'WP Framework Loaders', 'tenup-plugin' ); add_submenu_page( '', @@ -189,7 +189,7 @@ public static function register_page() { */ public static function render_page() { if ( ! current_user_can( self::CAPABILITY ) ) { - wp_die( esc_html__( 'You do not have permission to view this page.', 'tenup-framework' ) ); + wp_die( esc_html__( 'You do not have permission to view this page.', 'tenup-plugin' ) ); } // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- Read-only diagnostic; the value is nonce-verified below before use. @@ -203,11 +203,11 @@ public static function render_page() { echo '
'; self::render_styles(); - echo '

' . esc_html__( 'WP Framework Loaders', 'tenup-framework' ) . '

'; - echo '

' . esc_html__( 'Each card is a directory passed to ModuleInitialization::init_classes(), with the state of its class-loader cache. Caches are built at deploy time and read — never written — at runtime.', 'tenup-framework' ) . '

'; + echo '

' . esc_html__( 'WP Framework Loaders', 'tenup-plugin' ) . '

'; + echo '

' . esc_html__( 'Each card is a directory passed to ModuleInitialization::init_classes(), with the state of its class-loader cache. Caches are built at deploy time and read — never written — at runtime.', 'tenup-plugin' ) . '

'; if ( empty( $loaders ) ) { - echo '
' . esc_html__( 'No class loaders were recorded for this request.', 'tenup-framework' ) . '
'; + echo '
' . esc_html__( 'No class loaders were recorded for this request.', 'tenup-plugin' ) . '
'; echo '
'; return; } @@ -254,26 +254,26 @@ protected static function render_loader( array $loader, string $check ) { echo '
' . esc_html( sprintf( /* translators: %s: comma-separated list of unexpected filenames. */ - __( 'Unexpected files in the cache directory, likely left by an older version: %s. Delete them or redeploy.', 'tenup-framework' ), + __( 'Unexpected files in the cache directory, likely left by an older version: %s. Delete them or redeploy.', 'tenup-plugin' ), implode( ', ', $legacy ) ) ) . '
'; } echo ''; - self::render_row( __( 'Directory', 'tenup-framework' ), $directory ); - self::render_row( __( 'Framework version', 'tenup-framework' ), self::version_label( $loader ) ); - self::render_row( __( 'Cache file', 'tenup-framework' ), '' !== $cache_file ? $cache_file : '—' ); - self::render_row( __( 'Cache detail', 'tenup-framework' ), self::cache_detail( $loader ) ); - self::render_row( __( 'Discovery time', 'tenup-framework' ), self::format_duration( $loader['discovery_seconds'] ?? null ) ); - self::render_row( __( 'Class lookup time', 'tenup-framework' ), self::format_duration( $loader['lookup_seconds'] ?? null ) ); + self::render_row( __( 'Directory', 'tenup-plugin' ), $directory ); + self::render_row( __( 'Framework version', 'tenup-plugin' ), self::version_label( $loader ) ); + self::render_row( __( 'Cache file', 'tenup-plugin' ), '' !== $cache_file ? $cache_file : '—' ); + self::render_row( __( 'Cache detail', 'tenup-plugin' ), self::cache_detail( $loader ) ); + self::render_row( __( 'Discovery time', 'tenup-plugin' ), self::format_duration( $loader['discovery_seconds'] ?? null ) ); + self::render_row( __( 'Class lookup time', 'tenup-plugin' ), self::format_duration( $loader['lookup_seconds'] ?? null ) ); echo '
'; echo '
'; echo '' . esc_html( sprintf( /* translators: %d: number of classes. */ - _n( '%d class loaded', '%d classes loaded', count( $classes ), 'tenup-framework' ), + _n( '%d class loaded', '%d classes loaded', count( $classes ), 'tenup-plugin' ), count( $classes ) ) ) . ''; @@ -310,13 +310,13 @@ protected static function render_classes( array $classes ) { } echo ''; - echo ''; + echo ''; $module_init = ModuleInitialization::instance(); foreach ( $classes as $class ) { $reflection = $module_init->get_fully_loadable_class( $class ); - $file = $reflection ? (string) $reflection->getFileName() : __( 'Does not resolve — likely a stale cache entry.', 'tenup-framework' ); + $file = $reflection ? (string) $reflection->getFileName() : __( 'Does not resolve — likely a stale cache entry.', 'tenup-plugin' ); echo ''; } @@ -350,7 +350,7 @@ protected static function render_staleness( string $directory, array $classes, s self::CHECK_NONCE ); - echo '

' . esc_html__( 'Check this cache for staleness', 'tenup-framework' ) . '

'; + echo '

' . esc_html__( 'Check this cache for staleness', 'tenup-plugin' ) . '

'; return; } @@ -364,20 +364,20 @@ protected static function render_staleness( string $directory, array $classes, s $timing = sprintf( /* translators: %s: formatted duration. */ - __( 'Live discovery took %s.', 'tenup-framework' ), + __( 'Live discovery took %s.', 'tenup-plugin' ), self::format_duration( $live_seconds ) ); if ( empty( $removed ) && empty( $added ) ) { - echo '
' . esc_html__( 'Up to date — the cache matches a live scan.', 'tenup-framework' ) . ' ' . esc_html( $timing ) . '
'; + echo '
' . esc_html__( 'Up to date — the cache matches a live scan.', 'tenup-plugin' ) . ' ' . esc_html( $timing ) . '
'; return; } echo '
'; - echo '' . esc_html__( 'Stale — the cache differs from a live scan.', 'tenup-framework' ) . ' ' . esc_html( $timing ); + echo '' . esc_html__( 'Stale — the cache differs from a live scan.', 'tenup-plugin' ) . ' ' . esc_html( $timing ); if ( ! empty( $added ) ) { - echo '

' . esc_html__( 'On disk but missing from the cache:', 'tenup-framework' ) . '

    '; + echo '

    ' . esc_html__( 'On disk but missing from the cache:', 'tenup-plugin' ) . '

      '; foreach ( $added as $class ) { echo '
    • ' . esc_html( $class ) . '
      • '; } @@ -385,14 +385,14 @@ protected static function render_staleness( string $directory, array $classes, s } if ( ! empty( $removed ) ) { - echo '

      ' . esc_html__( 'In the cache but no longer on disk:', 'tenup-framework' ) . '

        '; + echo '

        ' . esc_html__( 'In the cache but no longer on disk:', 'tenup-plugin' ) . '

          '; foreach ( $removed as $class ) { echo '
        • ' . esc_html( $class ) . '
          • '; } echo '
        '; } - echo '

        ' . esc_html__( 'Regenerate the cache in your build (composer generate-class-cache) or remove the file and redeploy.', 'tenup-framework' ) . '

        '; + echo '

        ' . esc_html__( 'Regenerate the cache in your build (composer generate-class-cache) or remove the file and redeploy.', 'tenup-plugin' ) . '

        '; echo '
'; } @@ -429,16 +429,16 @@ protected static function format_duration( $seconds ): string { if ( $milliseconds < 1 ) { /* translators: %s: duration in milliseconds. */ - return sprintf( __( '%s ms', 'tenup-framework' ), number_format( $milliseconds, 3 ) ); + return sprintf( __( '%s ms', 'tenup-plugin' ), number_format( $milliseconds, 3 ) ); } if ( $milliseconds < 1000 ) { /* translators: %s: duration in milliseconds. */ - return sprintf( __( '%s ms', 'tenup-framework' ), number_format( $milliseconds, 2 ) ); + return sprintf( __( '%s ms', 'tenup-plugin' ), number_format( $milliseconds, 2 ) ); } /* translators: %s: duration in seconds. */ - return sprintf( __( '%s s', 'tenup-framework' ), number_format( $seconds, 2 ) ); + return sprintf( __( '%s s', 'tenup-plugin' ), number_format( $seconds, 2 ) ); } /** @@ -478,18 +478,18 @@ protected static function check_is_valid( string $token ): bool { */ protected static function owner_label( string $directory ): string { if ( '' === $directory ) { - return __( 'Unknown loader', 'tenup-framework' ); + return __( 'Unknown loader', 'tenup-plugin' ); } $roots = []; if ( defined( 'WP_PLUGIN_DIR' ) ) { - $roots[] = [ self::to_string( constant( 'WP_PLUGIN_DIR' ) ), __( 'Plugin', 'tenup-framework' ) ]; + $roots[] = [ self::to_string( constant( 'WP_PLUGIN_DIR' ) ), __( 'Plugin', 'tenup-plugin' ) ]; } if ( defined( 'WPMU_PLUGIN_DIR' ) ) { - $roots[] = [ self::to_string( constant( 'WPMU_PLUGIN_DIR' ) ), __( 'Must-use plugin', 'tenup-framework' ) ]; + $roots[] = [ self::to_string( constant( 'WPMU_PLUGIN_DIR' ) ), __( 'Must-use plugin', 'tenup-plugin' ) ]; } if ( function_exists( 'get_theme_root' ) ) { - $roots[] = [ self::to_string( get_theme_root() ), __( 'Theme', 'tenup-framework' ) ]; + $roots[] = [ self::to_string( get_theme_root() ), __( 'Theme', 'tenup-plugin' ) ]; } foreach ( $roots as $candidate ) { @@ -500,7 +500,7 @@ protected static function owner_label( string $directory ): string { $segment = explode( '/', $relative )[0]; /* translators: 1: owner type (Plugin/Theme), 2: plugin or theme folder name. */ - return sprintf( __( '%1$s: %2$s', 'tenup-framework' ), $type, $segment ); + return sprintf( __( '%1$s: %2$s', 'tenup-plugin' ), $type, $segment ); } } @@ -519,7 +519,7 @@ protected static function version_label( array $loader ): string { $reference = self::to_string( $loader['reference'] ?? '' ); if ( '' === $version ) { - $version = __( 'unknown', 'tenup-framework' ); + $version = __( 'unknown', 'tenup-plugin' ); } if ( '' !== $reference ) { @@ -544,38 +544,38 @@ protected static function cache_state( array $loader ): array { if ( ! empty( $loader['cache_disabled'] ) ) { return [ 'severity' => 'warn', - 'badge' => __( 'Caching disabled', 'tenup-framework' ), - 'note' => __( 'TENUP_FRAMEWORK_DISABLE_CLASS_CACHE is set, so any shipped cache is ignored and classes are discovered live on every request.', 'tenup-framework' ), + 'badge' => __( 'Caching disabled', 'tenup-plugin' ), + 'note' => __( 'TENUP_FRAMEWORK_DISABLE_CLASS_CACHE is set, so any shipped cache is ignored and classes are discovered live on every request.', 'tenup-plugin' ), ]; } if ( ! empty( $loader['cache_failed'] ) ) { return [ 'severity' => 'error', - 'badge' => __( 'Cache failed to load — running live', 'tenup-framework' ), - 'note' => __( 'A cache file is present but could not be read (corrupt or truncated), so the framework fell back to a live scan on every request. Rebuild the cache in your pipeline and redeploy.', 'tenup-framework' ), + 'badge' => __( 'Cache failed to load — running live', 'tenup-plugin' ), + 'note' => __( 'A cache file is present but could not be read (corrupt or truncated), so the framework fell back to a live scan on every request. Rebuild the cache in your pipeline and redeploy.', 'tenup-plugin' ), ]; } if ( empty( $loader['cache_exists'] ) ) { return [ 'severity' => 'warn', - 'badge' => __( 'Uncached — live discovery', 'tenup-framework' ), - 'note' => __( 'No cache file is present, so classes are discovered live on every request. That is the correct default for small projects; for large codebases, build a cache in your pipeline (see Build and Deployment).', 'tenup-framework' ), + 'badge' => __( 'Uncached — live discovery', 'tenup-plugin' ), + 'note' => __( 'No cache file is present, so classes are discovered live on every request. That is the correct default for small projects; for large codebases, build a cache in your pipeline (see Build and Deployment).', 'tenup-plugin' ), ]; } if ( empty( $loader['cache_used'] ) ) { return [ 'severity' => 'error', - 'badge' => __( 'Cache present but not used', 'tenup-framework' ), - 'note' => __( 'A cache file exists but is not being used. This is unexpected — check TENUP_FRAMEWORK_DISABLE_CLASS_CACHE.', 'tenup-framework' ), + 'badge' => __( 'Cache present but not used', 'tenup-plugin' ), + 'note' => __( 'A cache file exists but is not being used. This is unexpected — check TENUP_FRAMEWORK_DISABLE_CLASS_CACHE.', 'tenup-plugin' ), ]; } return [ 'severity' => 'ok', - 'badge' => __( 'Cache in use', 'tenup-framework' ), + 'badge' => __( 'Cache in use', 'tenup-plugin' ), 'note' => '', ]; } @@ -592,7 +592,7 @@ protected static function cache_detail( array $loader ): string { $cache_file = self::to_string( $loader['cache_file'] ?? '' ); if ( '' === $cache_file || ! file_exists( $cache_file ) ) { - return __( 'No cache file on disk.', 'tenup-framework' ); + return __( 'No cache file on disk.', 'tenup-plugin' ); } $mtime = (int) filemtime( $cache_file ); @@ -601,14 +601,14 @@ protected static function cache_detail( array $loader ): string { if ( ! $mtime ) { return sprintf( /* translators: %s: file size. */ - __( 'Built at an unknown time · %s', 'tenup-framework' ), + __( 'Built at an unknown time · %s', 'tenup-plugin' ), size_format( $size ) ); } return sprintf( /* translators: 1: relative age (e.g. "5 minutes"); 2: file size; 3: absolute build time in UTC. */ - __( 'Built %1$s ago · %2$s · %3$s', 'tenup-framework' ), + __( 'Built %1$s ago · %2$s · %3$s', 'tenup-plugin' ), human_time_diff( $mtime ), size_format( $size ), gmdate( 'Y-m-d H:i:s', $mtime ) . ' UTC' diff --git a/src/ModuleInitialization.php b/src/ModuleInitialization.php index a6ee4b1..9bbbfa1 100644 --- a/src/ModuleInitialization.php +++ b/src/ModuleInitialization.php @@ -96,6 +96,9 @@ private function __construct() { * * @param string $dir The directory to search for classes. * + * @throws \RuntimeException If the Module directory does not exist. + * @throws \Throwable If the live cache fallback fails to load. + * * @return array */ public function get_classes( $dir ) { @@ -106,7 +109,7 @@ public function get_classes( $dir ) { // The runtime only ever reads a pre-built cache; it never writes one. Caching is // therefore opt-in: with no cache file present we discover live on every request, // which is the correct default. A cache is produced at build time via the - // `tenup-framework-generate-class-cache` command and shipped as a build artefact. + // `tenup-framework-generate-class-cache` command and shipped as a build artifact. // // Define TENUP_FRAMEWORK_DISABLE_CLASS_CACHE to ignore any shipped cache and always // discover live (useful for debugging). @@ -123,16 +126,10 @@ public function get_classes( $dir ) { $this->cache_read_failed = false; + // Module class cache failure management - live fallback Discover::get() can issue a Throwable try { - // array_filter is inside the try so that a cache which parses but returns a - // non-array (not only a truncated one) also falls back rather than fataling here. return array_filter( $class_finder->get(), fn( $cl ) => is_string( $cl ) ); } catch ( \Throwable $e ) { - // A shipped cache file that is corrupt or truncated — a partial deploy, an - // interrupted build, a half-written rsync — would otherwise fatal on every request - // (the cache is executable PHP loaded with `require`). Fall back to a fresh live - // discovery so the site keeps working, uncached, until the cache is rebuilt. This - // is the same spirit as issue #30: a bad cache must never take the site down. $this->cache_read_failed = true; if ( function_exists( 'do_action' ) ) { @@ -147,6 +144,7 @@ public function get_classes( $dir ) { do_action( 'tenup_framework_cache_load_failed', $dir, $e ); } + // Live directory discovery fallback (issue #30) in case of cache failure. return array_filter( $this->build_discoverer( $dir )->get(), fn( $cl ) => is_string( $cl ) ); } } @@ -157,10 +155,13 @@ public function get_classes( $dir ) { * This is the build-time counterpart to get_classes(): it is the only place the * framework writes the cache, and it deliberately makes no WordPress calls so it can * run from a plain CLI script during CI without bootstrapping WordPress. The resulting - * file is then deployed as a build artefact and read (never rewritten) at runtime. + * file is then deployed as a build artifact and read (never rewritten) at runtime. * * @param string $dir The directory to search for classes. * + * @throws \RuntimeException If a cache from a previous build cannot be cleared, or if no + * cache file was written. + * * @return array The discovered class names that were cached. */ public function generate_cache( $dir = '' ) { @@ -177,10 +178,27 @@ public function generate_cache( $dir = '' ) { ) ); - // cache() forces a fresh discovery and overwrites any existing cache file, so a - // regenerate always reflects the current code rather than a previous build. + $cache_file = $this->get_cache_directory( $dir ) . '/' . self::CACHE_FILENAME; + + // Clear previous cache file *before* writing (avoiding issue #30), for in place rebuilds. + // No WordPress bootstrap, so unlink() is used over wp_delete_file(). + // phpcs:ignore WordPress.WP.AlternativeFunctions.unlink_unlink + if ( file_exists( $cache_file ) && ! unlink( $cache_file ) ) { + // phpcs:ignore WordPress.Security.EscapeOutput.ExceptionNotEscaped + throw new \RuntimeException( 'Could not remove the existing class cache at "' . $cache_file . '".' ); + } + $classes = $class_finder->cache(); + // Spatie's file driver writes with mkdir()/file_put_contents(), discarding return values. + // This can surface an un-writable target or a full disk, avoiding false positives builds. + clearstatcache( true, $cache_file ); + + if ( ! is_file( $cache_file ) || filesize( $cache_file ) < 1 ) { + // phpcs:ignore WordPress.Security.EscapeOutput.ExceptionNotEscaped + throw new \RuntimeException( 'Failed to write the class cache to "' . $cache_file . '". Check the directory is writable and the disk is not full.' ); + } + return array_filter( $classes, fn( $cl ) => is_string( $cl ) ); } diff --git a/tests/Bin/GenerateClassCacheTest.php b/tests/Bin/GenerateClassCacheTest.php index 81d10db..0d37702 100644 --- a/tests/Bin/GenerateClassCacheTest.php +++ b/tests/Bin/GenerateClassCacheTest.php @@ -114,6 +114,63 @@ public function test_caches_multiple_directories() { $this->assertContains( 'TenupFrameworkExamples\\Widgets\\Card', $cached ); } + /** + * A cache that cannot be written fails loudly instead of reporting success. + * + * Spatie's file driver ignores the return values of mkdir()/file_put_contents(), so without + * an explicit check the command printed "Cached N class(es)" and exited 0 having written + * nothing — a broken build staying green. + * + * @return void + */ + public function test_unwritable_directory_fails_instead_of_reporting_success() { + $dir = $this->example_copy( 'plugin-inc' ); + chmod( $dir, 0555 ); + + $result = $this->run_bin( [ $dir ] ); + + // Restore permissions first so teardown can always clean up. + chmod( $dir, 0755 ); + + $this->assertSame( 1, $result['exit'], 'An unwritable target must fail the build.' ); + $this->assertStringContainsString( 'Failed to write the class cache', $result['stderr'] ); + $this->assertStringNotContainsString( 'Cached', $result['stdout'] ); + $this->assertFileDoesNotExist( $this->cache_file_path( $dir ) ); + } + + /** + * A regenerate that cannot replace the previous build's cache fails the build rather than + * leaving the old file to be deployed as if it were freshly built (issue #30). + * + * The old file is unlinked before the write, so a write that fails for a reason unrelated to + * directory permissions (a full disk) leaves nothing behind. When the directory itself is + * unwritable the old file cannot be removed at all — so the guarantee that matters is the + * non-zero exit, which stops the pipeline before it can ship the stale cache. + * + * @return void + */ + public function test_regenerate_that_cannot_replace_a_stale_cache_fails_the_build() { + $dir = $this->example_copy( 'plugin-inc' ); + + // First build succeeds. + $this->assertSame( 0, $this->run_bin( [ $dir ] )['exit'] ); + $cache_file = $this->cache_file_path( $dir ); + $this->assertFileExists( $cache_file ); + + // Make only the cache directory unwritable, so the rewrite cannot happen. + $cache_dir = dirname( $cache_file ); + chmod( $cache_dir, 0555 ); + + $result = $this->run_bin( [ $dir ] ); + + // Restore permissions first so teardown can always clean up. + chmod( $cache_dir, 0755 ); + + $this->assertSame( 1, $result['exit'], 'A cache that cannot be replaced must fail the build.' ); + $this->assertStringContainsString( 'Could not remove the existing class cache', $result['stderr'] ); + $this->assertStringNotContainsString( 'Cached', $result['stdout'] ); + } + /** * Run the bin script with the given arguments, returning its stdout, stderr and exit code. * diff --git a/tests/Debug/LoaderDebugTest.php b/tests/Debug/LoaderDebugTest.php index 6eab4fd..57812ae 100644 --- a/tests/Debug/LoaderDebugTest.php +++ b/tests/Debug/LoaderDebugTest.php @@ -193,7 +193,6 @@ public function test_owner_label_derives_plugin_name() { define( 'WP_PLUGIN_DIR', '/srv/site/wp-content/plugins' ); $method = ( new \ReflectionClass( LoaderDebug::class ) )->getMethod( 'owner_label' ); - $method->setAccessible( true ); $this->assertSame( 'Plugin: demo', @@ -430,7 +429,6 @@ public function test_cache_detail_shows_size_and_utc_build_time() { */ private function invoke_protected( string $method, array $args ) { $reflection = ( new \ReflectionClass( LoaderDebug::class ) )->getMethod( $method ); - $reflection->setAccessible( true ); return $reflection->invokeArgs( null, $args ); } diff --git a/tests/FrameworkTestSetup.php b/tests/FrameworkTestSetup.php index d879f85..834a498 100644 --- a/tests/FrameworkTestSetup.php +++ b/tests/FrameworkTestSetup.php @@ -77,10 +77,11 @@ protected function setUp(): void { // phpcs:ignore WordPress.NamingConventions.V } /** - * Reset the ModuleInitialization singleton so its accumulated `$classes` do not leak between - * tests. The suite is not process-isolated (the trait-level annotation does not take effect), - * so without this a class registered in one test would be seen as "already initialized" in a - * later one. + * Reset the ModuleInitialization singleton so its accumulated `$classes` do not leak + * between tests. + * + * Since here the `@runTestsInSeparateProcesses` annotation on this trait *is* in effect, + * this guards in case of removed trait annotation of multiple single test init_classes() calls. * * @return void */ @@ -90,7 +91,6 @@ protected function reset_module_initialization(): void { } $instance = ( new \ReflectionClass( \TenupFramework\ModuleInitialization::class ) )->getProperty( 'instance' ); - $instance->setAccessible( true ); $instance->setValue( null, null ); } @@ -107,13 +107,8 @@ protected function reset_loader_debug(): void { $reflection = new \ReflectionClass( \TenupFramework\Debug\LoaderDebug::class ); - $loaders = $reflection->getProperty( 'loaders' ); - $loaders->setAccessible( true ); - $loaders->setValue( null, [] ); - - $booted = $reflection->getProperty( 'booted' ); - $booted->setAccessible( true ); - $booted->setValue( null, false ); + $reflection->getProperty( 'loaders' )->setValue( null, [] ); + $reflection->getProperty( 'booted' )->setValue( null, false ); unset( $GLOBALS['tenup_framework_debug_page_registered'] ); } diff --git a/tests/ModuleInitializationTest.php b/tests/ModuleInitializationTest.php index 5f054ff..ff099db 100644 --- a/tests/ModuleInitializationTest.php +++ b/tests/ModuleInitializationTest.php @@ -262,6 +262,14 @@ public function test_get_classes_falls_back_to_live_when_cache_is_corrupt() { * Defining TENUP_FRAMEWORK_DISABLE_CLASS_CACHE forces live discovery even when a * cache file is present. * + * The define() below is process-wide and cannot be undone, so this test must not share a + * process with the tests that assert a cache *is* used. The trait-level + * `@runTestsInSeparateProcesses` already guarantees that; the explicit annotation here + * makes the requirement local and survives that annotation being removed. + * + * @runInSeparateProcess + * @preserveGlobalState disabled + * * @return void */ public function test_disable_constant_forces_live_discovery() {
' . esc_html__( 'Class', 'tenup-framework' ) . '' . esc_html__( 'File', 'tenup-framework' ) . '
' . esc_html__( 'Class', 'tenup-plugin' ) . '' . esc_html__( 'File', 'tenup-plugin' ) . '
' . esc_html( $class ) . '' . esc_html( $file ) . '