tests(lightwallet): add perf-budget smoke test #8853
Conversation
|
cc @khempenius |
| displayValue: '11 requests • 164 KB', | ||
| details: { | ||
| items: [ | ||
| {resourceType: 'total', requestCount: 11, size: '168000±1000'}, |
There was a problem hiding this comment.
used ± to leave some leeway on some of the assets because most of them are shared with other smoke tests. This will allow the test to absorb changes to them of a few hundred bytes or so, but we can always make it exact.
| { | ||
| resourceType: 'image', | ||
| countOverBudget: '1 request', | ||
| sizeOverBudget: undefined, |
There was a problem hiding this comment.
Letting undefined values in the expectations match not defined properties in actual allows testing for things like sizeOverBudget when not over budget, when its value is set to undefined and so it lost in the JSON shuffle.
If we just left this sizeOverBudget off the expectations it would indeed match an actual undefined value, which is good...but it would also match if actual was erroneously returning a numeric sizeOverBudget, so it would be useless.
There was a problem hiding this comment.
should we add a comment somewhere to this effect?
There was a problem hiding this comment.
should we add a comment somewhere to this effect?
added a comment to make it clear to anyone reading the expectations file
| { | ||
| resourceType: 'image', | ||
| countOverBudget: '1 request', | ||
| sizeOverBudget: undefined, |
There was a problem hiding this comment.
should we add a comment somewhere to this effect?
| throttlingMethod: 'devtools', | ||
| onlyCategories: ['performance'], | ||
|
|
||
| // A mixture of under, over, and meeting budget to exercise all paths. |
There was a problem hiding this comment.
maybe include the path to the file where these budgets are being tested so we can look them up for correctness later? it's a bit weird that our "expectations" are basically here in the config that's shared with several other smokes
There was a problem hiding this comment.
it's a bit weird that our "expectations" are basically here in the config
I mean, that's kind of what all configs are :)
that's shared with several other smokes
I could split it into a new smoke budget category? Or is it fine to leave as one big perf category
| {resourceType: 'other', budget: 2}, // meets budget | ||
| {resourceType: 'third-party', budget: 0}, | ||
| ], | ||
| timings: [ |
There was a problem hiding this comment.
these aren't being used right now, correct?
There was a problem hiding this comment.
these aren't being used right now, correct?
right, they're parsed by budget.js, but the audit doesn't check them yet
* tests(lightwallet): add perf-budget smoke test * add undefined comment
added a smoke test for the
resource-summary(#8522) andperformance-budget(#8539) audits.This just rips off some things from the two other perf testing pages so we could get a decent mix of resources loaded. Feel free to make suggestions for improvements :)
Three other things done in the process:
perfsmoke test to its own config file (off of--preset=perf) so that it could include a budgetundefinedin smoke expectations match with a property not found on theactualresult. Previously the matchingactualproperty had to explicitly be set toundefinedto match, which isn't even possible because smoke test results are saved/loaded from JSON, whereundefinedproperties get dropped.'±'as an alias for'+/-'as one of the allowed operators, cause it's cooler :)part of #8331