Feat: editable dom element (#39341)

* feat: __testEditable allows editable region tests

It's not entirely isolated, but it makes it possible to select only the
element with id 'editable-only' which is built solely from code inside
the editable region.

* fix(client): missing editableContents -> ''

Previously it was added as the string 'undefined'

* fix: more informative error messages

* fix: DRY, correct and test getLines

client/src/client/frame-runner.js

@@ -7,8 +7,20 @@ document.__initTestFrame = initTestFrame;
 
 async function initTestFrame(e = { code: {} }) {
   const code = (e.code.contents || '').slice();
-  // eslint-disable-next-line no-unused-vars
   const editableContents = (e.code.editableContents || '').slice();
+  // __testEditable allows test authors to run tests against a transitory dom
+  // element built using only the code in the editable region.
+  // eslint-disable-next-line no-unused-vars
+  const __testEditable = cb => {
+    const div = document.createElement('div');
+    div.id = 'editable-only';
+    div.innerHTML = editableContents;
+    document.body.appendChild(div);
+    const out = cb();
+    document.body.removeChild(div);
+    return out;
+  };
+
   if (!e.getUserInput) {
     e.getUserInput = () => code;
   }
@@ -85,12 +97,8 @@ async function initTestFrame(e = { code: {} }) {
       if (!(err instanceof chai.AssertionError)) {
         console.error(err);
       }
-      return {
+      // return the error so that the curriculum tests are more informative
-        err: {
+      return err;
-          message: err.message,
-          stack: err.stack
-        }
-      };
     }
   };
 }

client/src/templates/Challenges/redux/index.js

@@ -3,6 +3,7 @@ import { createAction, handleActions } from 'redux-actions';
 import { createTypes } from '../../../../utils/stateManagement';
 
 import { createPoly } from '../../../../../utils/polyvinyl';
+import { getLines } from '../../../../../utils/get-lines';
 import challengeModalEpic from './challenge-modal-epic';
 import completionEpic from './completion-epic';
 import codeLockEpic from './code-lock-epic';
@@ -131,18 +132,6 @@ export const createFiles = createAction(types.createFiles, challengeFiles =>
     )
 );
 
-// TODO: secure with tests
-function getLines(contents, range) {
-  if (isEmpty(range)) {
-    return '';
-  }
-  const lines = contents.split('\n');
-  const editableLines = isEmpty(lines)
-    ? []
-    : lines.slice(range[0], range[1] - 1);
-  return editableLines.join('\n');
-}
-
 export const createQuestion = createAction(types.createQuestion);
 export const initTests = createAction(types.initTests);
 export const updateTests = createAction(types.updateTests);

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

@@ -58,7 +58,7 @@ function buildSourceMap(files) {
   return files.reduce(
     (sources, file) => {
       sources[file.name] += file.source || file.contents;
-      sources.editableContents += file.editableContents;
+      sources.editableContents += file.editableContents || '';
       return sources;
     },
     { index: '', editableContents: '' }

curriculum/test/test-challenges.js

@@ -24,6 +24,7 @@ const {
 const { assert, AssertionError } = require('chai');
 const Mocha = require('mocha');
 const { flatten, isEmpty, cloneDeep } = require('lodash');
+const { getLines } = require('../../utils/get-lines');
 
 const jsdom = require('jsdom');
 
@@ -363,7 +364,12 @@ function populateTestsForLang({ lang, challenges, meta }) {
           let { solutions = [] } = challenge;
 
           // if there's an empty string as solution, this is likely a mistake
-          // TODO: what does this look like now?
+          // TODO: what does this look like now? (this being detection of empty
+          // lines in solutions - rather than entirely missing solutions)
+
+          // We need to track where the solution came from to give better
+          // feedback if the solution is failing.
+          let solutionFromNext = false;
 
           if (isEmpty(solutions)) {
             // if there are no solutions in the challenge, it's assumed the next
@@ -371,9 +377,19 @@ function populateTestsForLang({ lang, challenges, meta }) {
             // This is expected to happen in the project based curriculum.
 
             const nextChallenge = challenges[id + 1];
-            // TODO: check this actually works...
+            // TODO: can this be dried out, ideally by removing the redux
+            // handler?
             if (nextChallenge) {
-              solutions = [nextChallenge.files];
+              const solutionFiles = cloneDeep(nextChallenge.files);
+              Object.keys(solutionFiles).forEach(key => {
+                const file = solutionFiles[key];
+                file.editableContents = getLines(
+                  file.contents,
+                  challenge.files[key].editableRegionBoundaries
+                );
+              });
+              solutions = [solutionFiles];
+              solutionFromNext = true;
             } else {
               throw Error('solution omitted');
             }
@@ -390,8 +406,6 @@ function populateTestsForLang({ lang, challenges, meta }) {
             );
           });
 
-          // console.log('filteredSolutions', filteredSolutions);
-
           if (isEmpty(filteredSolutions)) {
             it('Check tests. No solutions');
             return;
@@ -404,7 +418,8 @@ function populateTestsForLang({ lang, challenges, meta }) {
                 const testRunner = await createTestRunner(
                   challenge,
                   solution,
-                  buildChallenge
+                  buildChallenge,
+                  solutionFromNext
                 );
                 for (const test of tests) {
                   await testRunner(test);
@@ -418,13 +433,19 @@ function populateTestsForLang({ lang, challenges, meta }) {
   });
 }
 
-async function createTestRunner(challenge, solution, buildChallenge) {
+async function createTestRunner(
+  challenge,
+  solution,
+  buildChallenge,
+  solutionFromNext
+) {
   const { required = [], template } = challenge;
   // we should avoid modifying challenge, as it gets reused:
   const files = cloneDeep(challenge.files);
 
   Object.keys(solution).forEach(key => {
     files[key].contents = solution[key].contents;
+    files[key].editableContents = solution[key].editableContents;
   });
 
   const { build, sources, loadEnzyme } = await buildChallenge({
@@ -448,7 +469,11 @@ async function createTestRunner(challenge, solution, buildChallenge) {
         throw new AssertionError(err.message);
       }
     } catch (err) {
-      reThrow(err, text);
+      text = 'Test text: ' + text;
+      const message = solutionFromNext
+        ? 'Check next step for solution!\n' + text
+        : text;
+      reThrow(err, message);
     }
   };
 }
@@ -493,14 +518,11 @@ async function initializeTestRunner(build, sources, code, loadEnzyme) {
 }
 
 function reThrow(err, text) {
-  if (typeof err === 'string') {
+  const newMessage = `${text}
-    throw new AssertionError(
+  ${err.message}`;
-      `${text}
+  if (err.name === 'AssertionError') {
-         ${err}`
+    throw new AssertionError(newMessage);
-    );
   } else {
-    err.message = `${text}
+    throw Error(newMessage);
-       ${err.message}`;
-    throw err;
   }
 }

utils/get-lines.js

@@ -0,0 +1,16 @@
+const { isEmpty } = require('lodash');
+
+// TODO: secure with tests
+function getLines(contents, range) {
+  if (isEmpty(range)) {
+    return '';
+  }
+  const lines = contents.split('\n');
+  const editableLines =
+    isEmpty(lines) || range[1] <= range[0]
+      ? []
+      : lines.slice(range[0], range[1] - 1);
+  return editableLines.join('\n');
+}
+
+exports.getLines = getLines;

utils/get-lines.test.js

@@ -0,0 +1,35 @@
+/* global describe expect it */
+
+const { getLines } = require('./get-lines');
+
+const content = 'one\ntwo\nthree';
+
+describe('dasherize', () => {
+  it('returns a string', () => {
+    expect(getLines('')).toBe('');
+  });
+  it("returns '' when the second arg is empty", () => {
+    expect(getLines(content)).toBe('');
+  });
+  it("returns '' when the range is negative", () => {
+    expect(getLines(content, [1, -1])).toBe('');
+  });
+  it("returns '' when the range is [n,n]", () => {
+    expect(getLines(content, [0, 0])).toBe('');
+    expect(getLines(content, [1, 1])).toBe('');
+    expect(getLines(content, [2, 2])).toBe('');
+  });
+
+  it('returns the first line when the range is [0,2]', () => {
+    expect(getLines(content, [0, 2])).toBe('one');
+  });
+  it('returns the second line when the range is [1,3]', () => {
+    expect(getLines(content, [1, 3])).toBe('two');
+  });
+  it('returns the first and second lines when the range is [0,3]', () => {
+    expect(getLines(content, [0, 3])).toBe('one\ntwo');
+  });
+  it('returns the second and third lines when the range is [1,4]', () => {
+    expect(getLines(content, [1, 4])).toBe('two\nthree');
+  });
+});