fix(cli): add message for invalid assertion types

GoogleChrome · Sep 22, 2020 · 0f6d302 · 0f6d302
1 parent 56f6062
commit 0f6d302
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 5 deletions.
diff --git a/packages/cli/src/assert/assert.js b/packages/cli/src/assert/assert.js
@@ -91,7 +91,7 @@ async function runCommand(options) {
 
       const auditTitlePart = result.auditTitle || '';
       const documentationPart = result.auditDocumentationLink || '';
-      const titleAndDocs = [auditTitlePart, documentationPart]
+      const titleAndDocs = [auditTitlePart, documentationPart, result.message]
         .filter(Boolean)
         .map(s => `       ` + s)
         .join('\n');

diff --git a/packages/utils/src/assertions.js b/packages/utils/src/assertions.js
@@ -25,6 +25,7 @@ const {computeRepresentativeRuns} = require('./representative-runs.js');
  * @property {string|undefined} [auditProperty]
  * @property {string|undefined} [auditTitle]
  * @property {string|undefined} [auditDocumentationLink]
+ * @property {string} [message]
  */
 
 /** @typedef {StrictOmit<AssertionResult, 'url'>} AssertionResultNoURL */
@@ -96,18 +97,40 @@ function getValueForAggregationMethod(values, aggregationMethod, assertionType)
 function getAssertionResult(auditResults, aggregationMethod, assertionType, expectedValue) {
   const values = auditResults.map(AUDIT_TYPE_VALUE_GETTERS[assertionType]);
   const filteredValues = values.filter(isFiniteNumber);
+  const {operator, passedFn} = AUDIT_TYPE_OPERATORS[assertionType];
 
   if (
     (!filteredValues.length && aggregationMethod !== 'pessimistic') ||
     (filteredValues.length !== values.length && aggregationMethod === 'pessimistic')
   ) {
-    const didRun = values.map(value => (isFiniteNumber(value) ? 1 : 0));
+    /** @type {string|undefined} */
+    let message = undefined;
+    if (values.some(v => v === undefined)) {
+      const userAction = `"${assertionType}" might not be a valid assertion for this audit.`;
+      message = `Audit did not produce a value at all. ${userAction}`;
+    } else {
+      const userAction =
+        aggregationMethod === 'pessimistic'
+          ? 'Try using an aggregationMethod other than "pessimistic".'
+          : 'If this is reproducible, please file an issue on GitHub.';
+      message = `Audit failed to produce a valid value. ${userAction}`;
+    }
+
+    const didRun = values.map(value => (isFiniteNumber(value) ? value : NaN));
+
     return [
-      {name: 'auditRan', expected: 1, actual: 0, values: didRun, operator: '==', passed: false},
+      {
+        name: assertionType,
+        expected: expectedValue,
+        actual: NaN,
+        values: didRun,
+        operator,
+        passed: false,
+        message,
+      },
     ];
   }
 
-  const {operator, passedFn} = AUDIT_TYPE_OPERATORS[assertionType];
   const actualValue = getValueForAggregationMethod(
     filteredValues,
     aggregationMethod,

diff --git a/packages/utils/test/assertions.test.js b/packages/utils/test/assertions.test.js
@@ -30,6 +30,9 @@ describe('getAllAssertionResults', () => {
             score: 0,
             details: {items: [1, 2, 3, 4]},
           },
+          'cumulative-layout-shift': {
+            numericValue: 0,
+          },
         },
       },
       {
@@ -46,6 +49,9 @@ describe('getAllAssertionResults', () => {
             score: 0,
             details: {items: [1, 2]},
           },
+          'cumulative-layout-shift': {
+            numericValue: 0,
+          },
         },
       },
     ];
@@ -73,6 +79,7 @@ describe('getAllAssertionResults', () => {
     const assertions = {
       'first-contentful-paint': ['error', {minScore: 0.5}],
       'network-requests': ['warn', {maxLength: 10}],
+      'cumulative-layout-shift': ['error', {maxNumericValue: 1}],
     };
 
     const results = getAllAssertionResults({assertions}, lhrs);
@@ -201,6 +208,29 @@ describe('getAllAssertionResults', () => {
     expect(results).toMatchObject([{actual: 0.8, expected: 0.9}]);
   });
 
+  it('should warn on incorrect assertion type', () => {
+    const assertions = {
+      'network-requests': ['error', {maxNumericValue: 10}],
+    };
+
+    const results = getAllAssertionResults({assertions}, lhrs);
+    expect(results).toEqual([
+      {
+        actual: NaN,
+        auditId: 'network-requests',
+        expected: 10,
+        level: 'error',
+        message:
+          'Audit did not produce a value at all. "maxNumericValue" might not be a valid assertion for this audit.',
+        name: 'maxNumericValue',
+        operator: '<=',
+        passed: false,
+        url: 'http://page-1.com',
+        values: [NaN, NaN],
+      },
+    ]);
+  });
+
   it('should respect notApplicable', () => {
     const assertions = {
       'network-requests': 'error',
@@ -312,6 +342,7 @@ describe('getAllAssertionResults', () => {
       const assertions = {
         'first-contentful-paint': ['warn', {aggregationMethod: 'pessimistic', minScore: 1}],
         'network-requests': ['warn', {aggregationMethod: 'pessimistic', maxLength: 1}],
+        'cumulative-layout-shift': ['warn', {aggregationMethod: 'pessimistic', maxNumericValue: 1}],
       };
 
       const results = getAllAssertionResults({assertions}, lhrs);
@@ -425,7 +456,7 @@ describe('getAllAssertionResults', () => {
 
       lhrs[1].audits['first-contentful-paint'].score = null;
       const results = getAllAssertionResults({assertions}, lhrs);
-      expect(results).toMatchObject([{actual: 0, expected: 1, name: 'auditRan'}]);
+      expect(results).toMatchObject([{actual: NaN, expected: 0.9, name: 'minScore'}]);
     });
   });