tests(smoke): assert errorCode in expectations #6851
Conversation
connorjclark
requested review from
brendankenny and
patrickhulce
as code owners
| @@ -274,11 +280,12 @@ function reportAssertion(assertion) { | |||
| /** | |||
| * Log all the comparisons between actual and expected test results, then print | |||
| * summary. Returns count of passed and failed tests. | |||
| * @param {{finalUrl: !Object, audits: !Array<!Object>}} results | |||
| * @param {{finalUrl: !Object, audits: !Array<!Object>, errorCode: !Object}} results | |||
There was a problem hiding this comment.
FWIW, we don't need all these ! anymore, so it'd be cool to be cleaning them up as we go if we can, but I don't think any of the comments are real atm
| @@ -199,8 +199,8 @@ function findDifference(path, actual, expected) { | |||
|
|
|||
| /** | |||
| * Collate results into comparisons of actual and expected scores on each audit. | |||
| * @param {{finalUrl: string, audits: !Array}} actual | |||
| * @param {{finalUrl: string, audits: !Array}} expected | |||
| * @param {{finalUrl: string, audits: !Array, errorCode: string}} actual | |||
There was a problem hiding this comment.
| * @param {{finalUrl: string, audits: !Array, errorCode: string}} actual | |
| * @param {{finalUrl: string, audits: Array, errorCode?: string}} actual |
There was a problem hiding this comment.
I don't think these files are actually type checked though, audits isn't an Array...
There was a problem hiding this comment.
Hah, it certainly isn't. Opened #6852.
brendankenny
changed the title
There was a problem hiding this comment.
LGTM2
All the annotations are super ancient and were for Closure, and definitely weren't updated as the LHR took form. We can address all in one go for #6852
|
we should also be asserting the LHR form rather than the audit results and a few properties moved to the top level, but also for another day :) |
errorCodewasn't being asserted for smoke tests. with this code, it will be. fin.only effects the
errorssmoke tests