Add Site Health check for Full Page Cache usage #263

manuelRod · 2022-03-24T16:53:44Z

Summary

Relevant technical choices

As @westonruter proposed, I've adapted and migrated the full page caching Site Health test from AMP to the performance plugin.

Checklist

PR has either [Focus] or Infrastructure label.
PR has a [Type] label.
PR has a milestone or the no milestone label.

…e-cache

ffPjrZUGXfcxuAj · 2022-03-31T08:10:48Z

please add
'x-aruba-cache' => $cache_hit_callback,
for new aruba hispeed cache service

mxbclang · 2022-04-01T14:54:39Z

@manuelRod Since I'm working on updating the language on the other Site Health reports, I took a look at the copy for this one as well and left some thoughts. Let me know if you have any questions!

manuelRod · 2022-04-04T07:33:08Z

Thanks for that @bethanylang, where I can find those thoughts?

westonruter

Overall looks good to me, but it looked good to me in the AMP plugin when I merged it, so I'm probably not the best to review for merging here.

modules/site-health/audit-full-page-cache/load.php

manuelRod · 2022-04-12T10:37:01Z

please add 'x-aruba-cache' => $cache_hit_callback, for new aruba hispeed cache service

added

felixarntz

@manuelRod Overall the code here looks solid. I left a few minor things for some cleanup, particularly I'd suggest to remove two of the functions that are only used in one place and actually make the code more complex to follow rather than less because of splitting how the $result is generated.

modules/site-health/audit-full-page-cache/helper.php

modules/site-health/audit-full-page-cache/load.php

tests/modules/site-health/audit-full-page-cache/audit-full-page-cache-test.php

modules/site-health/audit-full-page-cache/helper.php

tests/modules/site-health/audit-full-page-cache/audit-full-page-cache-test.php

modules/site-health/audit-full-page-cache/load.php

felixarntz · 2022-04-13T23:50:24Z

@bethanylang Would be great to get your review here specifically on the copy in the Site Health check.

modules/site-health/audit-full-page-cache/load.php

felixarntz

@manuelRod This looks solid and should be close to merge, I mostly have some nit-picks around documentation. Please make sure to update all version references for new code in this PR to n.e.x.t, per our guidelines.

Last but not least, based on #310, please refresh this PR against latest trunk and add the corresponding entries for this new module to the CODEOWNERS file, with at least yourself as the module owner.

modules/site-health/audit-full-page-cache/helper.php

modules/site-health/audit-full-page-cache/load.php

tests/modules/site-health/audit-full-page-cache/audit-full-page-cache-test.php

modules/site-health/audit-full-page-cache/helper.php

.github/CODEOWNERS

felixarntz · 2022-05-10T19:59:33Z

@manuelRod @westonruter Given that this is an entirely new module and the 1.1.0 release is about to be finalized with the release coming next week, I think with the remaining iterations to make this won't give us enough time for testing prior to the release, so I think this would be better to include in the following 1.2.0 release. Let's make sure we finalize the PR soon so that it can be merged into trunk shortly after 1.1.0, to have time for testing and potential further iterations.

akshitsethi · 2022-05-25T11:45:10Z

I tested the module and so far it looks good to me. Multiple scenarios (looback request failed, without cache, with cache plugin) were tested and I've attached the screenshot for each below:

Loopback failure

Without cache

Cache enabled

I'll test some more scenarios and update it over here.

felixarntz

@manuelRod Looks good to me, great stuff!

I left a few minor points on documentation and indentation, please address these. None of these are too critical though, so I'm already going to mark this as approved from my end.

modules/site-health/audit-full-page-cache/load.php

Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>

felixarntz

Great, thanks @manuelRod!

@bethanylang You previously requested changes regarding the copy. Can you please give this another pass and approve once it's good from your end?

mxbclang

Language LGTM!

mxbclang · 2022-06-03T10:54:51Z

@felixarntz @manuelRod Apologies if I was a blocker here; just approved on my side. Looks like this is good to go for 1.2.0! 🥳

felixarntz · 2022-06-03T16:27:53Z

Thanks @bethanylang!

manuelRod · 2022-06-07T12:07:07Z

awesome, thanks guys!

manuelRod added 8 commits March 18, 2022 12:49

First commit, adding amp tests, start refactoring

463486e

some refactoring + coding standards

f0c7beb

refactoring in helper file + wp standards

19609d4

Adding first unit tests

6f8ca76

Merge branch 'release/1.0.0-beta.3' into add/sitehealth-test-full-pag…

c295ace

…e-cache

Adding tests

0a78791

test refactor and bugfix

b9c704f

ups

f6098e7

manuelRod requested a review from audrasjb as a code owner March 24, 2022 16:53

manuelRod marked this pull request as draft March 24, 2022 16:53

manuelRod mentioned this pull request Mar 24, 2022

Add Site Health test for full page caching (advanced cache) #220

Closed

fixing test

fea0974

adamsilverstein requested a review from westonruter March 29, 2022 15:24

manuelRod added 2 commits March 30, 2022 11:17

Fixing array header scenario per AMP approach ampproject/amp-wp#6938

ce59323

fixing multisite test

b548bfb

manuelRod marked this pull request as ready for review March 30, 2022 10:01

westonruter approved these changes Apr 5, 2022

View reviewed changes

manuelRod added 2 commits April 12, 2022 12:31

adding aruba cache

d1fb05a

removing escaping html, its exported to json and will cause problems

30a35b6

manuelRod added [Focus] Site Health [Type] Feature A new feature within an existing module labels Apr 12, 2022

manuelRod added this to the 1.0.0 milestone Apr 12, 2022

felixarntz reviewed Apr 13, 2022

View reviewed changes

felixarntz requested a review from mxbclang April 13, 2022 23:50

mxbclang suggested changes Apr 14, 2022

View reviewed changes

mxbclang added the Needs Review label Apr 19, 2022

felixarntz requested changes May 3, 2022

View reviewed changes

manuelRod added 2 commits May 4, 2022 16:18

Merge branch 'trunk' into add/sitehealth-test-full-page-cache

3f8f505

Adding CODEOWNERS

7a239fa

manuelRod requested a review from JustinyAhin as a code owner May 4, 2022 14:21

manuelRod added 3 commits May 4, 2022 16:24

replacing @SInCE to n.e.x.t

dfad675

moving things around

2e6d1d9

updating loopback error test

fded890

adamsilverstein requested review from felixarntz and adamsilverstein May 10, 2022 15:28

adamsilverstein reviewed May 10, 2022

View reviewed changes

.github/CODEOWNERS Outdated Show resolved Hide resolved

felixarntz modified the milestones: 1.1.0, 1.2.0 May 10, 2022

Adding @westonruter as code owner

a8c3f7f

felixarntz added the [Module] Audit Full Page Cache label May 25, 2022

felixarntz approved these changes May 26, 2022

View reviewed changes

felixarntz requested a review from mxbclang May 26, 2022 20:50

manuelRod and others added 4 commits May 30, 2022 13:14

Update modules/site-health/audit-full-page-cache/load.php

e137e28

Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>

Update modules/site-health/audit-full-page-cache/load.php

6f5eb4a

Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>

Update modules/site-health/audit-full-page-cache/load.php

c1dbc3b

Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>

Update modules/site-health/audit-full-page-cache/load.php

e130163

Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>

felixarntz approved these changes Jun 1, 2022

View reviewed changes

mxbclang approved these changes Jun 3, 2022

View reviewed changes

felixarntz merged commit 6c3e795 into WordPress:trunk Jun 3, 2022

felixarntz changed the title ~~Add Site Health test for full page caching~~ Add Site Health check for Full Page Cache usage Jun 3, 2022

Add Site Health check for Full Page Cache usage #263

Add Site Health check for Full Page Cache usage #263

Uh oh!

Conversation

manuelRod commented Mar 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Relevant technical choices

Checklist

Uh oh!

ffPjrZUGXfcxuAj commented Mar 31, 2022

Uh oh!

mxbclang commented Apr 1, 2022

Uh oh!

manuelRod commented Apr 4, 2022

Uh oh!

westonruter left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

manuelRod commented Apr 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

felixarntz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

felixarntz commented Apr 13, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

felixarntz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

felixarntz commented May 10, 2022

Uh oh!

akshitsethi commented May 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Loopback failure

Without cache

Cache enabled

Uh oh!

felixarntz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

felixarntz left a comment

Choose a reason for hiding this comment

Uh oh!

mxbclang left a comment

Choose a reason for hiding this comment

Uh oh!

mxbclang commented Jun 3, 2022

manuelRod commented Mar 24, 2022 •

edited

Loading

manuelRod commented Apr 12, 2022 •

edited

Loading

akshitsethi commented May 25, 2022 •

edited

Loading