diff --git a/client/src/client/frame-runner.js b/client/src/client/frame-runner.js index c8a2430af2..bff1a2d1da 100644 --- a/client/src/client/frame-runner.js +++ b/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 { - err: { - message: err.message, - stack: err.stack - } - }; + // return the error so that the curriculum tests are more informative + return err; } }; } diff --git a/client/src/templates/Challenges/redux/index.js b/client/src/templates/Challenges/redux/index.js index 52f88c666d..38d6449c51 100644 --- a/client/src/templates/Challenges/redux/index.js +++ b/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); diff --git a/client/src/templates/Challenges/utils/build.js b/client/src/templates/Challenges/utils/build.js index 1486c58a8f..02965c3573 100644 --- a/client/src/templates/Challenges/utils/build.js +++ b/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: '' } diff --git a/curriculum/test/test-challenges.js b/curriculum/test/test-challenges.js index 3fffe9fd29..3d613169b8 100644 --- a/curriculum/test/test-challenges.js +++ b/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') { - throw new AssertionError( - `${text} - ${err}` - ); + const newMessage = `${text} + ${err.message}`; + if (err.name === 'AssertionError') { + throw new AssertionError(newMessage); } else { - err.message = `${text} - ${err.message}`; - throw err; + throw Error(newMessage); } } diff --git a/utils/get-lines.js b/utils/get-lines.js new file mode 100644 index 0000000000..be3e154626 --- /dev/null +++ b/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; diff --git a/utils/get-lines.test.js b/utils/get-lines.test.js new file mode 100644 index 0000000000..c9a46bf7c5 --- /dev/null +++ b/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'); + }); +});