🏗 Add amp validate-html-fixtures to AMP's PR check workflow

amp.js

@@ -80,6 +80,7 @@ createTask('storybook');
 createTask('sweep-experiments', 'sweepExperiments');
 createTask('test-report-upload', 'testReportUpload');
 createTask('unit');
+createTask('validate-html-fixtures', 'validateHtmlFixtures');
 createTask('validator');
 createTask('validator-cpp', 'validatorCpp', 'validator');
 createTask('validator-webui', 'validatorWebui', 'validator');

build-system/common/utils.js

@@ -68,13 +68,14 @@ function getValidExperiments() {
 
 /**
  * Gets the list of files changed on the current branch that match the given
- * array of glob patterns
+ * array of glob patterns using the given options.
  *
  * @param {!Array<string>} globs
+ * @param {!Object} options
  * @return {!Array<string>}
  */
-function getFilesChanged(globs) {
-  const allFiles = globby.sync(globs, {dot: true});
+function getFilesChanged(globs, options) {
+  const allFiles = globby.sync(globs, options);
   return gitDiffNameOnlyMain().filter((changedFile) => {
     return fs.existsSync(changedFile) && allFiles.includes(changedFile);
   });
@@ -132,7 +133,7 @@ function getFilesToCheck(globs, options = {}, ignoreFile = undefined) {
     return logFiles(ignored.filter(getFilesFromArgv()));
   }
   if (argv.local_changes) {
-    const filesChanged = ignored.filter(getFilesChanged(globs));
+    const filesChanged = ignored.filter(getFilesChanged(globs, options));
     if (filesChanged.length == 0) {
       log(green('INFO: ') + 'No files to check in this PR');
       return [];

build-system/pr-check/build-targets.js

@@ -38,9 +38,10 @@ let buildTargets;
  * Used to prevent the repeated expansion of globs during PR jobs.
  */
 let lintFiles;
+let htmlFixtureFiles;
+let invalidWhitespaceFiles;
 let presubmitFiles;
 let prettifyFiles;
-let invalidWhitespaceFiles;
 
 /***
  * All of AMP's build targets that can be tested during CI.
@@ -54,6 +55,7 @@ const Targets = {
   DEV_DASHBOARD: 'DEV_DASHBOARD',
   DOCS: 'DOCS',
   E2E_TEST: 'E2E_TEST',
+  HTML_FIXTURES: 'HTML_FIXTURES',
   INTEGRATION_TEST: 'INTEGRATION_TEST',
   INVALID_WHITESPACES: 'INVALID_WHITESPACES',
   LINT: 'LINT',
@@ -189,6 +191,13 @@ const targetMatchers = {
       })
     );
   },
+  [Targets.HTML_FIXTURES]: (file) => {
+    return (
+      htmlFixtureFiles.includes(file) ||
+      file == 'build-system/tasks/validate-html-fixtures.js' ||
+      file.startsWith('build-system/test-configs')
+    );
+  },
   [Targets.INTEGRATION_TEST]: (file) => {
     if (isOwnersFile(file)) {
       return false;
@@ -202,6 +211,13 @@ const targetMatchers = {
       })
     );
   },
+  [Targets.INVALID_WHITESPACES]: (file) => {
+    return (
+      invalidWhitespaceFiles.includes(file) ||
+      file == 'build-system/tasks/check-invalid-whitespaces.js' ||
+      file.startsWith('build-system/test-configs')
+    );
+  },
   [Targets.LINT]: (file) => {
     if (isOwnersFile(file)) {
       return false;
@@ -259,13 +275,6 @@ const targetMatchers = {
       file.startsWith('build-system/server/')
     );
   },
-  [Targets.INVALID_WHITESPACES]: (file) => {
-    return (
-      invalidWhitespaceFiles.includes(file) ||
-      file == 'build-system/tasks/check-invalid-whitespaces.js' ||
-      file.startsWith('build-system/test-configs')
-    );
-  },
   [Targets.UNIT_TEST]: (file) => {
     if (isOwnersFile(file)) {
       return false;
@@ -321,9 +330,10 @@ function determineBuildTargets() {
   }
   buildTargets = new Set();
   lintFiles = globby.sync(config.lintGlobs);
+  htmlFixtureFiles = globby.sync(config.htmlFixtureGlobs);
+  invalidWhitespaceFiles = globby.sync(config.invalidWhitespaceGlobs);
   presubmitFiles = globby.sync(config.presubmitGlobs);
   prettifyFiles = globby.sync(config.prettifyGlobs);
-  invalidWhitespaceFiles = globby.sync(config.invalidWhitespaceGlobs);
   const filesChanged = gitDiffNameOnlyMain();
   for (const file of filesChanged) {
     let isRuntimeFile = true;

build-system/pr-check/checks.js

@@ -29,6 +29,7 @@ const jobName = 'checks.js';
 function pushBuildWorkflow() {
   timedExecOrDie('amp presubmit');
   timedExecOrDie('amp check-invalid-whitespaces');
+  timedExecOrDie('amp validate-html-fixtures');
   timedExecOrDie('amp lint');
   timedExecOrDie('amp prettify');
   timedExecOrDie('amp ava');
@@ -62,6 +63,10 @@ async function prBuildWorkflow() {
     timedExecOrDie('amp check-invalid-whitespaces');
   }
 
+  if (buildTargetsInclude(Targets.HTML_FIXTURES)) {
+    timedExecOrDie('amp validate-html-fixtures');
+  }
+
   if (buildTargetsInclude(Targets.LINT)) {
     timedExecOrDie('amp lint');
   }

build-system/tasks/check-invalid-whitespaces.js

@@ -13,6 +13,8 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+'using strict';
+
 const {getFilesToCheck} = require('../common/utils');
 const {getStdout} = require('../common/process');
 const {green, red} = require('kleur/colors');

build-system/tasks/pr-check.js

@@ -59,6 +59,10 @@ async function prCheck() {
     runCheck('amp check-invalid-whitespaces --local_changes');
   }
 
+  if (buildTargetsInclude(Targets.HTML_FIXTURES)) {
+    runCheck('amp validate-html-fixtures --local_changes');
+  }
+
   if (buildTargetsInclude(Targets.LINT)) {
     runCheck('amp lint --local_changes');
   }

build-system/tasks/validate-html-fixtures.js

@@ -0,0 +1,111 @@
+/**
+ * Copyright 2021 The AMP HTML Authors. All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS-IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+'using strict';
+
+const {
+  log,
+  logLocalDev,
+  logWithoutTimestamp,
+  logWithoutTimestampLocalDev,
+} = require('../common/logging');
+const {cyan, green, red} = require('kleur/colors');
+const {getFilesToCheck} = require('../common/utils');
+const {getOutput} = require('../common/process');
+const {htmlFixtureGlobs} = require('../test-configs/config');
+const {JSDOM} = require('jsdom');
+
+/**
+ * Gets the AMP format type for the given HTML file by parsing its contents and
+ * examining the root element.
+ * @param {string} file
+ * @return {Promise<string>}
+ */
+async function getAmpFormat(file) {
+  const jsdom = await JSDOM.fromFile(file);
+  const {documentElement} = jsdom.window.document;
+  const isAds = documentElement.hasAttribute('amp4ads');
+  const isEmail = documentElement.hasAttribute('amp4email');
+  return isAds ? 'AMP4ADS' : isEmail ? 'AMP4EMAIL' : 'AMP';
+}
+
+/**
+ * Separates the list of files to check into groups based on their AMP formats.
+ * @param {Array<string>} filesToCheck
+ * @return {Promise<Object>}
+ */
+async function getFileGroups(filesToCheck) {
+  logLocalDev(green('Sorting HTML fixtures into format groups...'));
+  const fileGroups = {AMP4ADS: [], AMP4EMAIL: [], AMP: []};
+  await Promise.all(
+    filesToCheck.map(async (file) =>
+      fileGroups[await getAmpFormat(file)].push(file)
+    )
+  );
+  return fileGroups;
+}
+
+/**
+ * Runs amphtml-validator on the given list of files and prints results.
+ *
+ * @param {Array<string>} filesToCheck
+ * @return {Promise<void>}
+ */
+async function runCheck(filesToCheck) {
+  const fileGroups = await getFileGroups(filesToCheck);
+  const formats = Object.keys(fileGroups);
+  let foundValidationErrors = false;
+  for (const format of formats) {
+    if (fileGroups[format].length == 0) {
+      continue;
+    }
+    const files = fileGroups[format].join(' ');
+    logLocalDev(green('Validating'), cyan(format), green('fixtures...'));
+    const validateFixturesCmd = `FORCE_COLOR=1 npx amphtml-validator --html_format ${format} ${files}`;
+    const result = getOutput(validateFixturesCmd);
+    logWithoutTimestampLocalDev(result.stdout);
+    if (result.stderr) {
+      logWithoutTimestamp(result.stderr);
+      log(red('ERROR:'), 'Found errors in', cyan(format), 'fixtures.');
+      foundValidationErrors = true;
+    }
+  }
+  if (foundValidationErrors) {
+    throw new Error('Please address the errors listed above.');
+  }
+  log(green('SUCCESS:'), 'All HTML fixtures are valid.');
+}
+
+/**
+ * Makes sure that HTML fixtures used during tests contain valid AMPHTML.
+ */
+async function validateHtmlFixtures() {
+  const filesToCheck = getFilesToCheck(htmlFixtureGlobs, {}, '.gitignore');
+  if (filesToCheck.length == 0) {
+    return;
+  }
+  await runCheck(filesToCheck);
+}
+
+validateHtmlFixtures.description =
+  'Makes sure that HTML fixtures used during tests contain valid AMPHTML.';
+validateHtmlFixtures.flags = {
+  'files': 'Checks just the specified files',
+  'local_changes': 'Checks just the files changed in the local branch',
+};
+
+module.exports = {
+  validateHtmlFixtures,
+};