Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🏗 Execute integration tests on both prod and canary config, with experiments derandomized #32808

Merged
merged 6 commits into from
Feb 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 24 additions & 2 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 >>
danielrozenberg marked this conversation as resolved.
Show resolved Hide resolved
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 >>
danielrozenberg marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down Expand Up @@ -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'
Expand Down
14 changes: 14 additions & 0 deletions build-system/pr-check/module-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}

Expand All @@ -42,6 +55,7 @@ function prBuildWorkflow() {
downloadNomoduleOutput();
downloadModuleOutput();
timedExecOrDie('gulp update-packages');
prependConfig();
timedExecOrDie('gulp integration --nobuild --compiled --headless --esm');
} else {
printSkipMessage(
Expand Down
12 changes: 12 additions & 0 deletions build-system/pr-check/nomodule-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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(
Expand Down
39 changes: 28 additions & 11 deletions build-system/tasks/prepend-global/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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);

Expand Down Expand Up @@ -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);
Expand All @@ -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};
Expand Down Expand Up @@ -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}
Expand All @@ -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;
Expand Down Expand Up @@ -275,7 +292,8 @@ async function prependGlobal() {
argv.local_dev,
argv.local_branch,
argv.branch,
argv.fortesting
argv.fortesting,
argv.derandomize
);
}

Expand Down Expand Up @@ -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.',
};
15 changes: 12 additions & 3 deletions build-system/tasks/report-test-status.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,10 @@ const TEST_TYPE_SUBTYPES = isGithubActionsBuild()
'integration',
[
'unminified',
'nomodule',
'module',
'nomodule-prod',
'nomodule-canary',
'module-prod',
'module-canary',
'experimentA',
'experimentB',
'experimentC',
Expand Down Expand Up @@ -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) {
Expand Down