Skip to content

Commit

Permalink
core(fr): override quiet windows for observed performance (#12873)
Browse files Browse the repository at this point in the history
  • Loading branch information
@patrickhulce
patrickhulce authored Aug 6, 2021
1 parent 25fd665 commit 75066c9
Show file tree
Hide file tree
Showing 3 changed files with 125 additions and 62 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/smoke.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ jobs:

- run: sudo apt-get install xvfb
- name: yarn smoke --fraggle-rock
run: xvfb-run --auto-servernum yarn smoke --debug --fraggle-rock -j=1 --retries=2 a11y lantern seo-passing seo-failing csp
run: xvfb-run --auto-servernum yarn smoke --debug --fraggle-rock -j=1 --retries=2 a11y lantern seo-passing seo-failing csp metrics

# Fail if any changes were written to source files.
- run: git diff --exit-code
Expand Down
41 changes: 35 additions & 6 deletions lighthouse-core/fraggle-rock/config/config.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const path = require('path');
const log = require('lighthouse-logger');
const Runner = require('../../runner.js');
const defaultConfig = require('./default-config.js');
const {defaultNavigationConfig} = require('../../config/constants.js');
const {defaultNavigationConfig, nonSimulatedPassConfigOverrides} = require('../../config/constants.js'); // eslint-disable-line max-len
const {
isFRGathererDefn,
throwInvalidDependencyOrder,
Expand Down Expand Up @@ -157,13 +157,42 @@ function resolveArtifactsToDefns(artifacts, configDir) {
return artifactDefns;
}

/**
* Overrides the quiet windows when throttlingMethod requires observation.
*
* @param {LH.Config.NavigationDefn} navigation
* @param {LH.Config.Settings} settings
*/
function overrideNavigationThrottlingWindows(navigation, settings) {
if (navigation.disableThrottling) return;
if (settings.throttlingMethod === 'simulate') return;

navigation.cpuQuietThresholdMs = Math.max(
navigation.cpuQuietThresholdMs || 0,
nonSimulatedPassConfigOverrides.cpuQuietThresholdMs
);
navigation.networkQuietThresholdMs = Math.max(
navigation.networkQuietThresholdMs || 0,
nonSimulatedPassConfigOverrides.networkQuietThresholdMs
);
navigation.pauseAfterFcpMs = Math.max(
navigation.pauseAfterFcpMs || 0,
nonSimulatedPassConfigOverrides.pauseAfterFcpMs
);
navigation.pauseAfterLoadMs = Math.max(
navigation.pauseAfterLoadMs || 0,
nonSimulatedPassConfigOverrides.pauseAfterLoadMs
);
}

/**
*
* @param {LH.Config.NavigationJson[]|null|undefined} navigations
* @param {LH.Config.AnyArtifactDefn[]|null|undefined} artifactDefns
* @param {LH.Config.Settings} settings
* @return {LH.Config.NavigationDefn[] | null}
*/
function resolveNavigationsToDefns(navigations, artifactDefns) {
function resolveNavigationsToDefns(navigations, artifactDefns, settings) {
if (!navigations) return null;
if (!artifactDefns) throw new Error('Cannot use navigations without defining artifacts');

Expand All @@ -181,9 +210,9 @@ function resolveNavigationsToDefns(navigations, artifactDefns) {
return artifact;
});

// TODO(FR-COMPAT): enforce navigation throttling invariants

return {...navigationWithDefaults, artifacts};
const resolvedNavigation = {...navigationWithDefaults, artifacts};
overrideNavigationThrottlingWindows(resolvedNavigation, settings);
return resolvedNavigation;
});

assertArtifactTopologicalOrder(navigationDefns);
Expand All @@ -209,7 +238,7 @@ function initializeConfig(configJSON, context) {

const settings = resolveSettings(configWorkingCopy.settings || {}, context.settingsOverrides);
const artifacts = resolveArtifactsToDefns(configWorkingCopy.artifacts, configDir);
const navigations = resolveNavigationsToDefns(configWorkingCopy.navigations, artifacts);
const navigations = resolveNavigationsToDefns(configWorkingCopy.navigations, artifacts, settings);

/** @type {LH.Config.FRConfig} */
let config = {
Expand Down
144 changes: 89 additions & 55 deletions lighthouse-core/test/fraggle-rock/config/config-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
'use strict';

const BaseAudit = require('../../../audits/audit.js');
const {nonSimulatedPassConfigOverrides} = require('../../../config/constants.js');
const BaseGatherer = require('../../../fraggle-rock/gather/base-gatherer.js');
const {initializeConfig} = require('../../../fraggle-rock/config/config.js');

Expand Down Expand Up @@ -68,61 +69,6 @@ describe('Fraggle Rock Config', () => {
expect(() => initializeConfig(configJson, {gatherMode})).toThrow(/HTTPRedirect gatherer/);
});

it('should resolve navigation definitions', () => {
gatherMode = 'navigation';
const configJson = {
artifacts: [{id: 'Accessibility', gatherer: 'accessibility'}],
navigations: [{id: 'default', artifacts: ['Accessibility']}],
};
const {config} = initializeConfig(configJson, {gatherMode});

expect(config).toMatchObject({
artifacts: [{id: 'Accessibility', gatherer: {path: 'accessibility'}}],
navigations: [
{id: 'default', artifacts: [{id: 'Accessibility', gatherer: {path: 'accessibility'}}]},
],
});
});

it('should throw when navigations are defined without artifacts', () => {
const configJson = {
navigations: [{id: 'default', artifacts: ['Accessibility']}],
};

expect(() => initializeConfig(configJson, {gatherMode})).toThrow(/Cannot use navigations/);
});

it('should throw when navigations use unrecognized artifacts', () => {
const configJson = {
artifacts: [],
navigations: [{id: 'default', artifacts: ['Accessibility']}],
};

expect(() => initializeConfig(configJson, {gatherMode})).toThrow(/Unrecognized artifact/);
});

it('should set default properties on navigations', () => {
gatherMode = 'navigation';
const configJson = {
artifacts: [{id: 'Accessibility', gatherer: 'accessibility'}],
navigations: [{id: 'default', artifacts: ['Accessibility']}],
};
const {config} = initializeConfig(configJson, {gatherMode});

expect(config).toMatchObject({
navigations: [
{
id: 'default',
blankPage: 'about:blank',
artifacts: [{id: 'Accessibility', gatherer: {path: 'accessibility'}}],
disableThrottling: false,
networkQuietThresholdMs: 0,
cpuQuietThresholdMs: 0,
},
],
});
});

it('should filter configuration by gatherMode', () => {
const timespanGatherer = new BaseGatherer();
timespanGatherer.meta = {supportedModes: ['timespan']};
Expand Down Expand Up @@ -263,6 +209,94 @@ describe('Fraggle Rock Config', () => {
});
});

describe('.resolveNavigationsToDefns', () => {
it('should resolve navigation definitions', () => {
gatherMode = 'navigation';
const configJson = {
artifacts: [{id: 'Accessibility', gatherer: 'accessibility'}],
navigations: [{id: 'default', artifacts: ['Accessibility']}],
};
const {config} = initializeConfig(configJson, {gatherMode});

expect(config).toMatchObject({
artifacts: [{id: 'Accessibility', gatherer: {path: 'accessibility'}}],
navigations: [
{id: 'default', artifacts: [{id: 'Accessibility', gatherer: {path: 'accessibility'}}]},
],
});
});

it('should throw when navigations are defined without artifacts', () => {
const configJson = {
navigations: [{id: 'default', artifacts: ['Accessibility']}],
};

expect(() => initializeConfig(configJson, {gatherMode})).toThrow(/Cannot use navigations/);
});

it('should throw when navigations use unrecognized artifacts', () => {
const configJson = {
artifacts: [],
navigations: [{id: 'default', artifacts: ['Accessibility']}],
};

expect(() => initializeConfig(configJson, {gatherMode})).toThrow(/Unrecognized artifact/);
});

it('should set default properties on navigations', () => {
gatherMode = 'navigation';
const configJson = {
artifacts: [{id: 'Accessibility', gatherer: 'accessibility'}],
navigations: [{id: 'default', artifacts: ['Accessibility']}],
};
const {config} = initializeConfig(configJson, {gatherMode});

expect(config).toMatchObject({
navigations: [
{
id: 'default',
blankPage: 'about:blank',
artifacts: [{id: 'Accessibility', gatherer: {path: 'accessibility'}}],
loadFailureMode: 'fatal',
disableThrottling: false,
networkQuietThresholdMs: 0,
cpuQuietThresholdMs: 0,
},
],
});
});

it('should ensure minimum quiet thresholds when throttlingMethod is devtools', () => {
gatherMode = 'navigation';
const configJson = {
artifacts: [{id: 'Accessibility', gatherer: 'accessibility'}],
navigations: [
{id: 'default', artifacts: ['Accessibility']},
{id: 'noThrottling', artifacts: ['Accessibility'], disableThrottling: true},
{id: 'alreadyHigh', artifacts: ['Accessibility'], cpuQuietThresholdMs: 10_000},
],
};

const {config} = initializeConfig(configJson, {
gatherMode,
settingsOverrides: {throttlingMethod: 'devtools'},
});

expect(config).toMatchObject({
navigations: [
{
pauseAfterFcpMs: nonSimulatedPassConfigOverrides.pauseAfterFcpMs,
pauseAfterLoadMs: nonSimulatedPassConfigOverrides.pauseAfterLoadMs,
networkQuietThresholdMs: nonSimulatedPassConfigOverrides.networkQuietThresholdMs,
cpuQuietThresholdMs: nonSimulatedPassConfigOverrides.cpuQuietThresholdMs,
},
{networkQuietThresholdMs: 0, cpuQuietThresholdMs: 0},
{cpuQuietThresholdMs: 10_000},
],
});
});
});

describe('.resolveExtensions', () => {
/** @type {LH.Config.Json} */
let extensionConfig;
Expand Down

0 comments on commit 75066c9

Please sign in to comment.