✨🏗 Add expectAsyncConsoleError, which enables tests to check for as…

…ync errors (#15621)
ampproject · May 26, 2018 · cbf3929 · cbf3929
1 parent 8fdc155
commit cbf3929
Show file tree

Hide file tree

Showing 3 changed files with 71 additions and 19 deletions.
diff --git a/.eslintrc b/.eslintrc
@@ -33,8 +33,7 @@
     "global": false,
     "describes": true,
     "allowConsoleError": false,
-    "dontWarnForConsoleError": false,
-    "warnForConsoleError": false
+    "expectAsyncConsoleError": false
   },
   "settings": {
     "jsdoc": {

diff --git a/extensions/amp-analytics/0.1/test/test-amp-analytics.js b/extensions/amp-analytics/0.1/test/test-amp-analytics.js
@@ -76,10 +76,21 @@ describes.realWin('amp-analytics', {
     'triggers': [{'on': 'visible', 'request': 'foo'}],
   };
 
+  const noTriggersError = '[AmpAnalytics <unknown id>] No triggers were ' +
+      'found in the config. No analytics data will be sent.';
+  const noRequestStringsError = '[AmpAnalytics <unknown id>] No request ' +
+      'strings defined. Analytics data will not be sent from this page.';
+  const oneScriptChildError = '[AmpAnalytics <unknown id>] The tag should ' +
+      'contain only one <script> child.';
+  const configParseError = '[AmpAnalytics <unknown id>] Analytics config ' +
+      'could not be parsed. Is it in a valid JSON format? TypeError: Cannot ' +
+      'read property \'toUpperCase\' of null';
+  const onAndRequestAttributesError = '[AmpAnalytics <unknown id>] "on" and ' +
+      '"request" attributes are required for data to be collected.';
+  const remoteConfigError = '[amp-analytics] Remote configs are not allowed ' +
+      'to specify transport iframe';
+
   beforeEach(() => {
-    // Temporarily disable console error check until we have a solution
-    // to catch async console errors.
-    dontWarnForConsoleError();
     win = env.win;
     doc = win.document;
     ampdoc = env.ampdoc;
@@ -119,9 +130,6 @@ describes.realWin('amp-analytics', {
     });
   });
 
-  afterEach(() => {
-    warnForConsoleError();
-  });
 
   function getAnalyticsTag(config, attrs) {
     config = JSON.stringify(config);
@@ -200,6 +208,12 @@ describes.realWin('amp-analytics', {
       }
       actualResults[vendor] = {};
       describe('analytics vendor: ' + vendor, function() {
+        beforeEach(() => {
+          if (!config.triggers) {
+            expectAsyncConsoleError(noTriggersError);
+          }
+        });
+
         for (const name in config.requests) {
           it('should produce request: ' + name +
               '. If this test fails update vendor-requests.json', function* () {
@@ -350,6 +364,8 @@ describes.realWin('amp-analytics', {
   });
 
   it('does not send a hit when config is not in a script tag', function() {
+    expectAsyncConsoleError(noTriggersError);
+    expectAsyncConsoleError(noRequestStringsError);
     const config = JSON.stringify(trivialConfig);
     const el = doc.createElement('amp-analytics');
     el.textContent = config;
@@ -390,6 +406,9 @@ describes.realWin('amp-analytics', {
   });
 
   it('does not send a hit when multiple child tags exist', function() {
+    expectAsyncConsoleError(oneScriptChildError);
+    expectAsyncConsoleError(noRequestStringsError);
+    expectAsyncConsoleError(noTriggersError);
     const analytics = getAnalyticsTag(trivialConfig);
     const script2 = document.createElement('script');
     script2.setAttribute('type', 'application/json');
@@ -401,6 +420,9 @@ describes.realWin('amp-analytics', {
 
   it('does not send a hit when script tag does not have a type attribute',
       function() {
+        expectAsyncConsoleError(configParseError);
+        expectAsyncConsoleError(noRequestStringsError);
+        expectAsyncConsoleError(noTriggersError);
         const el = doc.createElement('amp-analytics');
         const script = doc.createElement('script');
         script.textContent = JSON.stringify(trivialConfig);
@@ -418,6 +440,7 @@ describes.realWin('amp-analytics', {
       });
 
   it('does not send a hit when request is not provided', function() {
+    expectAsyncConsoleError(onAndRequestAttributesError);
     const analytics = getAnalyticsTag({
       'requests': {'foo': 'https://example.com/bar'},
       'triggers': [{'on': 'visible'}],
@@ -429,6 +452,7 @@ describes.realWin('amp-analytics', {
   });
 
   it('does not send a hit when request type is not defined', function() {
+    expectAsyncConsoleError(noRequestStringsError);
     const analytics = getAnalyticsTag({
       'triggers': [{'on': 'visible', 'request': 'foo'}],
     });
@@ -1181,6 +1205,7 @@ describes.realWin('amp-analytics', {
   });
 
   it('ignore transport iframe from remote config', () => {
+    expectAsyncConsoleError(remoteConfigError);
     const analytics = getAnalyticsTag({
       'vars': {'title': 'local'},
       'requests': {'foo': 'https://example.com/${title}'},

diff --git a/test/_init_tests.js b/test/_init_tests.js
@@ -45,15 +45,14 @@ let consoleErrorSandbox;
 let consoleErrorStub;
 let consoleInfoLogWarnSandbox;
 let testName;
+let expectedAsyncErrors;
 
 // Used to clean up global state between tests.
 let initialGlobalState;
 let initialWindowState;
 
 // All exposed describes.
 global.describes = describes;
-global.dontWarnForConsoleError = dontWarnForConsoleError;
-global.warnForConsoleError = warnForConsoleError;
 
 // Increase the before/after each timeout since certain times they have timedout
 // during the normal 2000 allowance.
@@ -279,25 +278,46 @@ function warnForConsoleError() {
   if (consoleErrorSandbox) {
     consoleErrorSandbox.restore();
   }
+  expectedAsyncErrors = [];
   consoleErrorSandbox = sinon.sandbox.create();
   const originalConsoleError = console/*OK*/.error;
   consoleErrorSandbox.stub(console, 'error').callsFake((...messages) => {
-    const errorMessage = messages.join(' ').split('\n', 1)[0]; // First line.
+    const message = messages.join(' ');
+    if (expectedAsyncErrors.includes(message)) {
+      expectedAsyncErrors.splice(expectedAsyncErrors.indexOf(message), 1);
+      return;
+    } else {
+      // We're throwing an error. Clean up other expected errors since they will
+      // never appear.
+      expectedAsyncErrors = [];
+    }
+    const errorMessage = message.split('\n', 1)[0]; // First line.
+    const {failOnConsoleError} = window.__karma__.config;
+    const terminator = failOnConsoleError ? '\'' : '';
+    const separator = failOnConsoleError ? '\n' : '\'\n';
     const helpMessage = '    The test "' + testName + '"' +
-        ' resulted in a call to console.error.\n' +
-        '    ⤷ If this is not expected, fix the code that generated ' +
+        ' resulted in a call to console.error. (See above line.)\n' +
+        '    ⤷ If the error is not expected, fix the code that generated ' +
             'the error.\n' +
-        '    ⤷ If this is expected, use the following pattern to wrap the ' +
-            'test code that generated the error:\n' +
+        '    ⤷ If the error is expected (and synchronous), use the following ' +
+            'pattern to wrap the test code that generated the error:\n' +
         '        \'allowConsoleError(() => { <code that generated the ' +
-            'error> });';
+            'error> });\'\n' +
+        '    ⤷ If the error is expected (and asynchronous), use the ' +
+            'following pattern at the top of the test:\n' +
+        '        \'expectAsyncConsoleError(<error text>);' + terminator;
     // TODO(rsimha, #14406): Simply throw here after all tests are fixed.
-    if (window.__karma__.config.failOnConsoleError) {
-      throw new Error(errorMessage + '\'\n' + helpMessage);
+    if (failOnConsoleError) {
+      throw new Error(errorMessage + separator + helpMessage);
     } else {
-      originalConsoleError(errorMessage + '\'\n' + helpMessage);
+      originalConsoleError(errorMessage + separator + helpMessage);
     }
   });
+  this.expectAsyncConsoleError = function(message) {
+    if (!expectedAsyncErrors.includes(message)) {
+      expectedAsyncErrors.push(message);
+    }
+  };
   this.allowConsoleError = function(func) {
     dontWarnForConsoleError();
     const result = func();
@@ -327,6 +347,14 @@ function dontWarnForConsoleError() {
 // Used to restore error level logging after each test.
 function restoreConsoleError() {
   consoleErrorSandbox.restore();
+  if (expectedAsyncErrors.length > 0) {
+    const helpMessage =
+        'The test "' + testName + '" called "expectAsyncConsoleError", ' +
+        'but there were no call(s) to console.error with these message(s): ' +
+        '"' + expectedAsyncErrors.join('", "') + '"';
+    throw new Error(helpMessage);
+  }
+  expectedAsyncErrors = [];
 }
 
 // Used to silence info, log, and warn level logging during each test.