🏗🐛 Print uncaught errors seen during tests (instead of rethrowing the…

…m) (#17213)
ampproject · Jul 31, 2018 · 2613175 · 2613175
1 parent f5f31d3
commit 2613175
Show file tree

Hide file tree

Showing 3 changed files with 4 additions and 27 deletions.
diff --git a/build-system/tasks/karma.conf.js b/build-system/tasks/karma.conf.js
@@ -227,9 +227,6 @@ module.exports = {
       // Longer timeout on Travis; fail quickly at local.
       timeout: process.env.TRAVIS ? 10000 : 2000,
     },
-    // TODO(rsimha, #14406): Remove this after all tests are fixed.
-    failOnConsoleError: !process.env.TRAVIS && !process.env.LOCAL_PR_CHECK,
-    // TODO(rsimha, #14432): Set to false after all tests are fixed.
     captureConsole: true,
     verboseLogging: false,
   },

diff --git a/build-system/tasks/runtime-test.js b/build-system/tasks/runtime-test.js
@@ -40,7 +40,6 @@ const {green, yellow, cyan, red, bold} = colors;
 const preTestTasks = argv.nobuild ? [] : (
   (argv.unit || argv.a4a || argv['local-changes']) ? ['css'] : ['build']);
 const ampConfig = (argv.config === 'canary') ? 'canary' : 'prod';
-const tooManyTestsToFix = 15;
 const extensionsCssMapPath = 'EXTENSIONS_CSS_MAP';
 
 /**
@@ -458,9 +457,6 @@ function runTests() {
       });
     }
     c.files = c.files.concat(config.commonUnitTestPaths, testsToRun);
-    if (testsToRun.length < tooManyTestsToFix) {
-      c.client.failOnConsoleError = true;
-    }
   } else if (argv.integration) {
     c.files = c.files.concat(
         config.commonIntegrationTestPaths, config.integrationTestPaths);
@@ -478,10 +474,6 @@ function runTests() {
     c.files = c.files.concat(config.testPaths);
   }
 
-  if (argv.travis_like) {
-    c.client.failOnConsoleError = false;
-  }
-
   // c.client is available in test browser via window.parent.karma.config
   c.client.amp = {
     useCompiledJs: !!argv.compiled,

diff --git a/test/_init_tests.js b/test/_init_tests.js
@@ -315,14 +315,7 @@ function warnForConsoleError() {
       }
     }
 
-    // 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. (See above line.)\n' +
         '    ⤷ If the error is not expected, fix the code that generated ' +
@@ -333,14 +326,9 @@ function warnForConsoleError() {
             'error> });\'\n' +
         '    ⤷ If the error is expected (and asynchronous), use the ' +
             'following pattern at the top of the test:\n' +
-        '        \'expectAsyncConsoleError(<string or regex>[, number of' +
-        ' times the error message repeats]);' + terminator;
-    // TODO(rsimha, #14406): Simply throw here after all tests are fixed.
-    if (failOnConsoleError) {
-      throw new Error(errorMessage + separator + helpMessage);
-    } else {
-      originalConsoleError(errorMessage + separator + helpMessage);
-    }
+        '        \'expectAsyncConsoleError(<string or regex>[, <number of' +
+        ' times the error message repeats>]);';
+    originalConsoleError(errorMessage + '\'\n' + helpMessage);
   });
   this.expectAsyncConsoleError = function(message, repeat = 1) {
     expectedAsyncErrors.push.apply(
@@ -355,7 +343,7 @@ function warnForConsoleError() {
       const helpMessage =
           'The test "' + testName + '" contains an "allowConsoleError" block ' +
           'that didn\'t result in a call to console.error.';
-      throw new Error(helpMessage);
+      originalConsoleError(helpMessage);
     }
     warnForConsoleError();
     return result;