fix: conditionally log non-assertion errors (JS)

console.logs and errors are only reported during the first evaluation of
the user's code.  This is because the code is evaluated for each test,
but the logs will not change between the build phases of the tests.

Errors thrown during testing (except failing assertions) are always
reported. This is to inform the user that their code is faulty, rather
than that it does not meet the challenge's requirements.

client/src/client/workers/test-evaluator.js

@@ -24,22 +24,37 @@ const __utils = (() => {
   }
 
   const oldLog = self.console.log.bind(self.console);
-  self.console.log = function proxyConsole(...args) {
+  function proxyLog(...args) {
     logs.push(args.map(arg => '' + JSON.stringify(arg, replacer)).join(' '));
     if (logs.join('\n').length > MAX_LOGS_SIZE) {
       flushLogs();
     }
     return oldLog(...args);
-  };
+  }
 
+  // unless data.type is truthy, this sends data out to the testRunner
   function postResult(data) {
     flushLogs();
     self.postMessage(data);
   }
 
+  function logToBoth(err) {
+    if (!(err instanceof chai.AssertionError)) {
+      // report to both the browser and the fcc consoles, discarding the
+      // stack trace via toString as it only useful to debug the site, not a
+      // specific challenge.
+      console.log(err.toString());
+    }
+  }
+
+  const toggleLogging = on => {
+    self.console.log = on ? proxyLog : () => {};
+  };
+
   return {
     postResult,
-    oldLog
+    logToBoth,
+    toggleLogging
   };
 })();
 
@@ -50,6 +65,8 @@ self.onmessage = async e => {
   const assert = chai.assert;
   // Fake Deep Equal dependency
   const DeepEqual = (a, b) => JSON.stringify(a) === JSON.stringify(b);
+
+  __utils.toggleLogging(e.data.firstTest);
   /* eslint-enable no-unused-vars */
   try {
     let testResult;
@@ -65,14 +82,11 @@ self.onmessage = async e => {
       if (__userCodeWasExecuted) {
         // rethrow error, since test failed.
         throw err;
-      } else if (e.data.testString) {
-        // report errors to dev console if tests are running (since some
-        // challenges should pass with code that throws errors)
-        __utils.oldLog(err);
-      } else {
-        // user is editing code, so both consoles should report errors
-        console.log(err.toString());
       }
+      // log build errors
+      __utils.logToBoth(err);
+      // the tests may not require working code, so they are evaluated even if
+      // the user code does not get executed.
       testResult = eval(e.data.testString);
     }
     /* eslint-enable no-eval */
@@ -83,15 +97,17 @@ self.onmessage = async e => {
       pass: true
     });
   } catch (err) {
+    // report errors that happened during testing, so the user learns of
+    // execution errors and not just build errors.
+    __utils.toggleLogging(true);
+    __utils.logToBoth(err);
+    // postResult flushes the logs and must be called after logging is finished.
     __utils.postResult({
       err: {
         message: err.message,
         stack: err.stack
       }
     });
-    if (!(err instanceof chai.AssertionError)) {
-      console.error(err);
-    }
   }
 };
 

client/src/templates/Challenges/redux/execute-challenge-saga.js

@@ -92,10 +92,18 @@ function* buildChallengeData(challengeData) {
 
 function* executeTests(testRunner, tests, testTimeout = 5000) {
   const testResults = [];
-  for (const { text, testString } of tests) {
+  for (let i = 0; i < tests.length; i++) {
+    const { text, testString } = tests[i];
     const newTest = { text, testString };
+    // only the last test outputs console.logs to avoid log duplication.
+    const firstTest = i === 1;
     try {
-      const { pass, err } = yield call(testRunner, testString, testTimeout);
+      const { pass, err } = yield call(
+        testRunner,
+        testString,
+        testTimeout,
+        firstTest
+      );
       if (pass) {
         newTest.pass = true;
       } else {

client/src/templates/Challenges/utils/build.js

@@ -99,9 +99,9 @@ function getJSTestRunner({ build, sources }, proxyLogger) {
 
   const testWorker = createWorker(testEvaluator, { terminateWorker: true });
 
-  return (testString, testTimeout) => {
+  return (testString, testTimeout, firstTest = true) => {
     return testWorker
-      .execute({ build, testString, code, sources }, testTimeout)
+      .execute({ build, testString, code, sources, firstTest }, testTimeout)
       .on('LOG', proxyLogger).done;
   };
 }