Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
new_audit: bf-cache #14465
new_audit: bf-cache #14465
Changes from 43 commits
69b34b6
648f72e
c25b196
29352f2
b89df49
111abd4
d2db291
7a669a0
795593f
4e4138a
e4aa601
8aac07f
892eb53
14dfafe
eacc071
c74bcad
71bd779
f1dd41a
fc67719
c866560
005eabd
b36a3d4
209995c
c959bd5
92b9c99
3afa9c2
f320249
cf751ef
d74cb5e
3bc53b1
47f8a46
eb2f032
442c231
b6aba19
27b096e
f1122e3
6a58cbd
c245ee6
1e453a7
919e51f
be1cb7c
a2c428a
fe5eacc
9f7c944
99902fd
5354cbe
922bf49
99aa1bc
3e489dc
5168bcf
80f5672
c011ba0
e528638
4caf208
b389d38
21b2347
59d2aa5
224e675
9f88317
76a16d8
8677704
9b130b9
50fcdba
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Maybe we should have a test-definition base config we extend from? we could also specify
only-categories: []
in it, forcing smoke configs that extend it to be more specific (I assume that works...)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.
Seems like overkill plus it would be confusing when writing a test to bounce between the smokehouse base config and the test specific config.
This issue is only present on a handful of tests.
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.
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.
Oh, I actually missed this. This shouldn't pass even if it's not actionable...it sucks for the user, but we really don't want to give the impression that your page is good for bfcache (at least on load) when it's not.
Lots of failing bfcache audits in the wild is also good motivation to fix non-actionable reasons or at least transform them into actionable ones.
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.
Another option would be to issue an audit-level warning if there are only non-actionable reasons.
Additionally, if we go with your suggestion we should circle back to @tunetheweb and @connorjclark's ideas for the audit title.
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 what that could look like:
Unfortunately the user still has to expand the passed audit dropdown to see it. Maybe we could add a "Passed with warning" section to performance like other categories?
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.
I think it should fail. It didn’t pass - it doesn’t matter that it was for an inactionable reason.