fix(tests): add validation of comment translations (#41537)

curriculum/test/test-challenges.js

@@ -24,7 +24,7 @@ const {
 
 const { assert, AssertionError } = require('chai');
 const Mocha = require('mocha');
-const { flatten, isEmpty, cloneDeep } = require('lodash');
+const { flatten, isEmpty, cloneDeep, isEqual } = require('lodash');
 const { getLines } = require('../../utils/get-lines');
 
 const jsdom = require('jsdom');
@@ -62,12 +62,14 @@ const TRANSLATABLE_COMMENTS = getTranslatableComments(
 // the config files are created during the build, but not before linting
 // eslint-disable-next-line import/no-unresolved
 const testEvaluator = require('../../config/client/test-evaluator').filename;
+const { inspect } = require('util');
 
 const commentExtractors = {
   html: require('./utils/extract-html-comments'),
   js: require('./utils/extract-js-comments'),
   jsx: require('./utils/extract-jsx-comments'),
-  css: require('./utils/extract-css-comments')
+  css: require('./utils/extract-css-comments'),
+  scriptJs: require('./utils/extract-script-js-comments')
 };
 
 // rethrow unhandled rejections to make sure the tests exit with -1
@@ -329,7 +331,7 @@ function populateTestsForLang({ lang, challenges, meta }) {
 
               // We get all the actual comments using the appropriate parsers
               if (file.ext === 'html') {
-                const commentTypes = ['css', 'html'];
+                const commentTypes = ['css', 'html', 'scriptJs'];
                 for (let type of commentTypes) {
                   const newComments = commentExtractors[type](file.contents);
                   for (const [key, value] of Object.entries(newComments)) {
@@ -342,17 +344,22 @@ function populateTestsForLang({ lang, challenges, meta }) {
                 comments = commentExtractors[file.ext](file.contents);
               }
 
-              // Then we compare the number of times a given comment appears
-              // (count) with the number of times the text within it appears
-              // (commentTextCount)
-              for (const [comment, count] of Object.entries(comments)) {
-                const commentTextCount =
-                  file.contents.split(comment).length - 1;
-                if (commentTextCount !== count)
-                  throw Error(
-                    `Translated comment text, ${comment}, should only appear inside comments`
-                  );
-              }
+              // Then we compare the number of times each comment appears in the
+              // translated text (commentMap) with the number of replacements
+              // made during translation (challenge.__commentCounts). If they
+              // differ, the translation must have gone wrong
+
+              const commentMap = new Map(Object.entries(comments));
+
+              if (isEmpty(challenge.__commentCounts) && isEmpty(commentMap))
+                return;
+
+              if (!isEqual(commentMap, challenge.__commentCounts))
+                throw Error(`Mismatch in ${challenge.title}. Replaced comments:
+${inspect(challenge.__commentCounts)}
+Comments in translated text:
+${inspect(commentMap)}
+`);
             });
           });
 

curriculum/test/utils/extract-css-comments.test.js

@@ -19,6 +19,39 @@ Some text
 </body>
 `;
 
+const outsideDeclarations = `
+<style>
+.body {
+  color: red;
+  /* comment 1 */
+}
+
+
+/* comment 1 */
+/* comment 2 */
+}
+
+</style>
+`;
+
+const mediaQuery = `
+<style>
+.body {
+  color: red;
+  /* comment 1 */
+}
+
+
+@media (max-width: 350px) {
+  :root {
+    /* comment 2 */
+
+    /* comment 2 */
+  }
+}
+</style>
+`;
+
 // NOTE: this is a bit finicky.  If the css is, say, missing a semi-colon,
 // nearby comments may be missed.
 describe('extractCSSComments', () => {
@@ -29,4 +62,18 @@ describe('extractCSSComments', () => {
     };
     expect(extractCSSComments(someHTMLWithCSS)).toEqual(commentCounts);
   });
+  it('should catch comments outside of declarations', () => {
+    const commentCounts = {
+      'comment 1': 2,
+      'comment 2': 1
+    };
+    expect(extractCSSComments(outsideDeclarations)).toEqual(commentCounts);
+  });
+  it('should catch comments inside of media queries', () => {
+    const commentCounts = {
+      'comment 1': 1,
+      'comment 2': 2
+    };
+    expect(extractCSSComments(mediaQuery)).toEqual(commentCounts);
+  });
 });

curriculum/test/utils/extract-script-js-comments.js

@@ -0,0 +1,12 @@
+var rehype = require('rehype');
+var vfile = require('vfile');
+
+var getComments = require('./plugins/get-script-js-comments');
+
+const processor = rehype().use(getComments);
+
+function extractComments(html) {
+  return processor.processSync(vfile(html)).data;
+}
+
+module.exports = extractComments;

curriculum/test/utils/extract-script-js-comments.test.js

@@ -0,0 +1,77 @@
+/* global expect */
+const extractScriptJSComments = require('./extract-script-js-comments');
+
+const inlineComments = `<body>
+Some text
+<script>
+
+// comment 1
+
+var x = 'y';
+
+// comment 2
+
+// comment 1
+
+</script>
+</body>
+`;
+
+const multilineComments = `<body>
+Some text
+<script>
+
+/*
+comment 1
+*/
+var x = 'y';
+
+/* comment 2 */
+
+/* comment 1 */
+
+</script>
+</body>
+`;
+
+const outsideScript = `<body>
+Some text
+<script>
+
+// comment 1
+
+var x = 'y';
+
+// comment 2
+
+// comment 1
+
+</script>
+
+// comment 2
+</body>
+`;
+
+describe('extractScriptJSComments', () => {
+  it('should catch inline comments', () => {
+    const commentCounts = {
+      'comment 1': 2,
+      'comment 2': 1
+    };
+    expect(extractScriptJSComments(inlineComments)).toEqual(commentCounts);
+  });
+  it('should catch multiline comments', () => {
+    const commentCounts = {
+      'comment 1': 2,
+      'comment 2': 1
+    };
+    expect(extractScriptJSComments(multilineComments)).toEqual(commentCounts);
+  });
+  it('should ignore comments outside script tags', () => {
+    const commentCounts = {
+      'comment 1': 2,
+      'comment 2': 1
+    };
+    expect(extractScriptJSComments(outsideScript)).toEqual(commentCounts);
+  });
+});

curriculum/test/utils/plugins/get-css-comments.js

@@ -1,9 +1,28 @@
 const { isEmpty } = require('lodash');
 const visit = require('unist-util-visit');
 var css = require('css');
-var visitCss = require('rework-visit');
 const { commentToData } = require('../comment-to-data');
 
+function visitComments(node, cb) {
+  node.rules.forEach(rule => {
+    if (rule.type === 'rule') {
+      visitDeclarations(rule.declarations, cb);
+    } else if (rule.type === 'comment') {
+      cb(rule.comment);
+    } else if (rule.type === 'media') {
+      visitComments(rule, cb);
+    }
+  });
+}
+
+function visitDeclarations(declarations, cb) {
+  declarations.forEach(dec => {
+    if (dec.type === 'comment') {
+      cb(dec.comment);
+    }
+  });
+}
+
 function plugin() {
   return transformer;
 
@@ -16,12 +35,9 @@ function plugin() {
     }
     function cssVisitor(node) {
       const ast = css.parse(node.value);
-      visitCss(ast.stylesheet, dec => {
-        let comments = dec
-          .filter(({ type }) => type === 'comment')
-          .map(({ comment }) => comment.trim());
-        comments.forEach(comment => commentToData(file, comment));
-      });
+      visitComments(ast.stylesheet, comment =>
+        commentToData(file, comment.trim())
+      );
     }
   }
 }

curriculum/test/utils/plugins/get-script-js-comments.js

@@ -0,0 +1,29 @@
+const { isEmpty } = require('lodash');
+const visit = require('unist-util-visit');
+const acorn = require('acorn');
+const { commentToData } = require('../comment-to-data');
+
+const parser = acorn.Parser;
+
+function plugin() {
+  return transformer;
+
+  function transformer(tree, file) {
+    if (isEmpty(file.data)) file.data = {};
+    visit(tree, { type: 'element', tagName: 'script' }, scriptVisitor);
+
+    function scriptVisitor(node) {
+      visit(node, 'text', jsVisitor);
+    }
+    function jsVisitor(node) {
+      let comments = [];
+      parser.parse(node.value, { onComment: comments, ecmaVersion: 2020 });
+
+      comments
+        .map(({ value }) => value.trim())
+        .forEach(comment => commentToData(file, comment));
+    }
+  }
+}
+
+module.exports = plugin;