Skip to content

Commit

Permalink
rm gatherer name
Browse files Browse the repository at this point in the history
  • Loading branch information
adamraine committed Jul 13, 2023
1 parent ae7380e commit 1550759
Show file tree
Hide file tree
Showing 8 changed files with 17 additions and 32 deletions.
17 changes: 8 additions & 9 deletions core/config/validation.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,25 +47,24 @@ function assertValidPluginName(config, pluginName) {

/**
* Throws an error if the provided object does not implement the required Fraggle Rock gatherer interface.
* @param {LH.Config.AnyFRGathererDefn} gathererDefn
* @param {LH.Config.AnyArtifactDefn} artifactDefn
*/
function assertValidFRGatherer(gathererDefn) {
const gatherer = gathererDefn.instance;
const gathererName = gatherer.name;
function assertValidArtifact(artifactDefn) {
const gatherer = artifactDefn.gatherer.instance;

if (typeof gatherer.meta !== 'object') {
throw new Error(`${gathererName} gatherer did not provide a meta object.`);
throw new Error(`Gatherer for ${artifactDefn.id} did not provide a meta object.`);
}

if (gatherer.meta.supportedModes.length === 0) {
throw new Error(`${gathererName} gatherer did not support any gather modes.`);
throw new Error(`Gatherer for ${artifactDefn.id} did not support any gather modes.`);
}

if (
typeof gatherer.getArtifact !== 'function' ||
gatherer.getArtifact === BaseFRGatherer.prototype.getArtifact
) {
throw new Error(`${gathererName} gatherer did not define a "getArtifact" method.`);
throw new Error(`Gatherer for ${artifactDefn.id} did not define a "getArtifact" method.`);
}
}

Expand Down Expand Up @@ -258,7 +257,7 @@ function assertValidConfig(resolvedConfig) {
throw new Error(`Config defined multiple artifacts with id '${artifactDefn.id}'`);
}
artifactIds.add(artifactDefn.id);
assertValidFRGatherer(artifactDefn.gatherer);
assertValidArtifact(artifactDefn);
}

for (const auditDefn of resolvedConfig.audits || []) {
Expand Down Expand Up @@ -303,7 +302,7 @@ function throwInvalidArtifactDependency(artifactId, dependencyKey) {
export {
isValidArtifactDependency,
assertValidPluginName,
assertValidFRGatherer,
assertValidArtifact,
assertValidFRNavigations,
assertValidAudit,
assertValidCategories,
Expand Down
14 changes: 0 additions & 14 deletions core/gather/base-gatherer.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,20 +54,6 @@ class FRGatherer {
* @return {LH.Gatherer.PhaseResult}
*/
getArtifact(passContext) { }

/**
* Legacy property used to define the artifact ID. In Fraggle Rock, the artifact ID lives on the config.
* @return {keyof LH.GathererArtifacts}
*/
get name() {
let name = this.constructor.name;
// Rollup will mangle class names in an known way–just trim until `$`.
if (name.includes('$')) {
name = name.substr(0, name.indexOf('$'));
}
// @ts-expect-error - assume that class name has been added to LH.GathererArtifacts.
return name;
}
}

export default FRGatherer;
2 changes: 1 addition & 1 deletion core/gather/gatherers/css-usage.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class CSSUsage extends FRGatherer {
);
Sentry.captureException(err, {
tags: {
gatherer: this.name,
gatherer: 'CSSUsage',
},
extra: {
url: event.header.sourceURL,
Expand Down
2 changes: 1 addition & 1 deletion core/gather/gatherers/trace-elements.js
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ class TraceElements extends FRGatherer {
});
} catch (err) {
Sentry.captureException(err, {
tags: {gatherer: this.name},
tags: {gatherer: 'TraceElements'},
level: 'error',
});
continue;
Expand Down
11 changes: 7 additions & 4 deletions core/test/config/validation-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,20 +84,22 @@ describe('Fraggle Rock Config Validation', () => {
});
});

describe('.assertValidFRGatherer', () => {
describe('.assertValidArtifact', () => {
it('should throw if gatherer does not have a meta object', () => {
const gatherer = new BaseGatherer();
// @ts-expect-error - We are intentionally creating a malformed input.
gatherer.meta = undefined;

const gathererDefn = {instance: gatherer};
const invocation = () => validation.assertValidFRGatherer(gathererDefn);
const artifactDefn = {id: 'NewArtifact', gatherer: gathererDefn};
const invocation = () => validation.assertValidArtifact(artifactDefn);
expect(invocation).toThrow(/did not provide a meta/);
});

it('should throw if gatherer does not have a supported modes', () => {
const gathererDefn = {instance: new BaseGatherer()};
const invocation = () => validation.assertValidFRGatherer(gathererDefn);
const artifactDefn = {id: 'NewArtifact', gatherer: gathererDefn};
const invocation = () => validation.assertValidArtifact(artifactDefn);
expect(invocation).toThrow(/did not support any gather modes/);
});

Expand All @@ -106,7 +108,8 @@ describe('Fraggle Rock Config Validation', () => {
gatherer.meta = {supportedModes: ['navigation']};

const gathererDefn = {instance: gatherer};
const invocation = () => validation.assertValidFRGatherer(gathererDefn);
const artifactDefn = {id: 'NewArtifact', gatherer: gathererDefn};
const invocation = () => validation.assertValidArtifact(artifactDefn);
expect(invocation).toThrow(/did not define.*getArtifact/);
});
});
Expand Down
1 change: 0 additions & 1 deletion core/test/gather/mock-driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ function createMockGathererInstance(meta) {

/** @return {LH.Gatherer.AnyFRGathererInstance} */
asGatherer() {
// @ts-expect-error - We'll rely on the tests passing to know this matches.
return this;
},
};
Expand Down
1 change: 0 additions & 1 deletion core/test/gather/navigation-runner-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ describe('NavigationRunner', () => {
function createGathererDefn() {
return {
instance: {
name: 'Accessibility',
meta: {supportedModes: []},
startInstrumentation: fnAny(),
stopInstrumentation: fnAny(),
Expand Down
1 change: 0 additions & 1 deletion types/gatherer.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,6 @@ declare module Gatherer {
type FRGatherPhase = keyof Omit<Gatherer.FRGathererInstance, 'name'|'meta'>

interface FRGathererInstance<TDependencies extends DependencyKey = DefaultDependenciesKey> {
name: keyof GathererArtifacts; // temporary COMPAT measure until artifact config support is available
meta: GathererMeta<TDependencies>;
startInstrumentation(context: FRTransitionalContext<DefaultDependenciesKey>): Promise<void>|void;
startSensitiveInstrumentation(context: FRTransitionalContext<DefaultDependenciesKey>): Promise<void>|void;
Expand Down

0 comments on commit 1550759

Please sign in to comment.