Skip to content

Commit

Permalink
🏗 Derive ava target paths (#36402)
Browse files Browse the repository at this point in the history
Derive directories that should trigger `amp ava` tests based on the test files location.

This prevents us from accidentally submitting new test files without a matching target directory (like I had to fix on 596fe2d).

Directories are the same as the test file, or its parent if the directory is named `test`. This matches the same fileset as before.
  • Loading branch information
alanorozco committed Oct 19, 2021
1 parent 3c3d825 commit 2e2efb5
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 22 deletions.
12 changes: 2 additions & 10 deletions build-system/pr-check/build-targets.js
Expand Up @@ -14,6 +14,7 @@ const {getLoggingPrefix, logWithoutTimestamp} = require('../common/logging');
const {gitDiffNameOnlyMain} = require('../common/git');
const {ignoreListFiles} = require('../tasks/check-ignore-lists');
const {isCiBuild} = require('../common/ci');
const {shouldTriggerAva} = require('../tasks/ava');

/**
* Used to prevent the repeated recomputing of build targets during PR jobs.
Expand Down Expand Up @@ -115,16 +116,7 @@ const targetMatchers = {
if (isOwnersFile(file)) {
return false;
}
return (
file == 'build-system/tasks/ava.js' ||
file.startsWith('build-system/release-tagger/') ||
file.startsWith('build-system/server/') ||
file.startsWith('build-system/tasks/css/') ||
file.startsWith('build-system/tasks/get-zindex/') ||
file.startsWith('build-system/tasks/make-extension/') ||
file.startsWith('build-system/tasks/markdown-toc/') ||
file.startsWith('build-system/tasks/prepend-global/')
);
return shouldTriggerAva(file);
},
[Targets.BABEL_PLUGIN]: (file) => {
if (isOwnersFile(file)) {
Expand Down
46 changes: 34 additions & 12 deletions build-system/tasks/ava.js
@@ -1,25 +1,46 @@
'use strict';

const argv = require('minimist')(process.argv.slice(2));
const {dirname, relative} = require('path');
const {execOrDie} = require('../common/exec');
const {sync: globbySync} = require('globby');

const testFiles = [
'build-system/release-tagger/test/*test*.js',
'build-system/server/app-index/test/*test*.js',
'build-system/server/test/app-utils.test.js',
'build-system/tasks/css/test/bento-css.test.js',
'build-system/tasks/get-zindex/get-zindex.test.js',
'build-system/tasks/make-extension/test/test.js',
'build-system/tasks/markdown-toc/test/test.js',
'build-system/tasks/prepend-global/prepend-global.test.js',
];

let targetFiles;

/**
* Determines whether to trigger ava by a file change adjacent to a test file.
* Considered adjacent when the file changed is in the same directory as one of
* the listed test files, or its parent if the directory is named `test`.
* @param {string} changedFile
* @return {boolean}
*/
function shouldTriggerAva(changedFile) {
if (!targetFiles) {
const thisFile = relative(process.cwd(), __filename);
const patterns = testFiles.map(
(pattern) => dirname(pattern).replace(/\/test$/, '') + '/'
);
targetFiles = new Set([thisFile, ...globbySync(patterns)]);
}
return targetFiles.has(changedFile);
}

/**
* Runs ava tests.
* @return {Promise<void>}
*/
async function ava() {
// These need equivalents for CI in build-system/pr-check/build-targets.js
// (see targetMatchers[Targets.AVA])
const testFiles = [
'build-system/release-tagger/test/*test*.js',
'build-system/server/app-index/test/*test*.js',
'build-system/server/test/app-utils.test.js',
'build-system/tasks/css/test/bento-css.test.js',
'build-system/tasks/get-zindex/get-zindex.test.js',
'build-system/tasks/make-extension/test/test.js',
'build-system/tasks/markdown-toc/test/test.js',
'build-system/tasks/prepend-global/prepend-global.test.js',
];
execOrDie(
[
'npx ava',
Expand All @@ -32,6 +53,7 @@ async function ava() {

module.exports = {
ava,
shouldTriggerAva,
};

ava.description = "Run ava tests for AMP's tasks";
Expand Down

0 comments on commit 2e2efb5

Please sign in to comment.