Skip to content

fix: skip is-crawlable audit when running onSuccess against DEPLOY_URL #621

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

11 changes: 9 additions & 2 deletions src/lib/get-settings/get-settings.test.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import getSettings from './index.js';

describe('replacements', () => {
it('should return nothing with no settings set', () => {
expect(getSettings()).toEqual(undefined);
it('should return the default config with no settings set', () => {
const derivedSettings = getSettings();
expect(derivedSettings.extends).toEqual('lighthouse:default');
expect(derivedSettings.settings).toEqual({});
});

it('should return a template config with preset set to desktop', () => {
Expand Down Expand Up @@ -34,6 +36,11 @@ describe('replacements', () => {
expect(derivedSettings.settings.locale).toEqual('es');
});

it('should skip is-crawlable audit when using deployUrl', () => {
const derivedSettings = getSettings({}, true);
expect(derivedSettings.settings.skipAudits).toEqual(['is-crawlable']);
});

it('should error with incorrect syntax for process.env.SETTINGS', () => {
process.env.SETTINGS = 'not json';
expect(getSettings).toThrow(/Invalid JSON/);
Expand Down
9 changes: 7 additions & 2 deletions src/lib/get-settings/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,8 @@ const mergeSettingsSources = (inputSettings = {}) => {
return Object.assign({}, envSettings, inputSettings);
};

const getSettings = (inputSettings) => {
const getSettings = (inputSettings, isUsingDeployUrl) => {
const settings = mergeSettingsSources(inputSettings);
if (Object.keys(settings).length === 0) return;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now we always pass back the default config, which is the same as if no config is passed to lighthouse at all

Choose a reason for hiding this comment

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

😊


// Set a base-level config based on the preset input value
// (desktop is currently the only supported option)
Expand All @@ -35,6 +34,12 @@ const getSettings = (inputSettings) => {
derivedSettings.settings.locale = settings.locale;
}

// If we are running against the Netlify deploy URL, the injected x-robots-tag will always cause the audit to fail,
// likely producing a false positive, so we skip in this case
if (isUsingDeployUrl) {
derivedSettings.settings.skipAudits = ['is-crawlable'];
}

return derivedSettings;
};

Expand Down
2 changes: 1 addition & 1 deletion src/lib/run-event/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ const runEvent = async ({
let errorMetadata = [];

try {
const settings = getSettings(inputs?.settings);
const settings = getSettings(inputs?.settings, isOnSuccess);

const allErrors = [];
const data = [];
Expand Down
Loading