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.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: hbhalodia <hbhalodia@git.wordpress.org>
Co-authored-by: westonruter <westonruter@git.wordpress.org>
Co-authored-by: b1ink0 <b1ink0@git.wordpress.org>

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

Hi @westonruter, Test fails because it is not able to mimic the production enviornment, If we have any workaround to check the both scenarios, I am happy to update the tests. Ref - #2031 (comment)

Hi @hbhalodia, I think removing the entire test would be the only solution, as wp_get_environment_type will always return local or development on the CI/CD, which will prevent the site health test from being added at all.

Hi @b1ink0, I have removed the tests for now.

Thank You,

Codecov Report

Attention: Patch coverage is 20.00000% with 4 lines in your changes missing coverage. Please review.

Project coverage is 68.02%. Comparing base (e741539) to head (f56f9d6).
Report is 8 commits behind head on trunk.

@@            Coverage Diff             @@
##            trunk    #2035      +/-   ##
==========================================
- Coverage   68.06%   68.02%   -0.05%     
==========================================
  Files          92       92              
  Lines        7626     7627       +1     
==========================================
- Hits         5191     5188       -3     
- Misses       2435     2439       +4     

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

I think removing the entire test would be the only solution, as wp_get_environment_type will always return local or development on the CI/CD, which will prevent the site health test from being added at all.

@b1ink0 @hbhalodia What about adding @runInSeparateProcess to the test so it can define the constant before the assertion?

I think removing the entire test would be the only solution, as wp_get_environment_type will always return local or development on the CI/CD, which will prevent the site health test from being added at all.

@b1ink0 @hbhalodia What about adding @runInSeparateProcess to the test so it can define the constant before the assertion?

Hi @westonruter, I can try that but how can I set 2 different values for the same constant? like we need to first test for local that key should not exists, and then again set the constant to production and check the key should exists. So that we can cover all the scenarios?

I checked this -

But still finding it complex on how to accomodate both scenarios?

OR, if I am thinking correctly,

Would add the normal test case with local scenario on current file as expected, and create a new file with @runInSeparateProcess and just call this one test again with mocking of constant to production?

@westonruter @runInSeparateProcess will not solve the issue, as the WP_ENVIRONMENT_TYPE constant is defined in /wordpress-phpunit/wp-tests-config.php in the Docker container. This config is generated by the wp-env package, and by default, it sets WP_ENVIRONMENT_TYPE to local. You can see that here: (https://github.com/WordPress/gutenberg/blob/a5e3592f0a4e66757f21754bfafe6c6ac7c83aea/packages/env/lib/config/parse-config.js#L99). As a result, WP_ENVIRONMENT_TYPE will be defined even before the test loads, which means the constant will still be defined even when @runInSeparateProcess is used.

This could only be overridden by modifying the .wp-env.json, but doing so would cause all tests to run in a defined environment, which is not the right approach.

The only other option I can think of is using the wp_get_development_mode function instead of wp_get_environment_type, as it allows its result to be overridden using the global variable $GLOBALS['_wp_tests_development_mode']. However, to use this, we also need to define the WP_RUN_CORE_TESTS constant, since this global is only supposed to be used in core tests.
Ref: (https://github.com/WordPress/wordpress-develop/blob/45cf47811a8adae99897ec4cceaf7fd3acbabe38/src/wp-includes/load.php#L328-L331).

If we go with this, then if you use wp-env for development, this site health test will not be displayed in your Site Health, because in this repo’s .wp-env.json, the WP_DEVELOPMENT_MODE constant is set to plugin.

For me, I mostly use LocalWP for development, so I have manually set the WP_DEVELOPMENT_MODE constant to plugin in the wp-config.php.

If we go with wp_get_development_mode, then the tests could be written like this:

diff --git a/plugins/performance-lab/includes/site-health/effective-asset-cache-headers/hooks.php b/plugins/performance-lab/includes/site-health/effective-asset-cache-headers/hooks.php
index cc083eee..07de32f1 100644
--- a/plugins/performance-lab/includes/site-health/effective-asset-cache-headers/hooks.php
+++ b/plugins/performance-lab/includes/site-health/effective-asset-cache-headers/hooks.php
@@ -32,7 +32,7 @@ function perflab_effective_asset_cache_headers_add_test( array $tests ): array {
 	 *
 	 * GH Issue: https://github.com/WordPress/performance/issues/2031
 	 */
-	if ( in_array( wp_get_environment_type(), array( 'local', 'development' ), true ) ) {
+	if ( in_array( wp_get_development_mode(), array( 'plugin' ), true ) ) {
 		unset( $tests['direct']['effective_asset_cache_headers'] );
 	}
 
diff --git a/plugins/performance-lab/tests/includes/site-health/effective-asset-cache-headers/test-effective-asset-cache-headers.php b/plugins/performance-lab/tests/includes/site-health/effective-asset-cache-headers/test-effective-asset-cache-headers.php
index b35cd092..9b104180 100644
--- a/plugins/performance-lab/tests/includes/site-health/effective-asset-cache-headers/test-effective-asset-cache-headers.php
+++ b/plugins/performance-lab/tests/includes/site-health/effective-asset-cache-headers/test-effective-asset-cache-headers.php
@@ -31,16 +31,47 @@ class Test_Effective_Asset_Cache_Headers extends WP_UnitTestCase {
 	/**
 	 * Test that the effective caching headers test is added to the site health tests.
 	 *
+	 * @runInSeparateProcess
+	 * @preserveGlobalState disabled
+	 *
+	 * @dataProvider data_provider_test_perflab_effective_asset_cache_headers_add_test
 	 * @covers ::perflab_effective_asset_cache_headers_add_test
+	 *
+	 * @param string $wp_development_mode The development mode to test against. Can be 'plugin' or '' (non-development mode).
 	 */
-	public function test_perflab_effective_asset_cache_headers_add_test(): void {
+	public function test_perflab_effective_asset_cache_headers_add_test( string $wp_development_mode ): void {
+		if ( ! defined( 'WP_RUN_CORE_TESTS' ) ) {
+			define( 'WP_RUN_CORE_TESTS', 'YES' );
+		}
+
+		$GLOBALS['_wp_tests_development_mode'] = $wp_development_mode;
+
 		$tests = array(
 			'direct' => array(),
 		);
 
 		$tests = perflab_effective_asset_cache_headers_add_test( $tests );
 
-		$this->assertArrayNotHasKey( 'effective_asset_cache_headers', $tests['direct'] );
+		if ( 'plugin' === $wp_development_mode ) {
+			// If in development mode, the test should not be added.
+			$this->assertArrayNotHasKey( 'effective_asset_cache_headers', $tests['direct'] );
+		} else {
+			$this->assertArrayHasKey( 'effective_asset_cache_headers', $tests['direct'] );
+			$this->assertEquals( 'Effective Caching Headers', $tests['direct']['effective_asset_cache_headers']['label'] );
+			$this->assertEquals( 'perflab_effective_asset_cache_headers_assets_test', $tests['direct']['effective_asset_cache_headers']['test'] );
+		}
+	}
+
+	/**
+	 * Data provider for test_perflab_effective_asset_cache_headers_add_test.
+	 *
+	 * @return array<array<string>> Data provider.
+	 */
+	public function data_provider_test_perflab_effective_asset_cache_headers_add_test(): array {
+		return array(
+			array( 'non_development_mode' => '' ),
+			array( 'development_mode' => 'plugin' ),
+		);
 	}
 
 	/**

Oh, I wasn't aware the constant was already defined up front. That does complicate things. Would it be feasible to add runkit as a dependency for running this test and if so utilize https://www.php.net/manual/en/function.runkit7-constant-remove.php to remove the constant? If runkit isn't available then the test could be skipped.

Hi @westonruter,

Adding runkit as a dependency seems a bit heavy for this specific use case.

There are a few technical challenges:

• The wp-env Docker environment doesn’t include runkit by default, so we’d need custom Docker setup.
• We’d have to ensure it works reliably across all our GitHub Actions workflows and testing environments.
• Since PHP 8 support in runkit7 is still incomplete and experimental, it doesn’t make sense to add it right now.
• Installing it would require significant changes to our testing infrastructure and a more complicated setup with wp-env.

While runkit could be helpful for testing hard-to-mock code (like constants or native functions), it feels like overkill for this particular PR.

For now, I’d lean toward removing the test or using wp_get_development_mode as I suggested in the #2035 (comment) .

For now, I’d lean toward removing the test

@b1ink0 Thanks for that feedback.

So removing test_perflab_effective_asset_cache_headers_add_test_is_attached_to_site_status_tests? That seems fine to me. As long as we are keeping the actual tests for perflab_effective_asset_cache_headers_assets_test() then testing whether or not the test is added is not important.

So removing test_perflab_effective_asset_cache_headers_add_test_is_attached_to_site_status_tests? That seems fine to me. As long as we are keeping the actual tests for perflab_effective_asset_cache_headers_assets_test() then testing whether or not the test is added is not important.

Only the test that checks whether the Site Health test was added needs to be removed. The other tests are not affected by this change, as it calls the functions directly to test them and doesn't rely on the filter.