From bf518c2b2cb09025c28c07c2686ee71585ace071 Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Tue, 25 Apr 2023 14:15:25 -0400 Subject: [PATCH] [snap tester] watch mode: ignore changes from test updates A bit of a hack - We currently trigger test runs when we detect changes in the test fixtures directory. This trigger is also hit when we run `snap` in update mode, since updating performs file writes. This PR will ignore subscription changes (callbacks) that trigger within 5 seconds of the last update. It seems difficult to be more granular with a timestamp, since `@parcel/watcher` doesn't give us the file change timestamp and (from my understanding), other promises and tasks can be queued to run between the update and callback. --- compiler/forget/packages/snap/src/runner.ts | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/compiler/forget/packages/snap/src/runner.ts b/compiler/forget/packages/snap/src/runner.ts index d290ebbe59..4787670dea 100644 --- a/compiler/forget/packages/snap/src/runner.ts +++ b/compiler/forget/packages/snap/src/runner.ts @@ -329,6 +329,15 @@ export async function main(opts: RunnerOptions): Promise { // safe to use a cached compiler version let compilerVersion = 0; let isCompilerValid = false; + let lastUpdate = -1; + + function isRealUpdate(): boolean { + // Try to ignore changes that occurred as a result of our explicitly updating + // fixtures in update(). + // Currently keeps a timestamp of last known changes, and ignore events that occurred + // around that timestamp. + return performance.now() - lastUpdate > 5000; + } function onStart() { // Notify the user when compilation starts but don't clear the screen yet @@ -349,6 +358,9 @@ export async function main(opts: RunnerOptions): Promise { report(results); } const end = performance.now(); + if (mode === Mode.Update) { + lastUpdate = end; + } console.log(`Completed in ${Math.floor(end - start)} ms`); } else { console.error( @@ -378,9 +390,6 @@ export async function main(opts: RunnerOptions): Promise { }); // Watch the fixtures directory for changes - // TODO: ignore changes that occurred as a result of our explicitly updating - // fixtures in update() - maybe keep a timestamp of last known changes, and - // ignore events that occurred prior to that timestamp. const fileSubscription = watcher.subscribe( FIXTURES_PATH, async (err, _events) => { @@ -388,8 +397,10 @@ export async function main(opts: RunnerOptions): Promise { console.error(err); process.exit(1); } - // Fixtures changed, re-run tests - onChange({ mode: Mode.Test }); + if (isRealUpdate()) { + // Fixtures changed, re-run tests + onChange({ mode: Mode.Test }); + } } );