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 4 commits
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
20 changes: 13 additions & 7 deletions lighthouse-cli/test/smokehouse/smokehouse.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ const PAGE_HUNG_EXIT_CODE = 68;
const INSECURE_DOCUMENT_REQUEST_EXIT_CODE = 69;
const RETRIES = 3;
const NUMERICAL_EXPECTATION_REGEXP = /^(<=?|>=?)((\d|\.)+)$/;
const VERBOSE = Boolean(process.env.LH_SMOKE_VERBOSE);

/**
* Attempt to resolve a path locally. If this fails, attempts to locate the path
Expand Down Expand Up @@ -62,7 +63,7 @@ function resolveLocalOrCwd(payloadPath) {
* @return {ExpectedLHR}
*/
function runLighthouse(url, configPath, isDebug) {
isDebug = isDebug || Boolean(process.env.SMOKEHOUSE_DEBUG);
isDebug = isDebug || Boolean(process.env.LH_SMOKE_DEBUG);
Copy link
Member

Choose a reason for hiding this comment

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

doesn't debug also save the LHR? Up to y'all if you want that :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

nah. just writing to disk is good enough for me. verbose shouldn't be too verbose


const command = 'node';
const outputPath = `smokehouse-${Math.round(Math.random() * 100000)}.report.json`;
Expand Down Expand Up @@ -276,8 +277,12 @@ function reportAssertion(assertion) {
RegExp.prototype.toJSON = RegExp.prototype.toString;

if (assertion.equal) {
console.log(` ${log.greenify(log.tick)} ${assertion.category}: ` +
log.greenify(assertion.actual));
if (assertion.actual !== null && typeof assertion.actual === 'object') {
Copy link
Collaborator

Choose a reason for hiding this comment

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

super nit but mind flipping the order of these to is an object && is not null? took me a bit to realize this was just an object check

Copy link
Member

Choose a reason for hiding this comment

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

can we just use a helper method like so?

const isPlainObject = function (obj) {
	return Object.prototype.toString.call(obj) === '[object Object]';
};

Copy link
Member

Choose a reason for hiding this comment

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

Object.prototype.toString.call(obj) === '[object Object]';

what year is it?

Copy link
Member

Choose a reason for hiding this comment

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

:P

console.log(` ${log.greenify(log.tick)} ${assertion.category}`);
} else {
console.log(` ${log.greenify(log.tick)} ${assertion.category}: ` +
log.greenify(assertion.actual));
}
} else {
if (assertion.diff) {
const diff = assertion.diff;
Expand Down Expand Up @@ -311,16 +316,17 @@ function reportAssertion(assertion) {
* @return {{passed: number, failed: number}}
*/
function report(results) {
reportAssertion(results.finalUrl);
reportAssertion(results.errorCode);

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

[results.finalUrl, results.errorCode, ...results.audits].forEach(auditAssertion => {
if (auditAssertion.equal) {
correctCount++;
} else {
failedCount++;
}

if (!auditAssertion.equal || VERBOSE) {
reportAssertion(auditAssertion);
}
});
Expand Down