From 9089ddca5caa423808d966ed77f542e177f327c7 Mon Sep 17 00:00:00 2001 From: Oliver Eyton-Williams Date: Fri, 28 Aug 2020 17:10:37 +0200 Subject: [PATCH] fix: use location for language, not extension Rather than relying on .lang.md this expects to find the English source challenge in /curriculum/challenges/english/ --- .../challenge-stripped.md} | 0 .../challenge.md} | 0 .../challenge-html-comments.md} | 0 .../challenge-js-comments.md} | 0 .../challenge-jsx-comments.md} | 0 .../challenge.md} | 0 curriculum/getChallenges.acceptance.test.js | 32 +-- curriculum/getChallenges.js | 189 ++++++++---------- curriculum/getChallenges.test.js | 83 ++++---- 9 files changed, 138 insertions(+), 166 deletions(-) rename curriculum/__fixtures__/{challenge-stripped.chinese.md => chinese/challenge-stripped.md} (100%) rename curriculum/__fixtures__/{challenge.chinese.md => chinese/challenge.md} (100%) rename curriculum/__fixtures__/{challenge-html-comments.english.md => english/challenge-html-comments.md} (100%) rename curriculum/__fixtures__/{challenge-js-comments.english.md => english/challenge-js-comments.md} (100%) rename curriculum/__fixtures__/{challenge-jsx-comments.english.md => english/challenge-jsx-comments.md} (100%) rename curriculum/__fixtures__/{challenge.english.md => english/challenge.md} (100%) diff --git a/curriculum/__fixtures__/challenge-stripped.chinese.md b/curriculum/__fixtures__/chinese/challenge-stripped.md similarity index 100% rename from curriculum/__fixtures__/challenge-stripped.chinese.md rename to curriculum/__fixtures__/chinese/challenge-stripped.md diff --git a/curriculum/__fixtures__/challenge.chinese.md b/curriculum/__fixtures__/chinese/challenge.md similarity index 100% rename from curriculum/__fixtures__/challenge.chinese.md rename to curriculum/__fixtures__/chinese/challenge.md diff --git a/curriculum/__fixtures__/challenge-html-comments.english.md b/curriculum/__fixtures__/english/challenge-html-comments.md similarity index 100% rename from curriculum/__fixtures__/challenge-html-comments.english.md rename to curriculum/__fixtures__/english/challenge-html-comments.md diff --git a/curriculum/__fixtures__/challenge-js-comments.english.md b/curriculum/__fixtures__/english/challenge-js-comments.md similarity index 100% rename from curriculum/__fixtures__/challenge-js-comments.english.md rename to curriculum/__fixtures__/english/challenge-js-comments.md diff --git a/curriculum/__fixtures__/challenge-jsx-comments.english.md b/curriculum/__fixtures__/english/challenge-jsx-comments.md similarity index 100% rename from curriculum/__fixtures__/challenge-jsx-comments.english.md rename to curriculum/__fixtures__/english/challenge-jsx-comments.md diff --git a/curriculum/__fixtures__/challenge.english.md b/curriculum/__fixtures__/english/challenge.md similarity index 100% rename from curriculum/__fixtures__/challenge.english.md rename to curriculum/__fixtures__/english/challenge.md diff --git a/curriculum/getChallenges.acceptance.test.js b/curriculum/getChallenges.acceptance.test.js index 335aa09b31..2a2eece528 100644 --- a/curriculum/getChallenges.acceptance.test.js +++ b/curriculum/getChallenges.acceptance.test.js @@ -16,9 +16,10 @@ describe('translation parser', () => { return Promise.all([ parseMarkdown(path.resolve(__dirname, '__fixtures__/combined.md')), parseTranslation( - path.resolve(__dirname, '__fixtures__/challenge.english.md'), - path.resolve(__dirname, '__fixtures__/challenge.chinese.md'), - SIMPLE_TRANSLATION + path.resolve(__dirname, '__fixtures__/english/challenge.md'), + path.resolve(__dirname, '__fixtures__/chinese/challenge.md'), + SIMPLE_TRANSLATION, + 'chinese' ) ]).then(xs => expect(xs[1]).toEqual(xs[0])); }); @@ -30,10 +31,11 @@ describe('translation parser', () => { parseTranslation( path.resolve( __dirname, - '__fixtures__/challenge-html-comments.english.md' + '__fixtures__/english/challenge-html-comments.md' ), - path.resolve(__dirname, '__fixtures__/challenge.chinese.md'), - SIMPLE_TRANSLATION + path.resolve(__dirname, '__fixtures__/chinese/challenge.md'), + SIMPLE_TRANSLATION, + 'chinese' ) ]).then(xs => expect(xs[1]).toEqual(xs[0])); }); @@ -45,10 +47,11 @@ describe('translation parser', () => { parseTranslation( path.resolve( __dirname, - '__fixtures__/challenge-jsx-comments.english.md' + '__fixtures__/english/challenge-jsx-comments.md' ), - path.resolve(__dirname, '__fixtures__/challenge.chinese.md'), - SIMPLE_TRANSLATION + path.resolve(__dirname, '__fixtures__/chinese/challenge.md'), + SIMPLE_TRANSLATION, + 'chinese' ) ]).then(xs => expect(xs[1]).toEqual(xs[0])); }); @@ -60,10 +63,11 @@ describe('translation parser', () => { parseTranslation( path.resolve( __dirname, - '__fixtures__/challenge-js-comments.english.md' + '__fixtures__/english/challenge-js-comments.md' ), - path.resolve(__dirname, '__fixtures__/challenge.chinese.md'), - SIMPLE_TRANSLATION + path.resolve(__dirname, '__fixtures__/chinese/challenge.md'), + SIMPLE_TRANSLATION, + 'chinese' ) ]).then(xs => expect(xs[1]).toEqual(xs[0])); }); @@ -71,8 +75,8 @@ describe('translation parser', () => { return Promise.all([ parseMarkdown(path.resolve(__dirname, '__fixtures__/combined.md')), parseTranslation( - path.resolve(__dirname, '__fixtures__/challenge.english.md'), - path.resolve(__dirname, '__fixtures__/challenge-stripped.chinese.md'), + path.resolve(__dirname, '__fixtures__/english/challenge.md'), + path.resolve(__dirname, '__fixtures__/chinese/challenge-stripped.md'), SIMPLE_TRANSLATION ) ]).then(xs => expect(xs[1]).toEqual(xs[0])); diff --git a/curriculum/getChallenges.js b/curriculum/getChallenges.js index 4be22455a8..ff62305906 100644 --- a/curriculum/getChallenges.js +++ b/curriculum/getChallenges.js @@ -3,6 +3,7 @@ const { findIndex, reduce, toString } = require('lodash'); const readDirP = require('readdirp-walk'); const { parseMarkdown } = require('../tools/challenge-md-parser'); const fs = require('fs'); +const util = require('util'); /* eslint-disable max-len */ const { mergeChallenges, @@ -17,6 +18,8 @@ const { createPoly } = require('../utils/polyvinyl'); const { blockNameify } = require('../utils/block-nameify'); const { supportedLangs } = require('./utils'); +const access = util.promisify(fs.access); + const challengesDir = path.resolve(__dirname, './challenges'); const metaDir = path.resolve(challengesDir, '_meta'); exports.challengesDir = challengesDir; @@ -47,14 +50,15 @@ exports.getChallengesForLang = function getChallengesForLang(lang) { readDirP({ root: getChallengesDirForLang(lang) }) .on('data', file => { running++; - buildCurriculum(file, curriculum).then(done); + buildCurriculum(file, curriculum, lang).then(done); }) .on('end', done); }); }; -async function buildCurriculum(file, curriculum) { - const { name, depth, path: filePath, fullPath, stat } = file; +async function buildCurriculum(file, curriculum, lang) { + const { name, depth, path: filePath, stat } = file; + const createChallenge = createChallengeCreator(challengesDir, lang); if (depth === 1 && stat.isDirectory()) { // extract the superBlock info const { order, name: superBlock } = superBlockInfo(name); @@ -106,79 +110,85 @@ async function buildCurriculum(file, curriculum) { } const { meta } = challengeBlock; - const challenge = await createChallenge(fullPath, meta); + const challenge = await createChallenge(filePath, meta); challengeBlock.challenges = [...challengeBlock.challenges, challenge]; } -async function parseTranslation(engPath, transPath, dict) { +async function parseTranslation(engPath, transPath, dict, lang) { const engChal = await parseMarkdown(engPath); const translatedChal = await parseMarkdown(transPath); const engWithTranslatedComments = translateCommentsInChallenge( engChal, - getChallengeLang(transPath), + lang, dict ); + return mergeChallenges(engWithTranslatedComments, translatedChal); } -exports.parseTranslation = parseTranslation; - -async function createChallenge(fullPath, maybeMeta) { - let meta; - if (maybeMeta) { - meta = maybeMeta; - } else { - const metaPath = path.resolve( - metaDir, - `./${getBlockNameFromFullPath(fullPath)}/meta.json` +function createChallengeCreator(basePath, lang) { + const hasEnglishSource = hasEnglishSourceCreator(basePath); + return async function createChallenge(filePath, maybeMeta) { + let meta; + if (maybeMeta) { + meta = maybeMeta; + } else { + const metaPath = path.resolve( + metaDir, + `./${getBlockNameFromPath(filePath)}/meta.json` + ); + meta = require(metaPath); + } + const { name: superBlock } = superBlockInfoFromPath(filePath); + if (!supportedLangs.includes(lang)) + throw Error(`${lang} is not a accepted language. + Trying to parse ${filePath}`); + if (lang !== 'english' && !(await hasEnglishSource(filePath))) + throw Error(`Missing English challenge for +${filePath} +It should be in +${path.resolve(basePath, 'english', filePath)} +`); + // assumes superblock names are unique + // while the auditing is ongoing, we default to English for un-audited certs + // once that's complete, we can revert to using isEnglishChallenge(fullPath) + const useEnglish = lang === 'english' || !isAuditedCert(lang, superBlock); + const challenge = await (useEnglish + ? parseMarkdown(path.resolve(basePath, 'english', filePath)) + : parseTranslation( + path.resolve(basePath, 'english', filePath), + path.resolve(basePath, lang, filePath), + COMMENT_TRANSLATIONS, + lang + )); + const challengeOrder = findIndex( + meta.challengeOrder, + ([id]) => id === challenge.id ); - meta = require(metaPath); - } - const { name: superBlock } = superBlockInfoFromFullPath(fullPath); - const lang = getChallengeLang(fullPath); - if (!supportedLangs.includes(lang)) - throw Error(`${lang} is not a accepted language. -Trying to parse ${fullPath}`); - // assumes superblock names are unique - // while the auditing is ongoing, we default to English for un-audited certs - // once that's complete, we can revert to using isEnglishChallenge(fullPath) - const isEnglish = - isEnglishChallenge(fullPath) || !isAuditedCert(lang, superBlock); - if (isEnglish) fullPath = getEnglishPath(fullPath); - const challenge = await (isEnglish - ? parseMarkdown(fullPath) - : parseTranslation( - getEnglishPath(fullPath), - fullPath, - COMMENT_TRANSLATIONS - )); - const challengeOrder = findIndex( - meta.challengeOrder, - ([id]) => id === challenge.id - ); - const { - name: blockName, - order, - superOrder, - isPrivate, - required = [], - template, - time - } = meta; - challenge.block = blockName; - challenge.dashedName = dasherize(challenge.title); - challenge.order = order; - challenge.superOrder = superOrder; - challenge.superBlock = superBlock; - challenge.challengeOrder = challengeOrder; - challenge.isPrivate = challenge.isPrivate || isPrivate; - challenge.required = required.concat(challenge.required || []); - challenge.template = template; - challenge.time = time; + const { + name: blockName, + order, + superOrder, + isPrivate, + required = [], + template, + time + } = meta; + challenge.block = blockName; + challenge.dashedName = dasherize(challenge.title); + challenge.order = order; + challenge.superOrder = superOrder; + challenge.superBlock = superBlock; + challenge.challengeOrder = challengeOrder; + challenge.isPrivate = challenge.isPrivate || isPrivate; + challenge.required = required.concat(challenge.required || []); + challenge.template = template; + challenge.time = time; - return prepareChallenge(challenge); + return prepareChallenge(challenge); + }; } // TODO: tests and more descriptive name. @@ -203,8 +213,6 @@ function prepareChallenge(challenge) { challenge.name = nameify(challenge.title); if (challenge.files) { challenge.files = filesToObject(challenge.files); - // TODO: This should be something that can be folded into the above reduce - // EDIT: maybe not, now that we're doing the same for solutionFiles. challenge.files = Object.keys(challenge.files) .filter(key => challenge.files[key]) .map(key => challenge.files[key]) @@ -228,51 +236,23 @@ function prepareChallenge(challenge) { return challenge; } -exports.createChallenge = createChallenge; - -function getEnglishPath(fullPath) { - const posix = path - .normalize(fullPath) - .split(path.sep) - .join(path.posix.sep); - const match = posix.match(/(.*curriculum\/challenges\/)([^/]*)(.*)(\2)(.*)/); - const lang = getChallengeLang(fullPath); - if (!supportedLangs.includes(lang)) - throw Error(`${lang} is not a accepted language. -Trying to parse ${fullPath}`); - if (match) { - return path.join(match[1], 'english', match[3] + 'english' + match[5]); - } else { - throw Error(`Malformed challenge path, ${fullPath} unable to parse.`); - } +function hasEnglishSourceCreator(basePath) { + const englishRoot = path.resolve(__dirname, basePath, 'english'); + return async function(translationPath) { + return await access( + path.join(englishRoot, translationPath), + fs.constants.F_OK + ) + .then(() => true) + .catch(() => false); + }; } -function getChallengeLang(fullPath) { - const match = fullPath.match(/\.(\w+)\.md$/); - if (!match || match.length < 2) - throw Error(`Missing language extension for -${fullPath}`); - return fullPath.match(/\.(\w+)\.md$/)[1]; -} - -function isEnglishChallenge(fullPath) { - return getChallengeLang(fullPath) === 'english'; -} - -exports.getChallengeLang = getChallengeLang; -exports.getEnglishPath = getEnglishPath; -exports.isEnglishChallenge = isEnglishChallenge; - function superBlockInfoFromPath(filePath) { const [maybeSuper] = filePath.split(path.sep); return superBlockInfo(maybeSuper); } -function superBlockInfoFromFullPath(fullFilePath) { - const [, , maybeSuper] = fullFilePath.split(path.sep).reverse(); - return superBlockInfo(maybeSuper); -} - function superBlockInfo(fileName) { const [maybeOrder, ...superBlock] = fileName.split('-'); let order = parseInt(maybeOrder, 10); @@ -291,11 +271,10 @@ function getBlockNameFromPath(filePath) { return block; } -function getBlockNameFromFullPath(fullFilePath) { - const [, block] = fullFilePath.split(path.sep).reverse(); - return block; -} - function arrToString(arr) { return Array.isArray(arr) ? arr.join('\n') : toString(arr); } + +exports.hasEnglishSourceCreator = hasEnglishSourceCreator; +exports.parseTranslation = parseTranslation; +exports.createChallengeCreator = createChallengeCreator; diff --git a/curriculum/getChallenges.test.js b/curriculum/getChallenges.test.js index 44ce58b0a8..81dfda55f3 100644 --- a/curriculum/getChallenges.test.js +++ b/curriculum/getChallenges.test.js @@ -1,68 +1,57 @@ -/* global expect */ +/* global expect beforeAll */ const { - createChallenge, - getChallengeLang, - getEnglishPath, - isEnglishChallenge + challengesDir, + createChallengeCreator, + hasEnglishSourceCreator } = require('./getChallenges'); /* eslint-disable max-len */ -const INVALID_PATH = 'not/challenge/path'; -const ENGLISH_PATH = - 'curriculum/challenges/english/01-responsive-web-design/applied-accessibility/add-a-text-alternative-to-images-for-visually-impaired-accessibility.english.md'; -const CHINESE_PATH = - 'curriculum/challenges/chinese/01-responsive-web-design/applied-accessibility/add-a-text-alternative-to-images-for-visually-impaired-accessibility.chinese.md'; -const NOT_LANGUAGE_PATH = - 'curriculum/challenges/chinese/01-responsive-web-design/applied-accessibility/add-a-text-alternative-to-images-for-visually-impaired-accessibility.notlang.md'; -const MISSING_LANGUAGE_PATH = - 'curriculum/challenges/chinese/01-responsive-web-design/applied-accessibility/add-a-text-alternative-to-images-for-visually-impaired-english.md'; +const REAL_PATH = + '01-responsive-web-design/applied-accessibility/add-a-text-alternative-to-images-for-visually-impaired-accessibility.english.md'; +const REAL_MISSING_PATH = + '01-responsive-web-design/applied-accessibility/add-a-text-alternative-to-images-for-visually-impaired.md'; +const EXISTING_CHALLENGE_PATH = 'challenge.md'; +const MISSING_CHALLENGE_PATH = 'no/challenge.md'; /* eslint-enable max-len */ +let hasEnglishSource; +let createChallenge; +const basePath = '__fixtures__'; + describe('create non-English challenge', () => { describe('createChallenge', () => { - it('throws if the filename includes an invalid language', async () => { - await expect(createChallenge(NOT_LANGUAGE_PATH)).rejects.toThrow( + it('throws if lang is an invalid language', async () => { + createChallenge = createChallengeCreator(basePath, 'notlang'); + await expect(createChallenge(REAL_PATH)).rejects.toThrow( 'notlang is not a accepted language' ); }); - it('throws an error if the filename is missing a language', async () => { - await expect(createChallenge(MISSING_LANGUAGE_PATH)).rejects.toThrow( - `Missing language extension for -${MISSING_LANGUAGE_PATH}` + it('throws an error if the source challenge is missing', async () => { + createChallenge = createChallengeCreator(challengesDir, 'chinese'); + await expect(createChallenge(REAL_MISSING_PATH)).rejects.toThrow( + `Missing English challenge for +${REAL_MISSING_PATH} +It should be in +` ); }); }); - describe('getEnglishPath', () => { - it('returns the full path of the English version of the challenge', () => { - expect(getEnglishPath(CHINESE_PATH)).toBe(ENGLISH_PATH); + describe('hasEnglishSource', () => { + beforeAll(() => { + hasEnglishSource = hasEnglishSourceCreator(basePath); }); - it('throws an error if the path has the wrong directory structure', () => { - expect(() => getEnglishPath(INVALID_PATH)).toThrow(); + it('returns a boolean', async () => { + const sourceExists = await hasEnglishSource(EXISTING_CHALLENGE_PATH); + expect(typeof sourceExists).toBe('boolean'); }); - it('throws an error if the filename includes an invalid language', () => { - expect(() => getEnglishPath(NOT_LANGUAGE_PATH)).toThrow(); + it('returns true if the English challenge exists', async () => { + const sourceExists = await hasEnglishSource(EXISTING_CHALLENGE_PATH); + expect(sourceExists).toBe(true); }); - it('throws an error if the filename is missing a language', () => { - expect(() => getEnglishPath(MISSING_LANGUAGE_PATH)).toThrow(); - }); - }); - - describe('getChallengeLang', () => { - it("returns 'english' if the challenge is English", () => { - expect(getChallengeLang(ENGLISH_PATH)).toBe('english'); - }); - it("returns 'chinese' if the challenge is Chinese", () => { - expect(getChallengeLang(CHINESE_PATH)).toBe('chinese'); - }); - }); - - describe('isEnglishChallenge', () => { - it('returns true if the challenge is English', () => { - expect(isEnglishChallenge(ENGLISH_PATH)).toBe(true); - }); - it('returns false if the challenge is not English', () => { - expect(isEnglishChallenge(CHINESE_PATH)).toBe(false); + it('returns false if the English challenge is missing', async () => { + const sourceExists = await hasEnglishSource(MISSING_CHALLENGE_PATH); + expect(sourceExists).toBe(false); }); }); });