Skip to content

Commit

Permalink
support --bento_only option in unit tests (ampproject#37935)
Browse files Browse the repository at this point in the history
* create separate init-tests script for bento tests
* separates bento unit testing files from other unit files
* adds bento-specific unit testing utilities
* updates defineElement() fns to accept a window attribute (useful for testing)
  • Loading branch information
kvchari authored Mar 23, 2022
1 parent e207b99 commit 8614666
Show file tree
Hide file tree
Showing 18 changed files with 818 additions and 1,231 deletions.
4 changes: 2 additions & 2 deletions build-system/tasks/extension-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -804,8 +804,8 @@ function generateBentoEntryPointSource(name, toExport, outputFilename) {
import {BaseElement} from '../base-element';
import {defineBentoElement} from '${bentoCePath}';
function defineElement() {
defineBentoElement(__name__, BaseElement);
function defineElement(win) {
defineBentoElement(__name__, BaseElement, win);
}
${toExport ? 'export {defineElement};' : 'defineElement();'}
Expand Down
11 changes: 7 additions & 4 deletions build-system/tasks/runtime-test/helpers-unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,10 @@ function getJsFilesFor(cssFile, cssJsFileMap) {

/**
* Computes the list of unit tests to run under difference scenarios
* @param {{bentoOnly?: boolean}} [options]
* @return {Array<string>|void}
*/
function getUnitTestsToRun() {
function getUnitTestsToRun({bentoOnly = false} = {}) {
log(green('INFO:'), 'Determining which unit tests to run...');

if (isLargeRefactor()) {
Expand All @@ -153,7 +154,7 @@ function getUnitTestsToRun() {
return;
}

const tests = unitTestsToRun();
const tests = unitTestsToRun({bentoOnly});
if (tests.length == 0) {
log(
green('INFO:'),
Expand Down Expand Up @@ -181,15 +182,17 @@ function getUnitTestsToRun() {
* Extracts the list of unit tests to run based on the changes in the local
* branch. Return value is cached to optimize for multiple calls.
*
* @param {{bentoOnly?: boolean}} [options]
* @return {!Array<string>}
*/
function unitTestsToRun() {
function unitTestsToRun({bentoOnly = false} = {}) {
if (testsToRun) {
return testsToRun;
}
const cssJsFileMap = extractCssJsFileMap();
const filesChanged = gitDiffNameOnlyMain();
const {unitTestPaths} = testConfig;
const {bentoUnitTestPaths, unitTestPaths: nonBentoUnitTestPaths} = testConfig;
const unitTestPaths = bentoOnly ? bentoUnitTestPaths : nonBentoUnitTestPaths;
testsToRun = [];
let srcFiles = [];

Expand Down
18 changes: 12 additions & 6 deletions build-system/tasks/runtime-test/runtime-test-base.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@
const argv = require('minimist')(process.argv.slice(2));
const karmaConfig = require('../../test-configs/karma.conf');
const {
bentoUnitTestPaths,
commonIntegrationTestPaths,
commonUnitTestPaths,
getCommonUnitTestPaths,
integrationTestPaths,
karmaHtmlFixturesPath,
karmaJsPaths,
Expand Down Expand Up @@ -56,8 +57,9 @@ class RuntimeTestConfig {

/**
* @param {string} testType
* @param {{bentoOnly?: boolean}} [options]
*/
constructor(testType) {
constructor(testType, {bentoOnly = false} = {}) {
this.testType = testType;
/**
* TypeScript is used for typechecking here and is unable to infer the type
Expand All @@ -67,7 +69,7 @@ class RuntimeTestConfig {
Object.assign(this, karmaConfig);
this.updateBrowsers();
this.updateReporters();
this.updateFiles();
this.updateFiles({bentoOnly});
this.updatePreprocessors();
this.updateEsbuildConfig();
this.updateClient();
Expand Down Expand Up @@ -151,10 +153,12 @@ class RuntimeTestConfig {
/**
* Computes the set of files for Karma to load based on factors like test type,
* target browser, and flags.
* @param {{bentoOnly: boolean}} options
*/
updateFiles() {
updateFiles({bentoOnly}) {
switch (this.testType) {
case 'unit':
const commonUnitTestPaths = getCommonUnitTestPaths({bentoOnly});
if (argv.files || argv.filelist) {
this.files = commonUnitTestPaths
.concat(getFilesFromArgv())
Expand All @@ -166,10 +170,12 @@ class RuntimeTestConfig {
return;
}
if (argv.local_changes) {
this.files = commonUnitTestPaths.concat(unitTestsToRun());
this.files = commonUnitTestPaths.concat(unitTestsToRun({bentoOnly}));
return;
}
this.files = commonUnitTestPaths.concat(unitTestPaths);
this.files = commonUnitTestPaths.concat(
bentoOnly ? bentoUnitTestPaths : unitTestPaths
);
return;

case 'integration':
Expand Down
6 changes: 4 additions & 2 deletions build-system/tasks/unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,13 @@ class Runner extends RuntimeTestRunner {
* @return {Promise<void>}
*/
async function unit() {
const {bento_only: bentoOnly} = argv;
maybePrintArgvMessages();
if (argv.local_changes && !(await getUnitTestsToRun())) {
if (argv.local_changes && !(await getUnitTestsToRun({bentoOnly}))) {
return;
}

const config = new RuntimeTestConfig('unit');
const config = new RuntimeTestConfig('unit', {bentoOnly});
const runner = new Runner(config);

await runner.setup();
Expand All @@ -51,6 +52,7 @@ module.exports = {

unit.description = 'Run unit tests';
unit.flags = {
'bento_only': 'Run only bento standalone tests',
'chrome_canary': 'Run tests on Chrome Canary',
'chrome_flags': 'Use the given flags to launch Chrome',
'coverage': 'Run tests in code coverage mode',
Expand Down
22 changes: 19 additions & 3 deletions build-system/test-configs/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
* }>
*/
const initTestsPath = ['testing/init-tests.js'];
const initBentoTestsPaths = ['testing/init-bento-tests.js'];

const karmaHtmlFixturesPath = 'test/fixtures/*.html';

Expand Down Expand Up @@ -66,11 +67,18 @@ const karmaJsPaths = [
'test/**/*.js',
'ads/**/test/test-*.js',
'extensions/**/test/**/*.js',
'src/bento/components/**/test/*.js',
'src/bento/components/**/test/**/*.js',
'testing/**/*.js',
];

const commonUnitTestPaths = initTestsPath.concat(fixturesExamplesPaths);
/**
* @param {{bentoOnly?: boolean}} [options]
* @return {Array<*>}
*/
function getCommonUnitTestPaths({bentoOnly = false} = {}) {
const initTestPaths = bentoOnly ? initBentoTestsPaths : initTestsPath;
return initTestPaths.concat(fixturesExamplesPaths);
}

const commonIntegrationTestPaths = initTestsPath.concat(
fixturesExamplesPaths,
Expand All @@ -94,6 +102,13 @@ const unitTestPaths = [
'src/bento/components/**/test/unit/*.js',
];

const bentoUnitTestPaths = [
'extensions/**/test/standalone/*.js',
'extensions/**/test/standalone/unit/*.js',
'src/bento/components/**/test/standalone/*.js',
'src/bento/components/**/test/standalone/unit/*.js',
];

// TODO(amphtml): Opt-in more unit tests to run on Safari / FF / Edge.
const unitTestCrossBrowserPaths = [
'test/unit/core/test-error.js',
Expand Down Expand Up @@ -466,9 +481,10 @@ const changelogIgnoreFileTypes = /\.md|\.json|\.yaml|LICENSE|CONTRIBUTORS$/;

/** @const */
module.exports = {
bentoUnitTestPaths,
changelogIgnoreFileTypes,
commonIntegrationTestPaths,
commonUnitTestPaths,
getCommonUnitTestPaths,
devDashboardTestPaths,
e2eTestPaths,
htmlFixtureGlobs,
Expand Down
8 changes: 4 additions & 4 deletions build-system/test-configs/forbidden-terms.js
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ const forbiddenTermsGlobal = {
'src/amp-shadow.js',
'src/inabox/amp-inabox.js',
'src/service/ampdoc-impl.js',
'testing/init-tests.js',
'testing/init-tests-helpers.js',
'testing/describes.js',
'testing/iframe.js',
],
Expand Down Expand Up @@ -470,7 +470,7 @@ const forbiddenTermsGlobal = {
'src/service/cid-impl.js',
'src/service/standard-actions-impl.js',
'src/service/storage-impl.js',
'testing/init-tests.js',
'testing/init-tests-helpers.js',
'testing/fake-dom.js',
],
},
Expand Down Expand Up @@ -639,7 +639,7 @@ const forbiddenTermsGlobal = {
'src/core/mode/test.js',
'src/core/mode/local-dev.js',
'src/web-worker/web-worker.js', // Web worker custom error reporter.
'testing/init-tests.js',
'testing/init-tests-helpers.js',
'tools/experiments/experiments.js',
],
},
Expand Down Expand Up @@ -674,7 +674,7 @@ const forbiddenTermsGlobal = {
'Use of `this.skip()` is forbidden in test files. Use ' +
'`this.skipTest()` from within a `before()` block instead. See #17245.',
checkInTestFolder: true,
allowlist: ['testing/init-tests.js'],
allowlist: ['testing/init-tests-helpers.js'],
},
'[^\\.]makeBodyVisible\\(': {
message:
Expand Down
Loading

0 comments on commit 8614666

Please sign in to comment.