Skip to content

Commit

Permalink
feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
paulirish committed Apr 29, 2017
1 parent f0481df commit bfc1e62
Show file tree
Hide file tree
Showing 5 changed files with 10 additions and 38 deletions.
1 change: 1 addition & 0 deletions lighthouse-cli/test/smokehouse/pwa-config.js
Expand Up @@ -38,6 +38,7 @@ module.exports = {
]
},
{
passName: 'domstats',
gatherers: [
'dobetterweb/domstats',
'http-redirect'
Expand Down
7 changes: 3 additions & 4 deletions lighthouse-core/config/config.js
Expand Up @@ -129,13 +129,12 @@ function validatePasses(passes, audits, rootPath) {
});
});

// Passes should have unique passName's. Warn otherwise.
// Passes should have unique `passName`s. Warn otherwise.
const usedNames = new Set();
passes.forEach((pass, index) => {
passes.forEach(pass => {
const passName = pass.passName || Audit.DEFAULT_PASS;
if (usedNames.has(passName)) {
log.warn('config', `passes[${index}] may overwrite trace ` +
` of earlier pass without a unique passName (repeated name: ${passName}.`);
throw new Error(`Passes must have unique names (repeated passName: ${passName}.`);
}
usedNames.add(passName);
});
Expand Down
1 change: 0 additions & 1 deletion lighthouse-core/config/plots.json
@@ -1,6 +1,5 @@
{
"passes": [{
"recordNetwork": true,
"recordTrace": true,
"pauseBeforeTraceEndMs": 5000,
"useThrottling": true,
Expand Down
8 changes: 3 additions & 5 deletions lighthouse-core/gather/gather-runner.js
Expand Up @@ -85,7 +85,7 @@ class GatherRunner {
return Promise.resolve()
// Begin tracing only if requested by config.
.then(_ => options.config.recordTrace && driver.beginTrace(options.flags))
// Network is always recorded for internal use, even if not saved as artifact.
// Network is always recorded for gatherer use
.then(_ => driver.beginNetworkCollect(options))
// Navigate.
.then(_ => driver.gotoURL(options.url, {
Expand Down Expand Up @@ -238,7 +238,6 @@ class GatherRunner {
// an object with a traceEvents property. Normalize to object form.
passData.trace = Array.isArray(traceContents) ?
{traceEvents: traceContents} : traceContents;
passData.devtoolsLog = driver.devtoolsLog;
log.verbose('statusEnd', 'Retrieving trace');
});
}
Expand All @@ -249,7 +248,7 @@ class GatherRunner {
return driver.endNetworkCollect();
}).then(networkRecords => {
GatherRunner.assertPageLoaded(options.url, driver, networkRecords);
// expose devtoolsLog & networkRecords to gatherers
// Expose devtoolsLog & networkRecords to gatherers
passData.devtoolsLog = driver.devtoolsLog;
passData.networkRecords = networkRecords;
log.verbose('statusEnd', status);
Expand Down Expand Up @@ -351,8 +350,7 @@ class GatherRunner {
.then(_ => GatherRunner.pass(runOptions, gathererResults))
.then(_ => GatherRunner.afterPass(runOptions, gathererResults))
.then(passData => {
// If requested by config, merge trace and network data for this
// pass into tracingData.
// If requested by config, merge trace -> tracingData
const passName = config.passName || Audit.DEFAULT_PASS;
if (config.recordTrace) {
tracingData.traces[passName] = passData.trace;
Expand Down
31 changes: 3 additions & 28 deletions lighthouse-core/test/config/config-test.js
Expand Up @@ -82,21 +82,7 @@ describe('Config', () => {
audits: []
};

return new Promise((resolve, reject) => {
const warningListener = function(args) {
const warningMsg = args[1];
const isMatch = new RegExp(`overwrite.+${unlikelyPassName}`).test(warningMsg);
if (isMatch) {
log.events.removeListener('warning', warningListener);
resolve();
} else {
reject(isMatch);
}
};
log.events.addListener('warning', warningListener);

const _ = new Config(configJson);
});
assert.throws(_ => new Config(configJson), /unique/);
});

it('warns when traced twice with no passNames specified', () => {
Expand All @@ -109,18 +95,7 @@ describe('Config', () => {
audits: []
};

return new Promise((resolve, reject) => {
const warningListener = function(args) {
const warningMsg = args[1];
if (new RegExp(`overwrite.+${Audit.DEFAULT_PASS}`).test(warningMsg)) {
log.events.removeListener('warning', warningListener);
resolve();
}
};
log.events.addListener('warning', warningListener);

const _ = new Config(configJson);
});
assert.throws(_ => new Config(configJson), /unique/);
});

it('throws for unknown gatherers', () => {
Expand Down Expand Up @@ -298,7 +273,7 @@ describe('Config', () => {
},
passes: [
{recordTrace: true, gatherers: []},
{gatherers: ['accessibility']},
{passName: 'a11y', gatherers: ['accessibility']},
],
audits: [
'accessibility/color-contrast',
Expand Down

0 comments on commit bfc1e62

Please sign in to comment.