Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Omit page caching and object caching tests for Site Health if on WP>=6.1 #7643

Closed
milindmore22 opened this issue Oct 23, 2023 · 7 comments · Fixed by #7645
Closed

Omit page caching and object caching tests for Site Health if on WP>=6.1 #7643

milindmore22 opened this issue Oct 23, 2023 · 7 comments · Fixed by #7645
Assignees
Labels
Changelogged Whether the issue/PR has been added to release notes. Tech Debt Deprecations, inefficiencies, code health
Milestone

Comments

@milindmore22
Copy link
Collaborator

milindmore22 commented Oct 23, 2023

Feature Description

New description from @westonruter: The Site Health tests for page caching and object caching from the AMP plugin have been incorporated into WordPress core as of 6.1. See WordPress/performance#220 (Core-56041) and WordPress/performance#35 (Core-56040). At the very least these tests can be disabled in WP 6.1+. But I think it would be better to just remove them entirely.

Original description

We got a support topic where the user is getting a site health a warning for cache plugin, although he is using a cache plugin (Litespeed cache) because LiteSpeed uses a different header it's not able to detect it.

The Litespeed author suggested that we use a filter so that other vendors can pass their headers so it will fix warnings for their plugins.

Acceptance Criteria

No response

Implementation Brief

No response

QA Testing Instructions

No response

Demo

No response

Changelog Entry

No response

@milindmore22 milindmore22 added the Enhancement New feature or improvement of an existing one label Oct 23, 2023
@westonruter
Copy link
Member

Actually, since page caching detection is now part of Site Health in WordPress core, I think we can just remove our check entirely now, right?

There is a filter that caching plugins can use to add recognition of their own headers, as noted in the support topic.

@westonruter westonruter added this to the v2.4.3 milestone Oct 23, 2023
@westonruter westonruter added Tech Debt Deprecations, inefficiencies, code health and removed Enhancement New feature or improvement of an existing one labels Oct 23, 2023
@westonruter westonruter changed the title Introduce filter for Site health cache header detection notice. Remove page caching and object caching tests for Site Health Oct 23, 2023
@westonruter
Copy link
Member

For the impending release, we could just disable the tests by default. Then in the next release we could remove them entirely, if this would make it easier to get the release out the door.

@thelovekesh
Copy link
Collaborator

We are also using page cache tests internally.

$page_cache_detail = $this->site_health->get_page_cache_detail( true );

@westonruter
Copy link
Member

Presumably these should be using what core has now for WP>=6.1.

@thelovekesh
Copy link
Collaborator

I need to dig in more but seems like it could be tricky to determine due to the visibility of methods.

https://github.com/WordPress/wordpress-develop/blob/ce342f1a33051dcafdb47132f8e07d0597f49124/src/wp-admin/includes/class-wp-site-health.php#L3458

@westonruter
Copy link
Member

Ah yes. Well, in that case, we can just add a condition to only include the following tests if WP<6.1:

$tests['direct']['amp_persistent_object_cache'] = [
'label' => esc_html__( 'Persistent object cache', 'amp' ),
'test' => [ $this, 'persistent_object_cache' ],
];

if ( $this->supports_async_rest_tests( $tests ) ) {
$tests['async'][ self::TEST_PAGE_CACHING ] = [
'label' => esc_html__( 'Page caching', 'amp' ),
'test' => rest_url( self::REST_API_NAMESPACE . self::REST_API_PAGE_CACHE_ENDPOINT ),
'has_rest' => true,
'async_direct_test' => [ $this, 'page_cache' ],
];
}

The latter essentially already has such a test, but for WP 5.6.

@westonruter westonruter changed the title Remove page caching and object caching tests for Site Health Omit page caching and object caching tests for Site Health if on WP>=6.1 Oct 24, 2023
@pavanpatil1
Copy link

QA Passed ✅

Cross-verified the issue and the page caching and object caching test notices are not visible. Now, it is only visible if WP version is less than 6.1.

Before fix After fix
image image

@westonruter westonruter added the Changelogged Whether the issue/PR has been added to release notes. label Nov 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelogged Whether the issue/PR has been added to release notes. Tech Debt Deprecations, inefficiencies, code health
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants