Skip to content

Commit

Permalink
πŸ—πŸ›πŸš€ Fix and substantially speed up gulp unit --local_changes (#28131)
Browse files Browse the repository at this point in the history
  • Loading branch information
rsimha authored Apr 30, 2020
1 parent f4cc497 commit 79b4634
Show file tree
Hide file tree
Showing 8 changed files with 60 additions and 840 deletions.
1 change: 0 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ yarn-error.log
sc-*-linux*
sc-*-osx*
sauce_connect_*
EXTENSIONS_CSS_MAP
deps.txt
firebase
.firebaserc
Expand Down
2 changes: 1 addition & 1 deletion build-system/pr-check/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ const ESM_DIST_OUTPUT_FILE = isTravisBuild()
? `amp_esm_dist_${travisBuildNumber()}.zip`
: '';

const BUILD_OUTPUT_DIRS = 'build/ dist/ dist.3p/ EXTENSIONS_CSS_MAP';
const BUILD_OUTPUT_DIRS = 'build/ dist/ dist.3p/';
const APP_SERVING_DIRS = 'dist.tools/ examples/ test/manual/';

const OUTPUT_STORAGE_LOCATION = 'gs://amp-travis-builds';
Expand Down
1 change: 0 additions & 1 deletion build-system/tasks/clean.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ async function clean() {
'build',
'.amp-build',
'deps.txt',
'EXTENSIONS_CSS_MAP',
'build-system/runner/build',
'build-system/runner/dist',
'build-system/server/new-server/transforms/dist',
Expand Down
5 changes: 1 addition & 4 deletions build-system/tasks/css.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const {
toPromise,
watchDebounceDelay,
} = require('./helpers');
const {buildExtensions, extensions} = require('./extension-helpers');
const {buildExtensions} = require('./extension-helpers');
const {jsifyCssAsync} = require('./jsify-css');
const {maybeUpdatePackages} = require('./update-packages');

Expand Down Expand Up @@ -133,9 +133,6 @@ function compileCss(watch) {

const startTime = Date.now();

// Used by `gulp unit --local_changes` to map CSS files to JS files.
fs.writeFileSync('EXTENSIONS_CSS_MAP', JSON.stringify(extensions));

let promise = Promise.resolve();

cssEntryPoints.forEach((entryPoint) => {
Expand Down
39 changes: 19 additions & 20 deletions build-system/tasks/runtime-test/helpers-unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,23 @@
*/
'use strict';

const findImports = require('find-imports-forked');
const fs = require('fs');
const globby = require('globby');
const listImportsExports = require('list-imports-exports');
const log = require('fancy-log');
const minimatch = require('minimatch');
const path = require('path');
const testConfig = require('../../test-configs/config');

const {execOrDie} = require('../../common/exec');
const {extensions, maybeInitializeExtensions} = require('../extension-helpers');
const {gitDiffNameOnlyMaster} = require('../../common/git');
const {green, cyan} = require('ansi-colors');
const {isTravisBuild} = require('../../common/travis');
const {reportTestSkipped} = require('../report-test-status');

const EXTENSIONSCSSMAP = 'EXTENSIONS_CSS_MAP';
const LARGE_REFACTOR_THRESHOLD = 50;
const ROOT_DIR = path.resolve(__dirname, '../../../');
let testsToRun = null;

/**
* Returns true if the PR is a large refactor.
Expand All @@ -43,15 +44,14 @@ function isLargeRefactor() {
}

/**
* Extracts a mapping from CSS files to JS files from a well known file
* generated during `gulp css`.
* Extracts extension info and creates a mapping from CSS files in different
* source directories to their equivalent JS files in the 'build/' directory.
*
* @return {!Object<string, string>}
*/
function extractCssJsFileMap() {
const extensionsCssMap = fs.readFileSync(EXTENSIONSCSSMAP, 'utf8');
const extensionsCssMapJson = JSON.parse(extensionsCssMap);
const extensions = Object.keys(extensionsCssMapJson);
execOrDie('gulp css', {'stdio': 'ignore'});
maybeInitializeExtensions(extensions);
const cssJsFileMap = {};

// Adds an entry that maps a CSS file to a JS file
Expand All @@ -63,8 +63,8 @@ function extractCssJsFileMap() {
cssJsFileMap[cssFilePath] = jsFilePath;
}

extensions.forEach((extension) => {
const cssData = extensionsCssMapJson[extension];
Object.keys(extensions).forEach((extension) => {
const cssData = extensions[extension];
if (cssData['hasCss']) {
addCssJsEntry(cssData, cssData['name'], cssJsFileMap);
if (cssData.hasOwnProperty('cssBinaries')) {
Expand All @@ -85,12 +85,8 @@ function extractCssJsFileMap() {
* @return {!Array<string>}
*/
function getImports(jsFile) {
const imports = findImports([jsFile], {
flatten: true,
packageImports: false,
absoluteImports: true,
relativeImports: true,
});
const jsFileContents = fs.readFileSync(jsFile, 'utf8');
const {imports} = listImportsExports.parse(jsFileContents);
const files = [];
const jsFileDir = path.dirname(jsFile);
imports.forEach(function (file) {
Expand Down Expand Up @@ -159,15 +155,18 @@ function getUnitTestsToRun() {

/**
* Extracts the list of unit tests to run based on the changes in the local
* branch.
* branch. Return value is cached to optimize for multiple calls.
*
* @param {!Array<string>} unitTestPaths
* @return {!Array<string>}
*/
function unitTestsToRun(unitTestPaths = testConfig.unitTestPaths) {
function unitTestsToRun() {
if (testsToRun) {
return testsToRun;
}
const cssJsFileMap = extractCssJsFileMap();
const filesChanged = gitDiffNameOnlyMaster();
const testsToRun = [];
const {unitTestPaths} = testConfig;
testsToRun = [];
let srcFiles = [];

function isUnitTest(file) {
Expand Down
2 changes: 1 addition & 1 deletion build-system/tasks/runtime-test/runtime-test-base.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ function getFiles(testType) {
return files.concat(testConfig.unitTestOnSaucePaths);
}
if (argv.local_changes) {
return files.concat(unitTestsToRun(testConfig.unitTestPaths));
return files.concat(unitTestsToRun());
}
return files.concat(testConfig.unitTestPaths);

Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@
"express": "4.17.1",
"fancy-log": "1.3.3",
"fetch-mock": "9.4.0",
"find-imports-forked": "1.3.2",
"formidable": "1.2.2",
"fs-extra": "9.0.0",
"fuse.js": "5.2.3",
Expand Down Expand Up @@ -143,6 +142,7 @@
"karma-source-map-support": "1.4.0",
"karma-super-dots-reporter": "0.2.0",
"lazypipe": "1.0.2",
"list-imports-exports": "0.1.2",
"lolex": "5.1.2",
"magic-string": "0.25.7",
"markdown-link-check": "3.8.1",
Expand Down
Loading

0 comments on commit 79b4634

Please sign in to comment.