-
Notifications
You must be signed in to change notification settings - Fork 2
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
[Feature] Unit tests for the Full-Page Caching for 404s feature #76
[Feature] Unit tests for the Full-Page Caching for 404s feature #76
Conversation
…ractive/wp-alleyvate into feature/9/caching-404s
Looks like the php-cs-fixer doesn't like the |
I made it work but it seems we have conflicting tools here asking for different rules: |
Getting the object-cache to work here is proving more complicated than expected. Moving on as it is for now while I search for better support. Probably changing https://github.com/alleyinteractive/.github/blob/main/.github/workflows/php-tests.yml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few suggestions, but no blockers - good addition to the test suite 🍣
$url = home_url( self::TEMPLATE_GENERATOR_URI, 'https' ); | ||
|
||
// Replace http with https to ensure the styles don't get blocked due to insecure content. | ||
$url = str_replace( 'http://', 'https://', $url ); | ||
|
||
// This request will populate the cache using output buffering. | ||
if ( function_exists( 'wpcom_vip_file_get_contents' ) ) { | ||
if ( \function_exists( 'wpcom_vip_file_get_contents' ) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is the slash being added here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is being added by the php-cs-fixer. Not sure yet why.
Summary
As titled.
Notes for reviewers
See #46