Merge pull request #26741 from Microsoft/gulpWatch

Fix overlapping test runs in 'gulp watch'

Author: rbuckton

Gulpfile.js

@@ -24,10 +24,9 @@ const baselineAccept = require("./scripts/build/baselineAccept");
 const cmdLineOptions = require("./scripts/build/options");
 const exec = require("./scripts/build/exec");
 const browserify = require("./scripts/build/browserify");
-const debounce = require("./scripts/build/debounce");
 const prepend = require("./scripts/build/prepend");
 const { removeSourceMaps } = require("./scripts/build/sourcemaps");
-const { CancelSource, CancelError } = require("./scripts/build/cancellation");
+const { CancellationTokenSource, CancelError, delay, Semaphore } = require("prex"); 
 const { libraryTargets, generateLibs } = require("./scripts/build/lib");
 const { runConsoleTests, cleanTestDirs, writeTestConfigFile, refBaseline, localBaseline, refRwcBaseline, localRwcBaseline } = require("./scripts/build/tests");
 
@@ -534,57 +533,80 @@ gulp.task(
     ["watch-diagnostics", "watch-lib"].concat(useCompilerDeps),
     () => project.watch(tsserverProject, { typescript: useCompiler }));
 
-gulp.task(
-    "watch-local",
-    /*help*/ false,
-    ["watch-lib", "watch-tsc", "watch-services", "watch-server"]);
-
 gulp.task(
     "watch-runner",
     /*help*/ false,
     useCompilerDeps,
     () => project.watch(testRunnerProject, { typescript: useCompiler }));
 
-const watchPatterns = [
-    runJs,
-    typescriptDts,
-    tsserverlibraryDts
-];
+gulp.task(
+    "watch-local",
+    "Watches for changes to projects in src/ (but does not execute tests).",
+    ["watch-lib", "watch-tsc", "watch-services", "watch-server", "watch-runner", "watch-lssl"]);
 
 gulp.task(
     "watch",
-    "Watches for changes to the build inputs for built/local/run.js, then executes runtests-parallel.",
+    "Watches for changes to the build inputs for built/local/run.js, then runs tests.",
     ["build-rules", "watch-runner", "watch-services", "watch-lssl"],
     () => {
-        /** @type {CancelSource | undefined} */
-        let runTestsSource;
+        const sem = new Semaphore(1);
 
-        const fn = debounce(() => {
-            runTests().catch(error => {
-                if (error instanceof CancelError) {
-                    log.warn("Operation was canceled");
-                }
-                else {
-                    log.error(error);
-                }
-            });
-        }, /*timeout*/ 100, { max: 500 });
-
-        gulp.watch(watchPatterns, () => project.wait().then(fn));
+        gulp.watch([runJs, typescriptDts, tsserverlibraryDts], () => {
+            runTests();
+        });
 
         // NOTE: gulp.watch is far too slow when watching tests/cases/**/* as it first enumerates *every* file
         const testFilePattern = /(\.ts|[\\/]tsconfig\.json)$/;
         fs.watch("tests/cases", { recursive: true }, (_, file) => {
-            if (testFilePattern.test(file)) project.wait().then(fn);
+            if (testFilePattern.test(file)) runTests();
         });
 
-        function runTests() {
-            if (runTestsSource) runTestsSource.cancel();
-            runTestsSource = new CancelSource();
-            return cmdLineOptions.tests || cmdLineOptions.failed
-                ? runConsoleTests(runJs, "mocha-fivemat-progress-reporter", /*runInParallel*/ false, /*watchMode*/ true, runTestsSource.token)
-                : runConsoleTests(runJs, "min", /*runInParallel*/ true, /*watchMode*/ true, runTestsSource.token);
-        }
+        async function runTests() {
+            try {
+                // Ensure only one instance of the test runner is running at any given time.
+                if (sem.count > 0) {
+                    await sem.wait();
+                    try {
+                        // Wait for any concurrent recompilations to complete...
+                        try {
+                            await delay(100);
+                            while (project.hasRemainingWork()) {
+                                await project.waitForWorkToComplete();
+                                await delay(500);
+                            }
+                        }
+                        catch (e) {
+                            if (e instanceof CancelError) return;
+                            throw e;
+                        }
+
+                        // cancel any pending or active test run if a new recompilation is triggered
+                        const source = new CancellationTokenSource();
+                        project.waitForWorkToStart().then(() => {
+                            source.cancel();
+                        });
+                    
+                        if (cmdLineOptions.tests || cmdLineOptions.failed) {
+                            await runConsoleTests(runJs, "mocha-fivemat-progress-reporter", /*runInParallel*/ false, /*watchMode*/ true, source.token);
+                        }
+                        else {
+                            await runConsoleTests(runJs, "min", /*runInParallel*/ true, /*watchMode*/ true, source.token);
+                        }
+                    }
+                    finally {
+                        sem.release();
+                    }
+                }
+            }
+            catch (e) {
+                if (e instanceof CancelError) {
+                    log.warn("Operation was canceled");
+                }
+                else {
+                    log.error(e);
+                }
+            }
+        };
     });
 
 gulp.task("clean-built", /*help*/ false, [`clean:${diagnosticInformationMapTs}`], () => del(["built"]));

package.json

@@ -81,6 +81,7 @@
         "mocha": "latest",
         "mocha-fivemat-progress-reporter": "latest",
         "plugin-error": "latest",
+        "prex": "^0.4.3",
         "q": "latest",
         "remove-internal": "^2.9.2",
         "run-sequence": "latest",

scripts/build/cancellation.js

@@ -3,7 +3,7 @@ const cp = require("child_process");
 const log = require("fancy-log"); // was `require("gulp-util").log (see https://github.com/gulpjs/gulp-util)
 const isWin = /^win/.test(process.platform);
 const chalk = require("./chalk");
-const { CancelToken, CancelError } = require("./cancellation");
+const { CancellationToken, CancelError } = require("prex");
 
 module.exports = exec;
 
@@ -15,31 +15,36 @@ module.exports = exec;
  *
  * @typedef ExecOptions
  * @property {boolean} [ignoreExitCode]
- * @property {CancelToken} [cancelToken]
+ * @property {import("prex").CancellationToken} [cancelToken]
  */
 function exec(cmd, args, options = {}) {
     return /**@type {Promise<{exitCode: number}>}*/(new Promise((resolve, reject) => {
-        log(`> ${chalk.green(cmd)} ${args.join(" ")}`);
+        const { ignoreExitCode, cancelToken = CancellationToken.none } = options;
+        cancelToken.throwIfCancellationRequested();
+
         // TODO (weswig): Update child_process types to add windowsVerbatimArguments to the type definition
         const subshellFlag = isWin ? "/c" : "-c";
         const command = isWin ? [possiblyQuote(cmd), ...args] : [`${cmd} ${args.join(" ")}`];
-        const ex = cp.spawn(isWin ? "cmd" : "/bin/sh", [subshellFlag, ...command], { stdio: "inherit", windowsVerbatimArguments: true });
-        const subscription = options.cancelToken && options.cancelToken.subscribe(() => {
-            ex.kill("SIGINT");
-            ex.kill("SIGTERM");
+        
+        log(`> ${chalk.green(cmd)} ${args.join(" ")}`);
+        const proc = cp.spawn(isWin ? "cmd" : "/bin/sh", [subshellFlag, ...command], { stdio: "inherit", windowsVerbatimArguments: true });
+        const registration = cancelToken.register(() => {
+            log(`${chalk.red("killing")} '${chalk.green(cmd)} ${args.join(" ")}'...`);
+            proc.kill("SIGINT");
+            proc.kill("SIGTERM");
             reject(new CancelError());
         });
-        ex.on("exit", exitCode => {
-            subscription && subscription.unsubscribe();
-            if (exitCode === 0 || options.ignoreExitCode) {
+        proc.on("exit", exitCode => {
+            registration.unregister();
+            if (exitCode === 0 || ignoreExitCode) {
                 resolve({ exitCode });
             }
             else {
                 reject(new Error(`Process exited with code: ${exitCode}`));
             }
         });
-        ex.on("error", error => {
-            subscription && subscription.unsubscribe();
+        proc.on("error", error => {
+            registration.unregister();
             reject(error);
         });
     }));

scripts/build/exec.js

@@ -3,6 +3,8 @@ const path = require("path");
 const fs = require("fs");
 const gulp = require("./gulp");
 const gulpif = require("gulp-if");
+const log = require("fancy-log"); // was `require("gulp-util").log (see https://github.com/gulpjs/gulp-util)
+const chalk = require("./chalk");
 const sourcemaps = require("gulp-sourcemaps");
 const merge2 = require("merge2");
 const tsc = require("gulp-typescript");
@@ -12,23 +14,52 @@ const ts = require("../../lib/typescript");
 const del = require("del");
 const needsUpdate = require("./needsUpdate");
 const mkdirp = require("./mkdirp");
+const prettyTime = require("pretty-hrtime");
 const { reportDiagnostics } = require("./diagnostics");
+const { CountdownEvent, ManualResetEvent } = require("prex");
+
+const workStartedEvent = new ManualResetEvent();
+const countdown = new CountdownEvent(0);
 
 class CompilationGulp extends gulp.Gulp {
     /**
      * @param {boolean} [verbose]
      */
     fork(verbose) {
         const child = new ForkedGulp(this.tasks);
-        if (verbose) {
-            child.on("task_start", e => gulp.emit("task_start", e));
-            child.on("task_stop", e => gulp.emit("task_stop", e));
-            child.on("task_err", e => gulp.emit("task_err", e));
-            child.on("task_not_found", e => gulp.emit("task_not_found", e));
-            child.on("task_recursion", e => gulp.emit("task_recursion", e));
-        }
+        child.on("task_start", e => {
+            if (countdown.remainingCount === 0) {
+                countdown.reset(1);
+                workStartedEvent.set();
+                workStartedEvent.reset();
+            }
+            else {
+                countdown.add();
+            }
+            if (verbose) {
+                log('Starting', `'${chalk.cyan(e.task)}' ${chalk.gray(`(${countdown.remainingCount} remaining)`)}...`);
+            }
+        });
+        child.on("task_stop", e => {
+            countdown.signal();
+            if (verbose) {
+                log('Finished', `'${chalk.cyan(e.task)}' after ${chalk.magenta(prettyTime(/** @type {*}*/(e).hrDuration))} ${chalk.gray(`(${countdown.remainingCount} remaining)`)}`);
+            }
+        });
+        child.on("task_err", e => {
+            countdown.signal();
+            if (verbose) {
+                log(`'${chalk.cyan(e.task)}' ${chalk.red("errored after")} ${chalk.magenta(prettyTime(/** @type {*}*/(e).hrDuration))} ${chalk.gray(`(${countdown.remainingCount} remaining)`)}`);
+                log(e.err ? e.err.stack : e.message);
+            }
+        });
         return child;
     }
+
+    // @ts-ignore
+    start() {
+        throw new Error("Not supported, use fork.");
+    }
 }
 
 class ForkedGulp extends gulp.Gulp {
@@ -211,24 +242,26 @@ exports.flatten = flatten;
 
 /**
  * Returns a Promise that resolves when all pending build tasks have completed
+ * @param {import("prex").CancellationToken} [token]
  */
-function wait() {
-    return new Promise(resolve => {
-        if (compilationGulp.allDone()) {
-            resolve();
-        }
-        else {
-            const onDone = () => {
-                compilationGulp.removeListener("onDone", onDone);
-                compilationGulp.removeListener("err", onDone);
-                resolve();
-            };
-            compilationGulp.on("stop", onDone);
-            compilationGulp.on("err", onDone);
-        }
-    });
+function waitForWorkToComplete(token) {
+    return countdown.wait(token);
+}
+exports.waitForWorkToComplete = waitForWorkToComplete;
+
+/**
+ * Returns a Promise that resolves when all pending build tasks have completed
+ * @param {import("prex").CancellationToken} [token]
+ */
+function waitForWorkToStart(token) {
+    return workStartedEvent.wait(token);
+}
+exports.waitForWorkToStart = waitForWorkToStart;
+
+function getRemainingWork() {
+    return countdown.remainingCount > 0;
 }
-exports.wait = wait;
+exports.hasRemainingWork = getRemainingWork;
 
 /**
  * Resolve a TypeScript specifier into a fully-qualified module specifier and any requisite dependencies.