Skip to content

Commit

Permalink
🏗 report test status of experimentN as "skipped" when no such experim…
Browse files Browse the repository at this point in the history
…ent exists (#33243)
  • Loading branch information
danielrozenberg committed Mar 15, 2021
1 parent 7b25837 commit b9cdc05
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 26 deletions.
26 changes: 26 additions & 0 deletions build-system/common/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

const argv = require('minimist')(process.argv.slice(2));
const experimentsConfig = require('../global-configs/experiments-config.json');
const fs = require('fs-extra');
const globby = require('globby');
const path = require('path');
Expand Down Expand Up @@ -45,6 +46,29 @@ async function buildRuntime(opt_compiled = false) {
}
}

/**
* Extracts and validates the config for the given experiment.
* @param {string} experiment
* @return {Object|null}
*/
function getExperimentConfig(experiment) {
const config = experimentsConfig[experiment];
const valid =
config?.name &&
config?.define_experiment_constant &&
config?.expiration_date_utc &&
new Number(new Date(config.expiration_date_utc)) >= Date.now();
return valid ? config : null;
}

/**
* Returns the names of all valid experiments.
* @return {!Array<string>}
*/
function getValidExperiments() {
return Object.keys(experimentsConfig).filter(getExperimentConfig);
}

/**
* Gets the list of files changed on the current branch that match the given
* array of glob patterns
Expand Down Expand Up @@ -159,6 +183,8 @@ function installPackages(dir) {

module.exports = {
buildRuntime,
getExperimentConfig,
getValidExperiments,
getFilesChanged,
getFilesFromArgv,
getFilesToCheck,
Expand Down
2 changes: 1 addition & 1 deletion build-system/pr-check/experiment-build.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@
*/

const {
getExperimentConfig,
printSkipMessage,
timedExecOrDie,
uploadExperimentOutput,
} = require('./utils');
const {buildTargetsInclude, Targets} = require('./build-targets');
const {experiment} = require('minimist')(process.argv.slice(2));
const {getExperimentConfig} = require('../common/utils');
const {runCiJob} = require('./ci-job');

const jobName = `${experiment}-build.js`;
Expand Down
2 changes: 1 addition & 1 deletion build-system/pr-check/experiment-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@
*/

const {
getExperimentConfig,
downloadExperimentOutput,
printSkipMessage,
timedExecOrDie,
} = require('./utils');
const {buildTargetsInclude, Targets} = require('./build-targets');
const {experiment} = require('minimist')(process.argv.slice(2));
const {getExperimentConfig} = require('../common/utils');
const {runCiJob} = require('./ci-job');

const jobName = `${experiment}-tests.js`;
Expand Down
17 changes: 0 additions & 17 deletions build-system/pr-check/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
*/
'use strict';

const experimentsConfig = require('../global-configs/experiments-config.json');
const {
gitBranchCreationPoint,
gitBranchName,
Expand Down Expand Up @@ -338,28 +337,12 @@ async function processAndUploadNomoduleOutput() {
uploadNomoduleOutput();
}

/**
* Extracts and validates the config for the given experiment.
* @param {string} experiment
* @return {Object|null}
*/
function getExperimentConfig(experiment) {
const config = experimentsConfig[experiment];
const valid =
config?.name &&
config?.define_experiment_constant &&
config?.expiration_date_utc &&
new Date(config.expiration_date_utc) >= Date.now();
return valid ? config : null;
}

module.exports = {
abortTimedJob,
downloadExperimentOutput,
downloadUnminifiedOutput,
downloadNomoduleOutput,
downloadModuleOutput,
getExperimentConfig,
printChangeSummary,
printSkipMessage,
processAndUploadNomoduleOutput,
Expand Down
18 changes: 11 additions & 7 deletions build-system/tasks/report-test-status.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const {
const {ciJobUrl} = require('../common/ci');
const {cyan, yellow} = require('kleur/colors');
const {determineBuildTargets, Targets} = require('../pr-check/build-targets');
const {getValidExperiments} = require('../common/utils');
const {gitCommitHash} = require('../common/git');
const {log} = require('../common/logging');

Expand All @@ -50,13 +51,11 @@ const TEST_TYPE_SUBTYPES = isGithubActionsBuild()
'nomodule-canary',
'module-prod',
'module-canary',
'experimentA',
'experimentB',
'experimentC',
...getValidExperiments(),
],
],
['unit', ['unminified', 'local-changes']],
['e2e', ['nomodule', 'experimentA', 'experimentB', 'experimentC']],
['e2e', ['nomodule', ...getValidExperiments()]],
])
: new Map([]);
const TEST_TYPE_BUILD_TARGETS = new Map([
Expand All @@ -66,7 +65,7 @@ const TEST_TYPE_BUILD_TARGETS = new Map([
]);

/**
* @return {string|null}
* @return {string}
*/
function inferTestType() {
// Determine type (early exit if there's no match).
Expand All @@ -78,7 +77,7 @@ function inferTestType() {
? 'unit'
: null;
if (type == null) {
return null;
throw new Error('No valid test type was inferred');
}

// Determine subtype (more specific cases come first).
Expand Down Expand Up @@ -139,7 +138,7 @@ async function postReport(type, action) {
// Do not use `json: true` because the response is a string, not JSON.
});

log('Reported', cyan(`${type}/${action}`, 'to GitHub'));
log('Reported', cyan(`${type}/${action}`), 'to GitHub');
if (body.length > 0) {
log('Response was', cyan(body.substr(0, 100)));
}
Expand Down Expand Up @@ -192,6 +191,11 @@ async function reportAllExpectedTests() {
const buildTargets = determineBuildTargets();
for (const [type, subTypes] of TEST_TYPE_SUBTYPES) {
const testTypeBuildTargets = TEST_TYPE_BUILD_TARGETS.get(type);
if (testTypeBuildTargets === undefined) {
throw new Error(
`Undefined test type ${type} for build targets ${buildTargets}`
);
}
const action = testTypeBuildTargets.some((target) =>
buildTargets.has(target)
)
Expand Down

0 comments on commit b9cdc05

Please sign in to comment.