From c0329f1e4b280b84c7249a8571af3af3f6339ef8 Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Wed, 9 Oct 2019 14:41:18 -0500 Subject: [PATCH] refactor(cli): rename mergeMethod -> aggregationMethod --- docs/assertions.md | 4 +-- packages/utils/src/assertions.js | 37 ++++++++++++++----------- packages/utils/test/assertions.test.js | 38 +++++++++++++------------- types/assert.d.ts | 8 ++++-- 4 files changed, 48 insertions(+), 39 deletions(-) diff --git a/docs/assertions.md b/docs/assertions.md index 6f5029741..875d59f52 100644 --- a/docs/assertions.md +++ b/docs/assertions.md @@ -20,7 +20,7 @@ The `ci.assert` wrapper will be left out of future code samples in this file to ### Audits -The result of any audit in Lighthouse can be asserted. Assertions are keyed by the Lighthouse audit ID and follow an eslint-style format of `level | [level, options]`. For a reference of the audit IDs in each category, you can take a look at the [default Lighthouse config](https://github.com/GoogleChrome/lighthouse/blob/v5.5.0/lighthouse-core/config/default-config.js#L375-L407). When no options are set, the default options of `{"mergeMethod": "optimistic", "minScore": 1}` are used. +The result of any audit in Lighthouse can be asserted. Assertions are keyed by the Lighthouse audit ID and follow an eslint-style format of `level | [level, options]`. For a reference of the audit IDs in each category, you can take a look at the [default Lighthouse config](https://github.com/GoogleChrome/lighthouse/blob/v5.5.0/lighthouse-core/config/default-config.js#L375-L407). When no options are set, the default options of `{"aggregationMethod": "optimistic", "minScore": 1}` are used. ```json { @@ -53,7 +53,7 @@ The `score`, `details.items.length`, and `numericValue` properties of audit resu } ``` -### Merge Strategies +### Aggregation Strategies When checking the results of multiple Lighthouse runs, there are multiple strategies for aggregating the results before asserting the threshold. diff --git a/packages/utils/src/assertions.js b/packages/utils/src/assertions.js index a4acba9ee..26fe0bb13 100644 --- a/packages/utils/src/assertions.js +++ b/packages/utils/src/assertions.js @@ -8,7 +8,7 @@ const _ = require('./lodash.js'); const {computeRepresentativeRuns} = require('./representative-runs.js'); -/** @typedef {keyof StrictOmit|'auditRan'} AssertionType */ +/** @typedef {keyof StrictOmit|'auditRan'} AssertionType */ /** * @typedef AssertionResult @@ -64,12 +64,12 @@ function normalizeAssertion(assertion) { /** * @param {number[]} values - * @param {LHCI.AssertCommand.AssertionMergeMethod} mergeMethod + * @param {LHCI.AssertCommand.AssertionAggregationMethod} aggregationMethod * @param {AssertionType} assertionType * @return {number} */ -function getValueForMergeMethod(values, mergeMethod, assertionType) { - if (mergeMethod === 'median') { +function getValueForAggregationMethod(values, aggregationMethod, assertionType) { + if (aggregationMethod === 'median') { const medianIndex = Math.floor((values.length - 1) / 2); const sorted = values.slice().sort((a, b) => a - b); if (values.length % 2 === 1) return sorted[medianIndex]; @@ -77,32 +77,36 @@ function getValueForMergeMethod(values, mergeMethod, assertionType) { } const useMin = - (mergeMethod === 'optimistic' && assertionType.startsWith('max')) || - (mergeMethod === 'pessimistic' && assertionType.startsWith('min')); + (aggregationMethod === 'optimistic' && assertionType.startsWith('max')) || + (aggregationMethod === 'pessimistic' && assertionType.startsWith('min')); return useMin ? Math.min(...values) : Math.max(...values); } /** * @param {LH.AuditResult[]} auditResults - * @param {LHCI.AssertCommand.AssertionMergeMethod} mergeMethod + * @param {LHCI.AssertCommand.AssertionAggregationMethod} aggregationMethod * @param {AssertionType} assertionType * @param {number} expectedValue * @return {AssertionResultNoURL[]} */ -function getAssertionResult(auditResults, mergeMethod, assertionType, expectedValue) { +function getAssertionResult(auditResults, aggregationMethod, assertionType, expectedValue) { const values = auditResults.map(AUDIT_TYPE_VALUE_GETTERS[assertionType]); const filteredValues = values.filter(isFiniteNumber); if ( - (!filteredValues.length && mergeMethod !== 'pessimistic') || - (filteredValues.length !== values.length && mergeMethod === 'pessimistic') + (!filteredValues.length && aggregationMethod !== 'pessimistic') || + (filteredValues.length !== values.length && aggregationMethod === 'pessimistic') ) { const didRun = values.map(value => (isFiniteNumber(value) ? 1 : 0)); return [{name: 'auditRan', expected: 1, actual: 0, values: didRun, operator: '=='}]; } const {operator, passesFn} = AUDIT_TYPE_OPERATORS[assertionType]; - const actualValue = getValueForMergeMethod(filteredValues, mergeMethod, assertionType); + const actualValue = getValueForAggregationMethod( + filteredValues, + aggregationMethod, + assertionType + ); if (passesFn(actualValue, expectedValue)) return []; return [ @@ -122,7 +126,7 @@ function getAssertionResult(auditResults, mergeMethod, assertionType, expectedVa * @return {AssertionResultNoURL[]} */ function getAssertionResults(possibleAuditResults, options) { - const {minScore, maxLength, maxNumericValue, mergeMethod = 'optimistic'} = options; + const {minScore, maxLength, maxNumericValue, aggregationMethod = 'optimistic'} = options; if (possibleAuditResults.some(result => result === undefined)) { return [ { @@ -147,19 +151,19 @@ function getAssertionResults(possibleAuditResults, options) { if (maxLength !== undefined) { hadManualAssertion = true; - results.push(...getAssertionResult(auditResults, mergeMethod, 'maxLength', maxLength)); + results.push(...getAssertionResult(auditResults, aggregationMethod, 'maxLength', maxLength)); } if (maxNumericValue !== undefined) { hadManualAssertion = true; results.push( - ...getAssertionResult(auditResults, mergeMethod, 'maxNumericValue', maxNumericValue) + ...getAssertionResult(auditResults, aggregationMethod, 'maxNumericValue', maxNumericValue) ); } const realMinScore = minScore === undefined && !hadManualAssertion ? 1 : minScore; if (realMinScore !== undefined) { - results.push(...getAssertionResult(auditResults, mergeMethod, 'minScore', realMinScore)); + results.push(...getAssertionResult(auditResults, aggregationMethod, 'minScore', realMinScore)); } return results; @@ -334,7 +338,8 @@ function getAllFilteredAssertionResults(baseOptions, unfilteredLhrs) { const [level, assertionOptions] = normalizeAssertion(assertions[assertionKey]); if (level === 'off') continue; - const lhrsToUseForAudit = assertionOptions.mergeMethod === 'median-run' ? medianLhrs : lhrs; + const lhrsToUseForAudit = + assertionOptions.aggregationMethod === 'median-run' ? medianLhrs : lhrs; const auditResults = lhrsToUseForAudit.map(lhr => lhr.audits[auditId]); const assertionResults = getAssertionResultsForAudit( auditId, diff --git a/packages/utils/test/assertions.test.js b/packages/utils/test/assertions.test.js index e28c70ecd..f6ca43d80 100644 --- a/packages/utils/test/assertions.test.js +++ b/packages/utils/test/assertions.test.js @@ -120,7 +120,7 @@ describe('getAllAssertionResults', () => { it('should use minScore = 1 by default', () => { const assertions = { - 'first-contentful-paint': ['warn', {mergeMethod: 'optimistic'}], + 'first-contentful-paint': ['warn', {aggregationMethod: 'optimistic'}], }; const results = getAllAssertionResults({assertions}, lhrs); @@ -155,46 +155,46 @@ describe('getAllAssertionResults', () => { it('should de-dupe camelcase audits', () => { const assertions = { - firstContentfulPaint: ['warn', {mergeMethod: 'optimistic', minScore: 1}], - 'first-contentful-paint': ['warn', {mergeMethod: 'optimistic', minScore: 1}], + firstContentfulPaint: ['warn', {aggregationMethod: 'optimistic', minScore: 1}], + 'first-contentful-paint': ['warn', {aggregationMethod: 'optimistic', minScore: 1}], }; const results = getAllAssertionResults({assertions}, lhrs); expect(results).toMatchObject([{actual: 0.8}]); }); - describe('mergeMethod', () => { - it('should use mergeMethod optimistic', () => { + describe('aggregationMethod', () => { + it('should use aggregationMethod optimistic', () => { const assertions = { - 'first-contentful-paint': ['warn', {mergeMethod: 'optimistic', minScore: 1}], - 'network-requests': ['warn', {mergeMethod: 'optimistic', maxLength: 1}], + 'first-contentful-paint': ['warn', {aggregationMethod: 'optimistic', minScore: 1}], + 'network-requests': ['warn', {aggregationMethod: 'optimistic', maxLength: 1}], }; const results = getAllAssertionResults({assertions}, lhrs); expect(results).toMatchObject([{actual: 0.8}, {actual: 2}]); }); - it('should use mergeMethod pessimistic', () => { + it('should use aggregationMethod pessimistic', () => { const assertions = { - 'first-contentful-paint': ['warn', {mergeMethod: 'pessimistic', minScore: 1}], - 'network-requests': ['warn', {mergeMethod: 'pessimistic', maxLength: 1}], + 'first-contentful-paint': ['warn', {aggregationMethod: 'pessimistic', minScore: 1}], + 'network-requests': ['warn', {aggregationMethod: 'pessimistic', maxLength: 1}], }; const results = getAllAssertionResults({assertions}, lhrs); expect(results).toMatchObject([{actual: 0.6}, {actual: 4}]); }); - it('should use mergeMethod median', () => { + it('should use aggregationMethod median', () => { const assertions = { - 'first-contentful-paint': ['warn', {mergeMethod: 'median', minScore: 1}], - 'network-requests': ['warn', {mergeMethod: 'median', maxLength: 1}], + 'first-contentful-paint': ['warn', {aggregationMethod: 'median', minScore: 1}], + 'network-requests': ['warn', {aggregationMethod: 'median', maxLength: 1}], }; const results = getAllAssertionResults({assertions}, lhrs); expect(results).toMatchObject([{actual: 0.7}, {actual: 3}]); }); - it('should use mergeMethod median-run', () => { + it('should use aggregationMethod median-run', () => { const lhrs = [ // This is the "median-run" by FCP and interactive. { @@ -224,7 +224,7 @@ describe('getAllAssertionResults', () => { ]; const assertions = { - 'other-audit': ['warn', {mergeMethod: 'median-run', maxNumericValue: 10000}], + 'other-audit': ['warn', {aggregationMethod: 'median-run', maxNumericValue: 10000}], }; const results = getAllAssertionResults({assertions}, lhrs); @@ -245,7 +245,7 @@ describe('getAllAssertionResults', () => { it('should handle partial failure with mode optimistic', () => { const assertions = { - 'first-contentful-paint': ['warn', {mergeMethod: 'optimistic'}], + 'first-contentful-paint': ['warn', {aggregationMethod: 'optimistic'}], }; lhrs[1].audits['first-contentful-paint'].score = null; @@ -255,7 +255,7 @@ describe('getAllAssertionResults', () => { it('should handle partial failure with mode median', () => { const assertions = { - 'first-contentful-paint': ['warn', {mergeMethod: 'median'}], + 'first-contentful-paint': ['warn', {aggregationMethod: 'median'}], }; lhrs[1].audits['first-contentful-paint'].score = null; @@ -265,7 +265,7 @@ describe('getAllAssertionResults', () => { it('should handle partial failure when mode is pessimistic', () => { const assertions = { - 'first-contentful-paint': ['warn', {mergeMethod: 'pessimistic'}], + 'first-contentful-paint': ['warn', {aggregationMethod: 'pessimistic'}], }; lhrs[1].audits['first-contentful-paint'].score = null; @@ -292,7 +292,7 @@ describe('getAllAssertionResults', () => { it('should use the preset with changes', () => { const assertions = { - 'first-contentful-paint': ['warn', {mergeMethod: 'pessimistic', minScore: 0.6}], + 'first-contentful-paint': ['warn', {aggregationMethod: 'pessimistic', minScore: 0.6}], }; const results = getAllAssertionResults({preset: 'lighthouse:all', assertions}, lhrs); diff --git a/types/assert.d.ts b/types/assert.d.ts index 1e17da838..b2b5ea877 100644 --- a/types/assert.d.ts +++ b/types/assert.d.ts @@ -7,12 +7,16 @@ declare global { namespace LHCI { namespace AssertCommand { - export type AssertionMergeMethod = 'median' | 'optimistic' | 'pessimistic' | 'median-run'; + export type AssertionAggregationMethod = + | 'median' + | 'optimistic' + | 'pessimistic' + | 'median-run'; export type AssertionFailureLevel = 'off' | 'warn' | 'error'; export interface AssertionOptions { - mergeMethod?: AssertionMergeMethod; + aggregationMethod?: AssertionAggregationMethod; minScore?: number; maxLength?: number; maxNumericValue?: number;