From febba792e7c19109bd0a8dedf098d85d7617e344 Mon Sep 17 00:00:00 2001 From: Oliver Eyton-Williams Date: Tue, 19 Nov 2019 12:46:48 +0100 Subject: [PATCH] fix: allow log in testString, restrict test errors Console logs from testString get reported and test errors are sent to the dev console (JS). challenge building is only attempted if there is a build function to do so. Various functions have been renamed to better reflect what they do. --- client/src/client/workers/test-evaluator.js | 24 ++++++++++--------- .../redux/execute-challenge-saga.js | 21 +++++++++------- .../src/templates/Challenges/utils/build.js | 5 ++++ .../src/templates/Challenges/utils/frame.js | 4 ++-- 4 files changed, 32 insertions(+), 22 deletions(-) diff --git a/client/src/client/workers/test-evaluator.js b/client/src/client/workers/test-evaluator.js index c13b3c3b79..78d309cf41 100644 --- a/client/src/client/workers/test-evaluator.js +++ b/client/src/client/workers/test-evaluator.js @@ -38,7 +38,7 @@ const __utils = (() => { self.postMessage(data); } - function logToBoth(err) { + function log(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 @@ -47,14 +47,14 @@ const __utils = (() => { } } - const toggleLogging = on => { - self.console.log = on ? proxyLog : () => {}; + const toggleProxyLogger = on => { + self.console.log = on ? proxyLog : oldLog; }; return { postResult, - logToBoth, - toggleLogging + log, + toggleProxyLogger }; })(); @@ -66,7 +66,8 @@ self.onmessage = async e => { // Fake Deep Equal dependency const DeepEqual = (a, b) => JSON.stringify(a) === JSON.stringify(b); - __utils.toggleLogging(e.data.firstTest); + // User code errors should be reported, but only once: + __utils.toggleProxyLogger(e.data.firstTest); /* eslint-enable no-unused-vars */ try { let testResult; @@ -84,7 +85,7 @@ self.onmessage = async e => { throw err; } // log build errors - __utils.logToBoth(err); + __utils.log(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); @@ -97,10 +98,11 @@ 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); + // Errors from testing go to the browser console only. + __utils.toggleProxyLogger(false); + // Report execution errors in case user code has errors that are only + // uncovered during testing. + __utils.log(err); // postResult flushes the logs and must be called after logging is finished. __utils.postResult({ err: { diff --git a/client/src/templates/Challenges/redux/execute-challenge-saga.js b/client/src/templates/Challenges/redux/execute-challenge-saga.js index 3ab11d8a6c..477cf0dbf5 100644 --- a/client/src/templates/Challenges/redux/execute-challenge-saga.js +++ b/client/src/templates/Challenges/redux/execute-challenge-saga.js @@ -26,6 +26,7 @@ import { import { buildChallenge, + canBuildChallenge, getTestRunner, challengeHasPreview, updatePreview, @@ -153,15 +154,17 @@ function* previewChallengeSaga() { yield fork(takeEveryConsole, logProxy); const challengeData = yield select(challengeDataSelector); - const buildData = yield buildChallengeData(challengeData); - // evaluate the user code in the preview frame or in the worker - if (challengeHasPreview(challengeData)) { - const document = yield getContext('document'); - yield call(updatePreview, buildData, document, proxyLogger); - } else if (isJavaScriptChallenge(challengeData)) { - const runUserCode = getTestRunner(buildData, { proxyLogger }); - // without a testString the testRunner just evaluates the user's code - yield call(runUserCode, null, 5000); + if (canBuildChallenge(challengeData)) { + const buildData = yield buildChallengeData(challengeData); + // evaluate the user code in the preview frame or in the worker + if (challengeHasPreview(challengeData)) { + const document = yield getContext('document'); + yield call(updatePreview, buildData, document, proxyLogger); + } else if (isJavaScriptChallenge(challengeData)) { + const runUserCode = getTestRunner(buildData, { proxyLogger }); + // without a testString the testRunner just evaluates the user's code + yield call(runUserCode, null, 5000); + } } } catch (err) { console.log(err); diff --git a/client/src/templates/Challenges/utils/build.js b/client/src/templates/Challenges/utils/build.js index cc2529c18f..e38188c7e5 100644 --- a/client/src/templates/Challenges/utils/build.js +++ b/client/src/templates/Challenges/utils/build.js @@ -71,6 +71,11 @@ const buildFunctions = { [challengeTypes.backEndProject]: buildBackendChallenge }; +export function canBuildChallenge(challengeData) { + const { challengeType } = challengeData; + return buildFunctions.hasOwnProperty(challengeType); +} + export async function buildChallenge(challengeData) { const { challengeType } = challengeData; let build = buildFunctions[challengeType]; diff --git a/client/src/templates/Challenges/utils/frame.js b/client/src/templates/Challenges/utils/frame.js index 5badae5165..cf1cd9b040 100644 --- a/client/src/templates/Challenges/utils/frame.js +++ b/client/src/templates/Challenges/utils/frame.js @@ -104,8 +104,8 @@ const initTestFrame = frameReady => ctx => { const initMainFrame = (frameReady, proxyLogger) => ctx => { waitForFrame(ctx).then(() => { // Overwriting the onerror added by createHeader to catch any errors thrown - // after the frame is ready. It has to be overwritten, as proxyUpdateConsole - // cannot be added as part of createHeader. + // after the frame is ready. It has to be overwritten, as proxyLogger cannot + // be added as part of createHeader. ctx.window.onerror = function(msg) { console.log(msg); if (proxyLogger) {