From 75066c9503f27eefcd43aec7fdcd560195faf622 Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Fri, 6 Aug 2021 13:45:13 -0500 Subject: [PATCH] core(fr): override quiet windows for observed performance (#12873) --- .github/workflows/smoke.yml | 2 +- lighthouse-core/fraggle-rock/config/config.js | 41 ++++- .../test/fraggle-rock/config/config-test.js | 144 +++++++++++------- 3 files changed, 125 insertions(+), 62 deletions(-) diff --git a/.github/workflows/smoke.yml b/.github/workflows/smoke.yml index 8db90bf532a9..1eb3c50a483d 100644 --- a/.github/workflows/smoke.yml +++ b/.github/workflows/smoke.yml @@ -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 diff --git a/lighthouse-core/fraggle-rock/config/config.js b/lighthouse-core/fraggle-rock/config/config.js index ed8eb804b544..99cc3d0be4d3 100644 --- a/lighthouse-core/fraggle-rock/config/config.js +++ b/lighthouse-core/fraggle-rock/config/config.js @@ -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, @@ -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'); @@ -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); @@ -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 = { diff --git a/lighthouse-core/test/fraggle-rock/config/config-test.js b/lighthouse-core/test/fraggle-rock/config/config-test.js index 3d73b666ef4c..98ff41b958a8 100644 --- a/lighthouse-core/test/fraggle-rock/config/config-test.js +++ b/lighthouse-core/test/fraggle-rock/config/config-test.js @@ -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'); @@ -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']}; @@ -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;