Skip to content
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.

Sign up for GitHub

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

Jump to bottom

tests(smokehouse): fail on finalUrl/errorCode mismatches #7227

Merged
merged 6 commits into from
Feb 14, 2019
Merged

tests(smokehouse): fail on finalUrl/errorCode mismatches #7227

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
50f7a9c
smokehouse: count failedUrl/errorCode mismatches (#7225)
connorjclark Feb 12, 2019
c377eb0
show all assertions regardless of pass/fail state
connorjclark Feb 12, 2019
97ab34d
verbose env flag
connorjclark Feb 12, 2019
255bfbc
rename debug env, use Boolean
connorjclark Feb 13, 2019
72b2219
isPlainObject
connorjclark Feb 13, 2019
ab4eb40
Update lighthouse-cli/test/smokehouse/smokehouse.js
brendankenny Feb 14, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions lighthouse-cli/test/smokehouse/smokehouse.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,11 +311,23 @@ function reportAssertion(assertion) {
* @return {{passed: number, failed: number}}
*/
function report(results) {
let correctCount = 0;
let failedCount = 0;

reportAssertion(results.finalUrl);
if (results.finalUrl.equal) {
correctCount++;
} else {
failedCount++;
}

reportAssertion(results.errorCode);
if (results.errorCode.equal) {
correctCount++;
} else {
failedCount++;
}

let correctCount = 0;
let failedCount = 0;
results.audits.forEach(auditAssertion => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
results.audits.forEach(auditAssertion => {
[results.finalUrl, results.errorCode, ...results.audits].forEach(auditAssertion => {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could do this instead and drop the two reportAssertions above.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice

do we want to continue showing green ticks for passing? this is what it looks like with those changes + moving reportAssertion out of the if statement:

image

Keeping reportAssertion where it is-

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i like this best

image

if (auditAssertion.equal) {
correctCount++;
Expand Down