From 0f3f27287d11ce7373ad9450cb47d554795aab17 Mon Sep 17 00:00:00 2001 From: Oliver Eyton-Williams Date: Mon, 13 Jul 2020 18:59:40 +0200 Subject: [PATCH] fix: unify single and multifile testing --- curriculum/schema/challengeSchema.js | 16 ++--- curriculum/test/test-challenges.js | 61 +++++++++---------- .../challengeSeed-to-data.js | 50 +++++++-------- tools/challenge-md-parser/solution-to-data.js | 35 +++++++---- 4 files changed, 87 insertions(+), 75 deletions(-) diff --git a/curriculum/schema/challengeSchema.js b/curriculum/schema/challengeSchema.js index de6ad85427..ff520fe7bc 100644 --- a/curriculum/schema/challengeSchema.js +++ b/curriculum/schema/challengeSchema.js @@ -72,13 +72,15 @@ function getSchemaForLang(lang) { crossDomain: Joi.bool() }) ), - solutions: Joi.array().items(Joi.string().optional()), - solutionFiles: Joi.object().keys({ - indexcss: fileJoi, - indexhtml: fileJoi, - indexjs: fileJoi, - indexjsx: fileJoi - }), + solutions: Joi.array().items( + Joi.object().keys({ + indexcss: fileJoi, + indexhtml: fileJoi, + indexjs: fileJoi, + indexjsx: fileJoi, + indexpy: fileJoi + }) + ), superBlock: Joi.string(), superOrder: Joi.number(), suborder: Joi.number(), diff --git a/curriculum/test/test-challenges.js b/curriculum/test/test-challenges.js index 9cae567b65..e94e1d7e3a 100644 --- a/curriculum/test/test-challenges.js +++ b/curriculum/test/test-challenges.js @@ -360,31 +360,39 @@ function populateTestsForLang({ lang, challenges, meta }) { assert(fails, 'Test suit does not fail on the initial contents'); }); - let { solutions = [], solutionFiles = {} } = challenge; + let { solutions = [] } = challenge; - const noSolution = new RegExp('// solution required'); - solutions = solutions.filter( - solution => !!solution && !noSolution.test(solution) - ); - - // if there are no solutions in the challenge, it's assumed the next - // challenge's seed will be a solution to the current challenge. - // This is expected to happen in the project based curriculum. + // if there's an empty string as solution, this is likely a mistake + // TODO: what does this look like now? if (isEmpty(solutions)) { - if (!isEmpty(solutionFiles)) { - solutions = [solutionFiles]; - // TODO: there needs to be a way of telling that a challenge uses - // multiple files when it doesn't have anything in solutionFiles! + // if there are no solutions in the challenge, it's assumed the next + // challenge's seed will be a solution to the current challenge. + // This is expected to happen in the project based curriculum. + + const nextChallenge = challenges[id + 1]; + // TODO: check this actually works... + if (nextChallenge) { + solutions = [nextChallenge.files]; } else { - const nextChallenge = challenges[id + 1]; - if (nextChallenge) { - solutions = [nextChallenge.files[0].contents]; - } + throw Error('solution omitted'); } } - if (solutions.length === 0 && isEmpty(solutionFiles)) { + // TODO: the no-solution filtering is a little convoluted: + const noSolution = new RegExp('// solution required'); + + const solutionsAsArrays = solutions.map(toSortedArray); + + const filteredSolutions = solutionsAsArrays.filter(solution => { + return !isEmpty( + solution.filter(file => !noSolution.test(file.contents)) + ); + }); + + // console.log('filteredSolutions', filteredSolutions); + + if (isEmpty(filteredSolutions)) { it('Check tests. No solutions'); return; } @@ -415,20 +423,9 @@ async function createTestRunner(challenge, solution, buildChallenge) { // we should avoid modifying challenge, as it gets reused: const files = cloneDeep(challenge.files); - // TODO: there must be a better way of handling both single and multi-file - // solutions - if (typeof solution === 'object' && !isEmpty(solution)) { - Object.keys(solution).forEach(key => { - files[key].contents = solution[key].contents; - }); - } else if (solution) { - // fallback for single solution - const sortedFiles = toSortedArray(files); - - files[sortedFiles[0].key].contents = solution; - } else { - throw Error('Tried to create test runner without a solution.'); - } + Object.keys(solution).forEach(key => { + files[key].contents = solution[key].contents; + }); const { build, sources, loadEnzyme } = await buildChallenge({ files, diff --git a/tools/challenge-md-parser/challengeSeed-to-data.js b/tools/challenge-md-parser/challengeSeed-to-data.js index 1d69816256..c7170131dc 100644 --- a/tools/challenge-md-parser/challengeSeed-to-data.js +++ b/tools/challenge-md-parser/challengeSeed-to-data.js @@ -19,22 +19,23 @@ function defaultFile(lang) { tail: '' }; } -function createCodeGetter(key, regEx, seeds) { +function createCodeGetter(codeKey, regEx, seeds) { return container => { const { properties: { id } } = container; const lang = id.match(regEx)[1]; + const key = `index${lang}`; const code = select('code', container).children[0].value; - if (lang in seeds) { - seeds[lang] = { - ...seeds[lang], - [key]: code + if (key in seeds) { + seeds[key] = { + ...seeds[key], + [codeKey]: code }; } else { - seeds[lang] = { + seeds[key] = { ...defaultFile(lang), - [key]: code + [codeKey]: code }; } }; @@ -89,30 +90,28 @@ function createPlugin() { file.data = { ...file.data, - files: Object.keys(seeds).map(lang => seeds[lang]) + files: seeds }; // TODO: make this readable. - if (file.data.files) { - file.data.files.forEach(fileData => { - const editRegionMarkers = findRegionMarkers(fileData); - if (editRegionMarkers) { - fileData.contents = removeLines( - fileData.contents, - editRegionMarkers - ); + Object.keys(seeds).forEach(key => { + const fileData = seeds[key]; + const editRegionMarkers = findRegionMarkers(fileData); + if (editRegionMarkers) { + fileData.contents = removeLines( + fileData.contents, + editRegionMarkers + ); - if (editRegionMarkers[1] <= editRegionMarkers[0]) { - throw Error('Editable region must be non zero'); - } - fileData.editableRegionBoundaries = editRegionMarkers; - } else { - fileData.editableRegionBoundaries = []; + if (editRegionMarkers[1] <= editRegionMarkers[0]) { + throw Error('Editable region must be non zero'); } - }); - } - + fileData.editableRegionBoundaries = editRegionMarkers; + } else { + fileData.editableRegionBoundaries = []; + } + }); // TODO: TESTS! } } @@ -122,3 +121,4 @@ function createPlugin() { exports.challengeSeedToData = createPlugin; exports.createCodeGetter = createCodeGetter; +exports.defaultFile = defaultFile; diff --git a/tools/challenge-md-parser/solution-to-data.js b/tools/challenge-md-parser/solution-to-data.js index 5740a4a4b1..cd3fc5ae6c 100644 --- a/tools/challenge-md-parser/solution-to-data.js +++ b/tools/challenge-md-parser/solution-to-data.js @@ -2,23 +2,27 @@ const visit = require('unist-util-visit'); const { selectAll } = require('hast-util-select'); const { sectionFilter } = require('./utils'); -const { createCodeGetter } = require('./challengeSeed-to-data'); +const { createCodeGetter, defaultFile } = require('./challengeSeed-to-data'); +const { isEmpty } = require('lodash'); const solutionRE = /(.+)-solution$/; +function indexByKey(obj) { + return { [obj.key]: { ...obj } }; +} + function createPlugin() { return function transformer(tree, file) { function visitor(node) { if (sectionFilter(node, 'solution')) { // fallback for single-file challenges - const solutions = selectAll('code', node).map( - element => element.children[0].value - ); - file.data = { - ...file.data, - solutions - }; + const rawSolutions = selectAll('code', node).map(element => ({ + lang: element.properties.className[0].split('-')[1], + contents: element.children[0].value + })); + const solutionFiles = {}; + const codeDivs = selectAll('div', node); const solutionContainers = codeDivs.filter(({ properties: { id } }) => solutionRE.test(id) @@ -27,11 +31,20 @@ function createPlugin() { createCodeGetter('contents', solutionRE, solutionFiles) ); + const solutionsAsFiles = rawSolutions + .map(({ lang, contents }) => ({ + ...defaultFile(lang), + contents + })) + .map(indexByKey); + + const solutions = isEmpty(solutionFiles) + ? solutionsAsFiles + : [solutionFiles]; + file.data = { ...file.data, - solutionFiles: Object.keys(solutionFiles).map( - lang => solutionFiles[lang] - ) + solutions }; } }