Skip to content

Conversation

@westonruter
Copy link
Member

I was testing Optimization Detective and I was confused why it wasn't working. Pages which normally had been optimized were being served as if Optimization Detective wasn't even active. It turns out that this is a side effect of #1762 where we prevent any page processing when the REST API for storing URL Metrics is unavailable. Since loopback requests don't work in wp-env (WordPress/gutenberg#20569), neither does the Site Health test for checking whether the REST API is available.

Without the patch in this PR, a plugin can force optimization to apply even when the REST API isn't available via code like the following:

// The wp-env environment does not support loopback requests, so we need to pretend that everything is good.
add_filter(
	'pre_option_od_rest_api_unavailable',
	static function ( $pre ): string {
		if ( ! is_admin() ) {
			return '0';
		}
		return $pre;
	}
);

This could be added to the od-dev-mode plugin. But this plugin shouldn't have to be installed in order to test Optimization Detective in wp-env.

So this PR adds a condition (a7cedce) so that od_is_rest_api_unavailable() is checked alongside ( wp_get_environment_type() !== 'local' || class_exists( 'Test_OD_Optimization' ) ). Only if both are true will it short-circuit.

Additionally, to assist with helping users understand why Optimization Detective isn't doing anything, this PR includes the reasons for optimization being disabled in the console when WP_DEBUG is enabled.

@github-actions
Copy link

github-actions bot commented Jan 23, 2025

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: westonruter <[email protected]>
Co-authored-by: felixarntz <[email protected]>

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

@westonruter westonruter added the [Plugin] Optimization Detective Issues for the Optimization Detective plugin label Jan 23, 2025
@westonruter westonruter added the [Type] Bug An existing feature is broken label Jan 23, 2025
'reason' => __( 'Page is not optimized because od_can_optimize_response() returned false. This can be overridden with the od_can_optimize_response filter.', 'optimization-detective' ),
),
array(
'test' => ! ( od_is_rest_api_unavailable() && ( wp_get_environment_type() !== 'local' || class_exists( 'Test_OD_Optimization' ) ) ),
Copy link
Member Author

Choose a reason for hiding this comment

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

I wish the class_exists( 'Test_OD_Optimization' ) check weren't needed, but there's apparently no way to override the return value of wp_get_environment_type() in tests.

Copy link
Member

Choose a reason for hiding this comment

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

Not a blocker, but is there a more generic way to check if tests are being run? Maybe a constant that the WP core unit test suite always sets?

}
}
if ( count( $reasons ) > 0 ) {
if ( WP_DEBUG ) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Test coverage for this condition can't be added due to #1271.

@codecov
Copy link

codecov bot commented Jan 23, 2025

Codecov Report

Attention: Patch coverage is 82.35294% with 6 lines in your changes missing coverage. Please review.

Project coverage is 65.96%. Comparing base (b7c4475) to head (e6135b8).
Report is 29 commits behind head on trunk.

Files with missing lines Patch % Lines
plugins/optimization-detective/optimization.php 82.35% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##            trunk    #1822      +/-   ##
==========================================
+ Coverage   65.47%   65.96%   +0.49%     
==========================================
  Files          85       88       +3     
  Lines        6748     6876     +128     
==========================================
+ Hits         4418     4536     +118     
- Misses       2330     2340      +10     
Flag Coverage Δ
multisite 65.96% <82.35%> (+0.49%) ⬆️
single 38.64% <82.35%> (+0.76%) ⬆️

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.

@westonruter westonruter force-pushed the fix/short-circuit-rest-api-unavailable branch from 0cb53df to e9c0618 Compare January 23, 2025 22:39
Copy link
Member

@felixarntz felixarntz left a comment

Choose a reason for hiding this comment

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

@westonruter LGTM, just two minor notes.

'reason' => __( 'Page is not optimized because od_can_optimize_response() returned false. This can be overridden with the od_can_optimize_response filter.', 'optimization-detective' ),
),
array(
'test' => ! ( od_is_rest_api_unavailable() && ( wp_get_environment_type() !== 'local' || class_exists( 'Test_OD_Optimization' ) ) ),
Copy link
Member

Choose a reason for hiding this comment

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

Not a blocker, but is there a more generic way to check if tests are being run? Maybe a constant that the WP core unit test suite always sets?

Co-authored-by: Felix Arntz <[email protected]>
@westonruter westonruter enabled auto-merge January 24, 2025 01:15
@westonruter westonruter merged commit 0615c18 into trunk Jan 24, 2025
13 checks passed
@westonruter westonruter deleted the fix/short-circuit-rest-api-unavailable branch January 24, 2025 01:15
@westonruter westonruter mentioned this pull request Jan 26, 2025
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Plugin] Optimization Detective Issues for the Optimization Detective plugin [Type] Bug An existing feature is broken

Projects

Status: Done 😃

Development

Successfully merging this pull request may close these issues.

3 participants