Skip to content

Commit

Permalink
refactor(cli): rename mergeMethod -> aggregationMethod
Browse files Browse the repository at this point in the history
  • Loading branch information
@patrickhulce
patrickhulce committed Oct 9, 2019
1 parent df13eb4 commit c0329f1
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 39 deletions.
4 changes: 2 additions & 2 deletions docs/assertions.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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.

Expand Down
37 changes: 21 additions & 16 deletions packages/utils/src/assertions.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
const _ = require('./lodash.js');
const {computeRepresentativeRuns} = require('./representative-runs.js');

/** @typedef {keyof StrictOmit<LHCI.AssertCommand.AssertionOptions, 'mergeMethod'>|'auditRan'} AssertionType */
/** @typedef {keyof StrictOmit<LHCI.AssertCommand.AssertionOptions, 'aggregationMethod'>|'auditRan'} AssertionType */

/**
* @typedef AssertionResult
Expand Down Expand Up @@ -64,45 +64,49 @@ 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];
return (sorted[medianIndex] + sorted[medianIndex + 1]) / 2;
}

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 [
Expand All @@ -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 [
{
Expand All @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
38 changes: 19 additions & 19 deletions packages/utils/test/assertions.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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.
{
Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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);
Expand Down
8 changes: 6 additions & 2 deletions types/assert.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit c0329f1

Please sign in to comment.