diff --git a/lighthouse-core/audits/audit.js b/lighthouse-core/audits/audit.js index ae7e95bf00cd..cc41d8cd2e30 100644 --- a/lighthouse-core/audits/audit.js +++ b/lighthouse-core/audits/audit.js @@ -33,13 +33,6 @@ class Audit { throw new Error('Audit meta information must be overridden.'); } - /** - * @throws - */ - static audit() { - throw new Error('Audit audit() function must be overridden.'); - } - /** * @param {!AuditResultInput} result * @return {!AuditResult} diff --git a/lighthouse-core/config/index.js b/lighthouse-core/config/index.js index 645c51b71dbb..49b864ef018a 100644 --- a/lighthouse-core/config/index.js +++ b/lighthouse-core/config/index.js @@ -108,7 +108,7 @@ function cleanTrace(trace) { return trace; } -function filterPasses(passes, audits, paths) { +function filterPasses(passes, audits, rootPath) { const requiredGatherers = getGatherersNeededByAudits(audits); // Make sure we only have the gatherers that are needed by the audits @@ -117,12 +117,8 @@ function filterPasses(passes, audits, paths) { const freshPass = Object.assign({}, pass); freshPass.gatherers = freshPass.gatherers.filter(gatherer => { - try { - const GathererClass = GatherRunner.getGathererClass(gatherer, paths); - return requiredGatherers.has(GathererClass.name); - } catch (requireError) { - throw new Error(`Unable to locate gatherer: ${gatherer}`); - } + const GathererClass = GatherRunner.getGathererClass(gatherer, rootPath); + return requiredGatherers.has(GathererClass.name); }); return freshPass; @@ -172,77 +168,80 @@ function filterAudits(audits, auditWhitelist) { return filteredAudits; } -function expandAudits(audits, paths) { - const rootPath = path.join(__dirname, '../../'); +function expandAudits(audits, rootPath) { + const Runner = require('../runner'); return audits.map(audit => { - // Check each path to see if the audit can be located. First match wins. - const AuditClass = paths.reduce((definition, auditPath) => { - // If the definition has already been found, just propagate it. Otherwise try a search - // on the path in this iteration of the loop. - if (definition !== null) { - return definition; - } + // Firstly see if the audit is in Lighthouse itself. + const list = Runner.getAuditList(); + const coreAudit = list.find(a => a === `${audit}.js`); + + // Assume it's a core audit first. + let requirePath = path.resolve(__dirname, `../audits/${audit}`); + let AuditClass; - const requirePath = auditPath.startsWith('/') ? auditPath : path.join(rootPath, auditPath); + // If not, see if it can be found another way. + if (!coreAudit) { + // Firstly try and see if the audit resolves naturally through the usual means. try { - return require(`${requirePath}/${audit}`); + require.resolve(audit); + + // If the above works, update the path to the absolute value provided. + requirePath = audit; } catch (requireError) { - return null; + // If that fails, try and find it relative to any config path provided. + if (rootPath) { + requirePath = path.resolve(rootPath, audit); + } } - }, null); + } + + // Now try and require it in. If this fails then the audit file isn't where we expected it. + try { + AuditClass = require(requirePath); + } catch (requireError) { + AuditClass = null; + } if (!AuditClass) { - throw new Error(`Unable to locate audit: ${audit}`); + throw new Error(`Unable to locate audit: ${audit} (tried at ${requirePath})`); } // Confirm that the audit appears valid. - const auditValidation = validateAudit(AuditClass); - if (!auditValidation.valid) { - const errors = Object.keys(auditValidation) - .reduce((errorList, item) => { - // Ignore the valid property as it's generated from the other items in the object. - if (item === 'valid') { - return errorList; - } - - return errorList + (auditValidation[item] ? '' : `\n - ${item} is missing`); - }, ''); - - throw new Error(`Invalid audit class: ${errors}`); - } + assertValidAudit(audit, AuditClass); return AuditClass; }); } -function validateAudit(auditDefinition) { - const hasAuditMethod = typeof auditDefinition.audit === 'function'; - const hasMeta = typeof auditDefinition.meta === 'object'; - const hasMetaName = hasMeta && typeof auditDefinition.meta.name !== 'undefined'; - const hasMetaCategory = hasMeta && typeof auditDefinition.meta.category !== 'undefined'; - const hasMetaDescription = hasMeta && typeof auditDefinition.meta.description !== 'undefined'; - const hasMetaRequiredArtifacts = hasMeta && Array.isArray(auditDefinition.meta.requiredArtifacts); - const hasGenerateAuditResult = typeof auditDefinition.generateAuditResult === 'function'; - - return { - 'valid': ( - hasAuditMethod && - hasMeta && - hasMetaName && - hasMetaCategory && - hasMetaDescription && - hasMetaRequiredArtifacts && - hasGenerateAuditResult - ), - 'audit()': hasAuditMethod, - 'meta property': hasMeta, - 'meta.name property': hasMetaName, - 'meta.category property': hasMetaCategory, - 'meta.description property': hasMetaDescription, - 'meta.requiredArtifacts array': hasMetaRequiredArtifacts, - 'generateAuditResult()': hasGenerateAuditResult - }; +function assertValidAudit(audit, auditDefinition) { + if (typeof auditDefinition.audit !== 'function') { + throw new Error(`${audit} has no audit() method.`); + } + + if (typeof auditDefinition.meta.name !== 'string') { + throw new Error(`${audit} has no meta.name property, or the property is not a string.`); + } + + if (typeof auditDefinition.meta.category !== 'string') { + throw new Error(`${audit} has no meta.category property, or the property is not a string.`); + } + + if (typeof auditDefinition.meta.description !== 'string') { + throw new Error(`${audit} has no meta.description property, or the property is not a string.`); + } + + if (!Array.isArray(auditDefinition.meta.requiredArtifacts)) { + throw new Error( + `${audit} has no meta.requiredArtifacts property, or the property is not an array.` + ); + } + + if (typeof auditDefinition.generateAuditResult !== 'function') { + throw new Error( + `${audit} has no generateAuditResult() method. Did you inherit from the proper base class?` + ); + } } function expandArtifacts(artifacts, includeSpeedline) { @@ -300,58 +299,34 @@ class Config { * @constructor * @param{Object} config */ - constructor(configJSON, auditWhitelist) { + constructor(configJSON, auditWhitelist, configPath) { if (!configJSON) { configJSON = defaultConfig; } - this._configJSON = this._initRequirePaths(configJSON); + // Store the directory of the config path, if one was provided. + this._configDir = configPath ? path.dirname(configPath) : undefined; - this._audits = this.json.audits ? expandAudits( - filterAudits(this.json.audits, auditWhitelist), this.json.paths.audits + this._audits = configJSON.audits ? expandAudits( + filterAudits(configJSON.audits, auditWhitelist), this._configDir ) : null; // filterPasses expects audits to have been expanded - this._passes = this.json.passes ? - filterPasses(this.json.passes, this._audits, this.json.paths.gatherers) : + this._passes = configJSON.passes ? + filterPasses(configJSON.passes, this._audits, this._configDir) : null; - this._auditResults = this.json.auditResults ? Array.from(this.json.auditResults) : null; + this._auditResults = configJSON.auditResults ? Array.from(configJSON.auditResults) : null; this._artifacts = null; - if (this.json.artifacts) { - this._artifacts = expandArtifacts(this.json.artifacts, + if (configJSON.artifacts) { + this._artifacts = expandArtifacts(configJSON.artifacts, // If time-to-interactive is present, add the speedline artifact - this.json.audits && this.json.audits.find(a => a === 'time-to-interactive')); + configJSON.audits && configJSON.audits.find(a => a === 'time-to-interactive')); } - this._aggregations = this.json.aggregations ? Array.from(this.json.aggregations) : null; - } - - _initRequirePaths(configJSON) { - if (typeof configJSON.paths !== 'object') { - configJSON.paths = {}; - } - - if (!Array.isArray(configJSON.paths.audits)) { - configJSON.paths.audits = []; - } - - if (!Array.isArray(configJSON.paths.gatherers)) { - configJSON.paths.gatherers = []; - } - - // Make sure the default paths are prepended to the list - if (configJSON.paths.audits.indexOf('lighthouse-core/audits') === -1) { - configJSON.paths.audits.unshift('lighthouse-core/audits'); - } - - if (configJSON.paths.gatherers.indexOf('lighthouse-core/gather/gatherers') === -1) { - configJSON.paths.gatherers.unshift('lighthouse-core/gather/gatherers'); - } - - return configJSON; + this._aggregations = configJSON.aggregations ? Array.from(configJSON.aggregations) : null; } - /** @type {!Object} */ - get json() { - return this._configJSON; + /** @type {string} */ + get configDir() { + return this._configDir; } /** @type {Array} */ diff --git a/lighthouse-core/gather/gather-runner.js b/lighthouse-core/gather/gather-runner.js index 39e609a021e4..c3a0b4d8b003 100644 --- a/lighthouse-core/gather/gather-runner.js +++ b/lighthouse-core/gather/gather-runner.js @@ -202,8 +202,6 @@ class GatherRunner { return Promise.reject(new Error('You must provide a config')); } - const configJSON = options.config.json; - // Default mobile emulation and page loading to true. // The extension will switch these off initially. if (typeof options.flags.mobile === 'undefined') { @@ -214,7 +212,7 @@ class GatherRunner { options.flags.loadPage = true; } - passes = this.instantiateGatherers(passes, configJSON.paths.gatherers); + passes = this.instantiateGatherers(passes, options.config.configDir); return driver.connect() .then(_ => GatherRunner.setupDriver(driver, options)) @@ -258,36 +256,69 @@ class GatherRunner { }); } - static getGathererClass(gatherer, paths) { - const rootPath = path.join(__dirname, '../../'); - - // Check each path to see if the gatherer can be located. First match wins. - const gathererDefinition = paths.reduce((definition, gathererPath) => { - // If the definition has already been found, just propagate it. Otherwise try a search - // on the path in this iteration of the loop. - if (definition !== null) { - return definition; - } + static getGathererClass(gatherer, rootPath) { + const Runner = require('../runner'); + const list = Runner.getGathererList(); + const coreGatherer = list.find(a => a === `${gatherer}.js`); - const requirePath = gathererPath.startsWith('/') ? - gathererPath : - path.join(rootPath, gathererPath); + // Assume it's a core gatherer first. + let requirePath = path.resolve(__dirname, `./gatherers/${gatherer}`); + let GathererClass; + // If not, see if it can be found another way. + if (!coreGatherer) { + // Firstly try and see if the gatherer resolves naturally through the usual means. try { - return require(`${requirePath}/${gatherer}`); + require.resolve(gatherer); + + // If the above works, update the path to the absolute value provided. + requirePath = gatherer; } catch (requireError) { - return null; + // If that fails, try and find it relative to any config path provided. + if (rootPath) { + requirePath = path.resolve(rootPath, gatherer); + } } - }, null); + } + + // Now try and require it in. If this fails then the audit file isn't where we expected it. + try { + GathererClass = require(requirePath); + } catch (requireError) { + GathererClass = null; + } - if (!gathererDefinition) { - throw new Error(`Unable to locate gatherer: ${gatherer}`); + if (!GathererClass) { + throw new Error(`Unable to locate gatherer: ${gatherer} (tried at: ${requirePath})`); } - return gathererDefinition; + // Confirm that the gatherer appears valid. + this.assertValidGatherer(gatherer, GathererClass); + + return GathererClass; + } + + static assertValidGatherer(gatherer, GathererDefinition) { + const gathererInstance = new GathererDefinition(); + + if (typeof gathererInstance.beforePass !== 'function') { + throw new Error(`${gatherer} has no beforePass() method.`); + } + + if (typeof gathererInstance.pass !== 'function') { + throw new Error(`${gatherer} has no pass() method.`); + } + + if (typeof gathererInstance.afterPass !== 'function') { + throw new Error(`${gatherer} has no afterPass() method.`); + } + + if (typeof gathererInstance.artifact !== 'object') { + throw new Error(`${gatherer} has no artifact property.`); + } } - static instantiateGatherers(passes, paths) { + static instantiateGatherers(passes, rootPath) { return passes.map(pass => { pass.gatherers = pass.gatherers.map(gatherer => { // If this is already instantiated, don't do anything else. @@ -295,7 +326,7 @@ class GatherRunner { return gatherer; } - const GathererClass = GatherRunner.getGathererClass(gatherer, paths); + const GathererClass = GatherRunner.getGathererClass(gatherer, rootPath); return new GathererClass(); }); diff --git a/lighthouse-core/index.js b/lighthouse-core/index.js index 7ec2dfed636f..340ef95541db 100644 --- a/lighthouse-core/index.js +++ b/lighthouse-core/index.js @@ -62,7 +62,7 @@ module.exports = function(url, flags, configJSON) { } // Use ConfigParser to generate a valid config file - const config = new Config(configJSON, flags.auditWhitelist); + const config = new Config(configJSON, flags.auditWhitelist, flags.configPath); const driver = new ChromeProtocol(); @@ -73,3 +73,5 @@ module.exports = function(url, flags, configJSON) { module.exports.getAuditList = Runner.getAuditList; module.exports.traceCategories = require('./gather/drivers/driver').traceCategories; +module.exports.Audit = require('./audits/audit'); +module.exports.Gatherer = require('./gather/gatherers/gatherer'); diff --git a/lighthouse-core/runner.js b/lighthouse-core/runner.js index bd707d6d302e..e7210ff9248b 100644 --- a/lighthouse-core/runner.js +++ b/lighthouse-core/runner.js @@ -128,6 +128,16 @@ class Runner { .readdirSync(path.join(__dirname, './audits')) .filter(f => /\.js$/.test(f)); } + + /** + * Returns list of gatherer names for external querying. + * @return {!Array} + */ + static getGathererList() { + return fs + .readdirSync(path.join(__dirname, './gather/gatherers')) + .filter(f => /\.js$/.test(f)); + } } module.exports = Runner; diff --git a/lighthouse-core/test/config/index.js b/lighthouse-core/test/config/index.js index 093459a011c1..67c8ca756ce6 100644 --- a/lighthouse-core/test/config/index.js +++ b/lighthouse-core/test/config/index.js @@ -127,37 +127,42 @@ describe('Config', () => { return assert.equal(typeof config.audits[0], 'function'); }); - it('tests multiple paths for expanding audits', () => { - const config = new Config({ - paths: { - audits: ['/fake-path/'] - }, - audits: ['user-timings'] - }); - - assert.ok(Array.isArray(config.audits)); - assert.equal(config.audits.length, 1); - + it('throws when it audit is not found', () => { return assert.throws(_ => new Config({ - paths: { - audits: ['/fake-path/'] - }, - audits: ['non-existent-audit'] + audits: ['/fake-path/non-existent-audit'] })); }); + it('loads an audit relative to a config', () => { + return assert.doesNotThrow(_ => new Config({ + audits: ['../fixtures/valid-custom-audit'] + }, null, __filename)); + }); + it('throws when it finds invalid audits', () => { - const paths = { - audits: ['lighthouse-core/test/fixtures/invalid-audits'] - }; + assert.throws(_ => new Config({ + audits: ['../test/fixtures/invalid-audits/missing-audit'] + }), /audit\(\) method/); + + assert.throws(_ => new Config({ + audits: ['../test/fixtures/invalid-audits/missing-category'] + }), /meta.category property/); - assert.throws(_ => new Config({paths, audits: ['missing-meta']})); - assert.throws(_ => new Config({paths, audits: ['missing-audit']})); - assert.throws(_ => new Config({paths, audits: ['missing-category']})); - assert.throws(_ => new Config({paths, audits: ['missing-name']})); - assert.throws(_ => new Config({paths, audits: ['missing-description']})); - assert.throws(_ => new Config({paths, audits: ['missing-required-artifact']})); - return assert.throws(_ => new Config({paths, audits: ['missing-generate-audit-result']})); + assert.throws(_ => new Config({ + audits: ['../test/fixtures/invalid-audits/missing-name'] + }), /meta.name property/); + + assert.throws(_ => new Config({ + audits: ['../test/fixtures/invalid-audits/missing-description'] + }), /meta.description property/); + + assert.throws(_ => new Config({ + audits: ['../test/fixtures/invalid-audits/missing-required-artifacts'] + }), /meta.requiredArtifacts property/); + + return assert.throws(_ => new Config({ + audits: ['../test/fixtures/invalid-audits/missing-generate-audit-result'] + }), /generateAuditResult\(\) method/); }); it('expands artifacts', () => { diff --git a/lighthouse-core/test/fixtures/invalid-audits/missing-audit.js b/lighthouse-core/test/fixtures/invalid-audits/missing-audit.js index 36ccefb85cc0..e02404298663 100644 --- a/lighthouse-core/test/fixtures/invalid-audits/missing-audit.js +++ b/lighthouse-core/test/fixtures/invalid-audits/missing-audit.js @@ -23,6 +23,7 @@ class MissingRequiredArtifacts extends LighthouseAudit { static get meta() { return { name: 'missing-category', + category: 'Custom', description: 'Missing required artifacts', requiredArtifacts: ['HTML'] }; diff --git a/lighthouse-core/test/fixtures/invalid-gatherers/missing-after-pass.js b/lighthouse-core/test/fixtures/invalid-gatherers/missing-after-pass.js new file mode 100644 index 000000000000..8689890e470e --- /dev/null +++ b/lighthouse-core/test/fixtures/invalid-gatherers/missing-after-pass.js @@ -0,0 +1,29 @@ +/** + * + * Copyright 2016 Google Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +'use strict'; + +class MissingAfterPass { + constructor() { + this.artifact = {}; + } + + beforePass() {} + pass() {} +} + +module.exports = MissingAfterPass; diff --git a/custom/audit.js b/lighthouse-core/test/fixtures/invalid-gatherers/missing-artifact.js similarity index 83% rename from custom/audit.js rename to lighthouse-core/test/fixtures/invalid-gatherers/missing-artifact.js index e3df30e5db0b..79a5b748ca75 100644 --- a/custom/audit.js +++ b/lighthouse-core/test/fixtures/invalid-gatherers/missing-artifact.js @@ -15,5 +15,12 @@ * limitations under the License. */ -// Pass-through file for developer convenience. -module.exports = require('../lighthouse-core/audits/audit'); +'use strict'; + +class MissingArtifact { + beforePass() {} + pass() {} + afterPass() {} +} + +module.exports = MissingArtifact; diff --git a/lighthouse-core/test/fixtures/invalid-gatherers/missing-before-pass.js b/lighthouse-core/test/fixtures/invalid-gatherers/missing-before-pass.js new file mode 100644 index 000000000000..b18e3749b5cf --- /dev/null +++ b/lighthouse-core/test/fixtures/invalid-gatherers/missing-before-pass.js @@ -0,0 +1,29 @@ +/** + * + * Copyright 2016 Google Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +'use strict'; + +class MissingBeforePass { + constructor() { + this.artifact = {}; + } + + pass() {} + afterPass() {} +} + +module.exports = MissingBeforePass; diff --git a/custom/gatherer.js b/lighthouse-core/test/fixtures/invalid-gatherers/missing-pass.js similarity index 80% rename from custom/gatherer.js rename to lighthouse-core/test/fixtures/invalid-gatherers/missing-pass.js index 3190289a39be..a8f1201836bf 100644 --- a/custom/gatherer.js +++ b/lighthouse-core/test/fixtures/invalid-gatherers/missing-pass.js @@ -15,5 +15,15 @@ * limitations under the License. */ -// Pass-through file for developer convenience. -module.exports = require('../lighthouse-core/gather/gatherers/gatherer'); +'use strict'; + +class MissingPass { + constructor() { + this.artifact = {}; + } + + beforePass() {} + afterPass() {} +} + +module.exports = MissingPass; diff --git a/lighthouse-core/test/fixtures/valid-custom-audit.js b/lighthouse-core/test/fixtures/valid-custom-audit.js new file mode 100644 index 000000000000..49cb58ceb5c6 --- /dev/null +++ b/lighthouse-core/test/fixtures/valid-custom-audit.js @@ -0,0 +1,35 @@ +/** + * + * Copyright 2016 Google Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +'use strict'; + +const LighthouseAudit = require('../../audits/audit'); + +class ValidCustomAudit extends LighthouseAudit { + static get meta() { + return { + name: 'valid-audit', + category: 'Custom', + description: 'Valid Audit', + requiredArtifacts: ['HTML'] + }; + } + + static audit() {} +} + +module.exports = ValidCustomAudit; diff --git a/lighthouse-core/test/fixtures/custom-gatherers/custom-gatherer.js b/lighthouse-core/test/fixtures/valid-custom-gatherer.js similarity index 90% rename from lighthouse-core/test/fixtures/custom-gatherers/custom-gatherer.js rename to lighthouse-core/test/fixtures/valid-custom-gatherer.js index 6455a898f9ea..c77fe5d5bd66 100644 --- a/lighthouse-core/test/fixtures/custom-gatherers/custom-gatherer.js +++ b/lighthouse-core/test/fixtures/valid-custom-gatherer.js @@ -17,7 +17,7 @@ 'use strict'; -const LighthouseGatherer = require('../../../gather/gatherers/gatherer'); +const LighthouseGatherer = require('../../gather/gatherers/gatherer'); class CustomGatherer extends LighthouseGatherer {} diff --git a/lighthouse-core/test/gather/gather-runner.js b/lighthouse-core/test/gather/gather-runner.js index a05b46fa7090..e8829c00474f 100644 --- a/lighthouse-core/test/gather/gather-runner.js +++ b/lighthouse-core/test/gather/gather-runner.js @@ -23,6 +23,7 @@ const GatherRunner = require('../../gather/gather-runner'); const Audit = require('../../audits/audit'); const assert = require('assert'); const Config = require('../../config'); +const path = require('path'); class TestGatherer extends Gatherer { constructor() { @@ -314,14 +315,25 @@ describe('GatherRunner', function() { }); it('loads gatherers from custom paths', () => { - return assert.doesNotThrow(_ => GatherRunner.getGathererClass('custom-gatherer', - ['lighthouse-core/test/fixtures/custom-gatherers'])); + const root = path.resolve(__dirname, '../fixtures'); + + assert.doesNotThrow(_ => GatherRunner.getGathererClass(`${root}/valid-custom-gatherer`)); + return assert.doesNotThrow(_ => GatherRunner.getGathererClass('valid-custom-gatherer', root)); }); - it('checks multiple paths for gatherers', () => { - return assert.doesNotThrow(_ => GatherRunner.getGathererClass('custom-gatherer', - ['/fake-path/', - 'lighthouse-core/test/fixtures/custom-gatherers', - 'alt-path'])); + it('throws for invalid gatherers', () => { + const root = path.resolve(__dirname, '../fixtures/invalid-gatherers'); + + assert.throws(_ => GatherRunner.getGathererClass('missing-before-pass', root), + /beforePass\(\) method/); + + assert.throws(_ => GatherRunner.getGathererClass('missing-pass', root), + /pass\(\) method/); + + assert.throws(_ => GatherRunner.getGathererClass('missing-after-pass', root), + /afterPass\(\) method/); + + return assert.throws(_ => GatherRunner.getGathererClass('missing-artifact', root), + /artifact property/); }); });