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.

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

convert DoBetterWeb gatherers to new error system #1641

Merged
merged 7 commits into from
Feb 6, 2017
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,6 @@ class LinkBlockingFirstPaintAudit extends Audit {
*/
static computeAuditResultForTags(artifacts, tagFilter) {
const artifact = artifacts.TagsBlockingFirstPaint;
if (artifact.value === -1) {
return {
rawValue: -1,
debugString: artifact.debugString
};
}

const filtered = artifact.filter(item => item.tag.tagName === tagFilter);

Expand Down
4 changes: 0 additions & 4 deletions lighthouse-core/audits/dobetterweb/uses-optimized-images.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,6 @@ class UsesOptimizedImages extends Audit {
static audit_(artifacts, networkThroughput) {
const images = artifacts.OptimizedImages;

if (images.rawValue === -1) {
return UsesOptimizedImages.generateAuditResult(images);
}

const failedImages = [];
let totalWastedBytes = 0;
let hasAllEfficientImages = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,11 @@ class OptimizedImages extends Gatherer {
});
}

/**
* @param {!Object} options
* @param {{networkRecords: !Array<!NetworRecord>}} traceData
* @return {!Promise<!Array<!Object>}
*/
afterPass(options, traceData) {
const networkRecords = traceData.networkRecords;
const imageRecords = OptimizedImages.filterImageRequests(options.url, networkRecords);
Expand All @@ -131,8 +136,6 @@ class OptimizedImages extends Gatherer {
}

return results;
}).catch(err => {
return {rawValue: -1, debugString: err};
});
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,15 +147,14 @@ class TagsBlockingFirstPaint extends Gatherer {
return options.driver.evaluateScriptOnLoad(scriptSrc);
}

/**
* @param {!Object} options
* @param {{networkRecords: !Array<!NetworkRecord>}} tracingData
Copy link
Contributor

Choose a reason for hiding this comment

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

{!{networkRecords: !Array<!NetworkRecord>}}?

Copy link
Member Author

Choose a reason for hiding this comment

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

record types like this are non-nullable, so the ! is technically correct but unnecessary. It would have been really nice if everything had been non-nullable by default from the start :(

* @return {!Array<{tag: string, transferSize: number, startTime: number, endTime: number}>}
*/
afterPass(options, tracingData) {
return TagsBlockingFirstPaint
.findBlockingTags(options.driver, tracingData.networkRecords)
.catch(err => {
return {
value: -1,
debugString: err.message
};
});
.findBlockingTags(options.driver, tracingData.networkRecords);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,6 @@ const assert = require('assert');
/* eslint-env mocha */

describe('Link Block First Paint audit', () => {
it('fails when error input present', () => {
const debugString = 'the uniquest debug string';
const auditResult = LinkBlockingFirstPaintAudit.audit({
TagsBlockingFirstPaint: {
value: -1,
debugString
}
});
assert.equal(auditResult.rawValue, -1);
assert.strictEqual(auditResult.debugString, debugString);
});

it('fails when there are links found which block first paint', () => {
const linkDetails = {
tagName: 'LINK',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,6 @@ const assert = require('assert');
/* eslint-env mocha */

describe('Script Block First Paint audit', () => {
it('fails when error input present', () => {
const debugString = 'first paint debug string';
const auditResult = ScriptBlockingFirstPaintAudit.audit({
TagsBlockingFirstPaint: {
value: -1,
debugString
}
});
assert.equal(auditResult.rawValue, -1);
assert.strictEqual(auditResult.debugString, debugString);
});

it('fails when there are scripts found which block first paint', () => {
const scriptDetails = {
tagName: 'SCRIPT',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,18 +37,6 @@ function generateImage(type, originalSize, webpSize, jpegSize) {
/* eslint-env mocha */

describe('Page uses optimized images', () => {
it('fails when gatherer returns error', () => {
const debugString = 'All image optimizations failed.';
const auditResult = UsesOptimizedImagesAudit.audit_({
OptimizedImages: {
rawValue: -1,
debugString: debugString
},
});
assert.equal(auditResult.rawValue, -1);
assert.equal(auditResult.debugString, debugString);
});

it('fails when one jpeg image is unoptimized', () => {
const auditResult = UsesOptimizedImagesAudit.audit_({
OptimizedImages: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,12 +126,4 @@ describe('Optimized images', () => {
assert.ok(/whoops/.test(failed.err.message), 'passed along error message');
});
});

it('handles total driver failure', () => {
options.driver.evaluateAsync = () => Promise.reject(new Error('fatal'));
return optimizedImages.afterPass(options, traceData).then(artifact => {
assert.equal(artifact.rawValue, -1);
assert.ok(artifact.debugString);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -155,17 +155,4 @@ describe('First paint blocking tags', () => {
assert.deepEqual(artifact, expected);
});
});

it('handles driver failure', () => {
return tagsBlockingFirstPaint.afterPass({
driver: {
evaluateAsync() {
return Promise.reject(new Error('such a fail'));
}
}
}, traceData).then(artifact => {
assert.equal(artifact.value, -1);
assert.ok(artifact.debugString);
});
});
});