Skip to content

Commit

Permalink
fix unhandled promise rejections in runner testing
Browse files Browse the repository at this point in the history
make sure runner always returns a promise, and add a check that config.auditResults is an array to make this easier to debug in the future
  • Loading branch information
brendankenny committed Oct 4, 2016
1 parent 3fa3690 commit e04fec6
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 14 deletions.
4 changes: 4 additions & 0 deletions lighthouse-core/config/config.js
Expand Up @@ -294,6 +294,10 @@ class Config {

this._passes = configJSON.passes || null;
this._auditResults = configJSON.auditResults || null;
if (this._auditResults && !Array.isArray(this._auditResults)) {
throw new Error('config.auditResults must be an array');
}

this._aggregations = configJSON.aggregations || null;

this._audits = requireAudits(configJSON.audits, this._configDir);
Expand Down
3 changes: 2 additions & 1 deletion lighthouse-core/runner.js
Expand Up @@ -110,8 +110,9 @@ class Runner {
// If there are existing audit results, surface those here.
run = run.then(_ => config.auditResults);
} else {
throw new Error(
const err = Error(
'The config must provide passes and audits, artifacts and audits, or auditResults');
return Promise.reject(err);
}

// Format and aggregate results before returning.
Expand Down
47 changes: 34 additions & 13 deletions lighthouse-core/test/runner-test.js
Expand Up @@ -41,16 +41,20 @@ describe('Runner', () => {
});
});

it('throws when given neither passes nor artifacts', () => {
it('rejects when given neither passes nor artifacts', () => {
const url = 'https://example.com';
const config = new Config({
audits: [
'is-on-https'
]
});

return assert.throws(_ => Runner.run(fakeDriver, {url, config}),
/The config must provide passes/);
return Runner.run(fakeDriver, {url, config})
.then(_ => {
assert.ok(false);
}, err => {
assert.ok(/The config must provide passes/.test(err.message));
});
});

it('accepts existing artifacts', () => {
Expand All @@ -61,11 +65,17 @@ describe('Runner', () => {
],

artifacts: {
HTTPS: true
HTTPS: {
value: true
}
}
});

return assert.doesNotThrow(_ => Runner.run({}, {url, config}));
return Runner.run({}, {url, config}).then(results => {
// Mostly checking that this did not throw, but check representative values.
assert.equal(results.initialUrl, url);
assert.strictEqual(results.audits['is-on-https'].rawValue, true);
});
});

it('accepts trace artifacts as paths and outputs appropriate data', () => {
Expand Down Expand Up @@ -126,24 +136,31 @@ describe('Runner', () => {
});
});

it('throws when given neither audits nor auditResults', () => {
it('rejects when given neither audits nor auditResults', () => {
const url = 'https://example.com';
const config = new Config({
passes: [{
gatherers: ['https']
}]
});

return assert.throws(_ => Runner.run(fakeDriver, {url, config}),
/The config must provide passes/);
return Runner.run(fakeDriver, {url, config})
.then(_ => {
assert.ok(false);
}, err => {
assert.ok(/The config must provide passes/.test(err.message));
});
});

it('accepts existing auditResults', () => {
const url = 'https://example.com';
const config = new Config({
auditResults: {
HTTPS: true
},
auditResults: [{
name: 'is-on-https',
rawValue: true,
score: true,
displayValue: ''
}],

aggregations: [{
name: 'Aggregation',
Expand All @@ -155,15 +172,19 @@ describe('Runner', () => {
description: 'description',
audits: {
'is-on-https': {
value: true,
expectedValue: true,
weight: 1
}
}
}]
}]
});

return assert.doesNotThrow(_ => Runner.run(fakeDriver, {url, config}));
return Runner.run(fakeDriver, {url, config}).then(results => {
// Mostly checking that this did not throw, but check representative values.
assert.equal(results.initialUrl, url);
assert.strictEqual(results.audits['is-on-https'].rawValue, true);
});
});

it('returns an aggregation', () => {
Expand Down

0 comments on commit e04fec6

Please sign in to comment.