🏗 Execute integration tests on both prod and canary config, with expe…

…riments derandomized (#32808)
ampproject · Feb 25, 2021 · 01f115a · 01f115a
1 parent db55230
commit 01f115a
Show file tree

Hide file tree

Showing 5 changed files with 90 additions and 16 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -180,27 +180,41 @@ jobs:
   'Nomodule Tests':
     executor:
       name: amphtml-large-executor
+    parameters:
+      config:
+        description: 'Which config file to use'
+        type: enum
+        enum: ['prod', 'canary']
+    environment:
+      config: << parameters.config >>
     steps:
       - setup_vm
       - install_chrome
       - restore_karma_cache:
           cache_name: nomodule
       - run:
-          name: 'Nomodule Tests'
+          name: 'Nomodule Tests (using << parameters.config >>-config.json)'
           command: node build-system/pr-check/nomodule-tests.js
       - save_karma_cache:
           cache_name: nomodule
       - fail_fast
   'Module Tests':
     executor:
       name: amphtml-large-executor
+    parameters:
+      config:
+        description: 'Which config file to use'
+        type: enum
+        enum: ['prod', 'canary']
+    environment:
+      config: << parameters.config >>
     steps:
       - setup_vm
       - install_chrome
       - restore_karma_cache:
           cache_name: module
       - run:
-          name: 'Module Tests'
+          name: 'Module Tests (using << parameters.config >>-config.json)'
           command: node build-system/pr-check/module-tests.js
       - save_karma_cache:
           cache_name: module
@@ -324,10 +338,18 @@ workflows:
           requires:
             - 'Unminified Build'
       - 'Nomodule Tests':
+          name: 'Nomodule Tests (<< matrix.config >>)'
+          matrix:
+            parameters:
+              config: ['prod', 'canary']
           <<: *push_and_pr_builds
           requires:
             - 'Nomodule Build'
       - 'Module Tests':
+          name: 'Module Tests (<< matrix.config >>)'
+          matrix:
+            parameters:
+              config: ['prod', 'canary']
           <<: *push_and_pr_builds
           requires:
             - 'Nomodule Build'

diff --git a/build-system/pr-check/module-tests.js b/build-system/pr-check/module-tests.js
@@ -26,14 +26,27 @@ const {
   timedExecOrDie,
 } = require('./utils');
 const {buildTargetsInclude, Targets} = require('./build-targets');
+const {MINIFIED_TARGETS} = require('../tasks/helpers');
 const {runCiJob} = require('./ci-job');
 
 const jobName = 'module-tests.js';
 
+function prependConfig() {
+  // TODO(@ampproject/wg-infra): change prepend-global to take multiple target files instead of looping here.
+  for (const target of MINIFIED_TARGETS) {
+    for (const ext of ['js', 'mjs']) {
+      timedExecOrDie(
+        `gulp prepend-global --${process.env.config} --local_dev --fortesting --derandomize --target=dist/${target}.${ext}`
+      );
+    }
+  }
+}
+
 function pushBuildWorkflow() {
   downloadNomoduleOutput();
   downloadModuleOutput();
   timedExecOrDie('gulp update-packages');
+  prependConfig();
   timedExecOrDie('gulp integration --nobuild --compiled --headless --esm');
 }
 
@@ -42,6 +55,7 @@ function prBuildWorkflow() {
     downloadNomoduleOutput();
     downloadModuleOutput();
     timedExecOrDie('gulp update-packages');
+    prependConfig();
     timedExecOrDie('gulp integration --nobuild --compiled --headless --esm');
   } else {
     printSkipMessage(

diff --git a/build-system/pr-check/nomodule-tests.js b/build-system/pr-check/nomodule-tests.js
@@ -26,13 +26,24 @@ const {
   timedExecOrThrow,
 } = require('./utils');
 const {buildTargetsInclude, Targets} = require('./build-targets');
+const {MINIFIED_TARGETS} = require('../tasks/helpers');
 const {runCiJob} = require('./ci-job');
 
 const jobName = 'nomodule-tests.js';
 
+function prependConfig() {
+  // TODO(@ampproject/wg-infra): change prepend-global to take multiple target files instead of looping here.
+  for (const target of MINIFIED_TARGETS) {
+    timedExecOrDie(
+      `gulp prepend-global --${process.env.config} --local_dev --fortesting --derandomize --target=dist/${target}.js`
+    );
+  }
+}
+
 function pushBuildWorkflow() {
   downloadNomoduleOutput();
   timedExecOrDie('gulp update-packages');
+  prependConfig();
   try {
     timedExecOrThrow(
       'gulp integration --nobuild --headless --compiled --report',
@@ -51,6 +62,7 @@ function prBuildWorkflow() {
   if (buildTargetsInclude(Targets.RUNTIME, Targets.INTEGRATION_TEST)) {
     downloadNomoduleOutput();
     timedExecOrDie('gulp update-packages');
+    prependConfig();
     timedExecOrDie('gulp integration --nobuild --compiled --headless');
   } else {
     printSkipMessage(

diff --git a/build-system/tasks/prepend-global/index.js b/build-system/tasks/prepend-global/index.js
@@ -129,6 +129,7 @@ function valueOrDefault(value, defaultValue) {
  * @param {boolean=} opt_localBranch Whether to use the local branch version
  * @param {string=} opt_branch If not the local branch, which branch to use
  * @param {boolean=} opt_fortesting Whether to force getMode().test to be true
+ * @param {boolean=} opt_derandomize Whether to remove experiment randomization
  * @return {!Promise}
  */
 async function applyConfig(
@@ -138,7 +139,8 @@ async function applyConfig(
   opt_localDev,
   opt_localBranch,
   opt_branch,
-  opt_fortesting
+  opt_fortesting,
+  opt_derandomize
 ) {
   await checkoutBranchConfigs(filename, opt_localBranch, opt_branch);
 
@@ -173,6 +175,9 @@ async function applyConfig(
   if (opt_fortesting) {
     configJson = {test: true, ...configJson};
   }
+  if (opt_derandomize) {
+    configJson = derandomize(target, configJson);
+  }
   configString = JSON.stringify(configJson);
   const fileString = await prependConfig(configString, targetString);
   sanityCheck(fileString);
@@ -182,14 +187,15 @@ async function applyConfig(
     cyan(config) +
     (opt_localDev ? ', ' + cyan('localDev') : '') +
     (opt_fortesting ? ', ' + cyan('test') : '') +
+    (opt_derandomize ? ', ' + cyan('derandomized') : '') +
     ')';
   log('Applied AMP config', details, 'to', cyan(path.basename(target)));
 }
 
 /**
  * @param {string} target File containing the AMP runtime (amp.js or v0.js)
- * @param {string} configJson The json object in which to enable local dev
- * @return {string}
+ * @param {!JSON} configJson The json object in which to enable local dev
+ * @return {!JSON}
  */
 function enableLocalDev(target, configJson) {
   let LOCAL_DEV_AMP_CONFIG = {localDev: true};
@@ -217,6 +223,21 @@ function enableLocalDev(target, configJson) {
   return Object.assign(LOCAL_DEV_AMP_CONFIG, configJson);
 }
 
+/**
+ * @param {string} target File containing the AMP runtime (amp.js or v0.js)
+ * @param {!JSON} configJson The json object in which to enable local dev
+ * @return {!JSON}
+ */
+function derandomize(target, configJson) {
+  for (const [key, value] of Object.entries(configJson)) {
+    if (typeof value == 'number') {
+      configJson[key] = Math.round(value);
+    }
+  }
+  log('Derandomized experiements in', cyan(target));
+  return configJson;
+}
+
 /**
  * @param {string} target Target file from which to remove the AMP config
  * @return {!Promise}
@@ -243,10 +264,6 @@ async function prependGlobal() {
     return;
   }
 
-  if (argv.remove) {
-    return removeConfig(target);
-  }
-
   if (!(argv.prod || argv.canary)) {
     log(red('One of --prod or --canary should be provided.'));
     return;
@@ -275,7 +292,8 @@ async function prependGlobal() {
     argv.local_dev,
     argv.local_branch,
     argv.branch,
-    argv.fortesting
+    argv.fortesting,
+    argv.derandomize
   );
 }
 
@@ -307,7 +325,6 @@ prependGlobal.flags = {
   'local_branch':
     "  Don't switch branches and use the config from the local branch.",
   'fortesting': '  Force the config to return true for getMode().test',
-  'remove':
-    '  Removes previously prepended json config from the target ' +
-    'file (if present).',
+  'derandomize':
+    '  Rounds all experiment percentages to 0 or 1, whichever is closest.',
 };
diff --git a/build-system/tasks/report-test-status.js b/build-system/tasks/report-test-status.js
@@ -46,8 +46,10 @@ const TEST_TYPE_SUBTYPES = isGithubActionsBuild()
         'integration',
         [
           'unminified',
-          'nomodule',
-          'module',
+          'nomodule-prod',
+          'nomodule-canary',
+          'module-prod',
+          'module-canary',
           'experimentA',
           'experimentB',
           'experimentC',
@@ -99,7 +101,14 @@ function inferTestType() {
     ? 'nomodule'
     : 'unminified';
 
-  return `${type}/${subtype}`;
+  return `${type}/${subtype}${maybeAddConfigSubtype()}`;
+}
+
+function maybeAddConfigSubtype() {
+  if (isCircleciBuild() && process.env.config) {
+    return `-${process.env.config}`;
+  }
+  return '';
 }
 
 async function postReport(type, action) {