Fix unpredictable LCP element being identified in a URL Metric Group #1903
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## trunk #1903 +/- ##
==========================================
+ Coverage 69.59% 69.63% +0.03%
==========================================
Files 86 86
Lines 6983 6991 +8
==========================================
+ Hits 4860 4868 +8
Misses 2123 2123
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
e891fd0 to
f1e39a7
Compare
|
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
| // Prefer to use URL Metrics which have a current ETag. | ||
| $url_metrics = array_filter( | ||
| $this->url_metrics, | ||
| function ( OD_URL_Metric $url_metric ): bool { | ||
| return $url_metric->get_etag() === $this->get_collection()->get_current_etag(); | ||
| } | ||
| ); | ||
|
|
||
| // Otherwise, if no URL Metrics have a current ETag, fall back to using all the stale ones. | ||
| if ( count( $url_metrics ) === 0 ) { | ||
| $url_metrics = $this->url_metrics; | ||
| } | ||
|
|
||
| foreach ( $url_metrics as $url_metric ) { |
There was a problem hiding this comment.
This is the only actual plugin code change. Everything else is for tests.
There was a problem hiding this comment.
@westonruter This mostly looks good, just two points of feedback.
plugins/optimization-detective/tests/test-class-od-url-metrics-group.php
Outdated
Show resolved
Hide resolved
…ot messages" This reverts commit f1e39a7.
Addresses oversight from WordPress#1903
Addresses oversight from #1903
Summary
Fixes #1896
See #1902
Relevant technical choices
Now
\OD_URL_Metric_Group::get_lcp_element()will prefer to use URL Metrics which have a current ETag. If there are none, then it falls back to using all of the stale URL Metrics. This ensures that if a URL Metric group has two URL Metrics, one with a mismatched ETag and another with an ETag that matches the current ETag, that the logic will not possibly result in the LCP element being pulled from the URL Metric with the stale ETag, which can currently happen if there are an equal number of stale and fresh URL Metrics in a group.