From 4745125c2417d057025741d0fa0422f1ba583eb2 Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Tue, 15 Feb 2022 19:48:24 +0000 Subject: [PATCH 1/4] CLI: Add support for watching `.json`, `.cjs`, and `.mjs` files And add tests for verifying which files are and aren't reacted to, including coverage for the existing ignoring of events from "node_modules". This is prep for https://github.com/qunitjs/qunit/issues/1669. --- src/cli/run.js | 6 ++- test/cli/cli-watch.js | 54 +++++++++++++++++++ .../fixtures/expected/watch-tap-outputs.js | 23 ++++++++ test/cli/helpers/execute.js | 2 +- 4 files changed, 83 insertions(+), 2 deletions(-) diff --git a/src/cli/run.js b/src/cli/run.js index afcafb0f9..c761f759b 100644 --- a/src/cli/run.js +++ b/src/cli/run.js @@ -184,13 +184,17 @@ run.watch = function watch() { const args = Array.prototype.slice.call( arguments ); const baseDir = process.cwd(); + // Include ".json" for test suites that use a data files, + // and for changes to package.json that may affect how a file is parsed (e.g. type=module). + const includeExts = [ ".js", ".json", ".cjs", ".mjs" ]; + const watcher = watch( baseDir, { persistent: true, recursive: true, delay: 0, filter: ( fullpath ) => { return !/\/node_modules\//.test( fullpath ) && - /\.js$/.test( fullpath ); + includeExts.includes( path.extname( fullpath ) ); } }, ( event, fullpath ) => { console.log( `File ${event}: ${path.relative( baseDir, fullpath )}` ); diff --git a/test/cli/cli-watch.js b/test/cli/cli-watch.js index 434473462..e2bce6292 100644 --- a/test/cli/cli-watch.js +++ b/test/cli/cli-watch.js @@ -28,6 +28,10 @@ if ( process.platform === "win32" ) { } QUnit.module( "CLI Watch", function( hooks ) { + hooks.before( function() { + rimraf.sync( fixturePath ); + } ); + hooks.beforeEach( function() { fs.mkdirSync( path.dirname( fixturePath ), { recursive: true } ); fixturify.writeSync( fixturePath, { @@ -171,6 +175,56 @@ QUnit.module( "CLI Watch", function( hooks ) { assert.equal( result.stdout, expectedWatchOutput[ "remove-file" ] ); } ); + // Skip in coverage mode since NYC adds non-default extensions + QUnit[ process.env.NYC_PROCESS_ID ? "skip" : "test" ]( "default file extensions", async assert => { + fixturify.writeSync( fixturePath, { + "tests": { + "setup.js": "QUnit.on('runEnd', function() { process.send('runEnd'); });", + "foo.js": "QUnit.test('foo', function(assert) { assert.true(true); });" + } + } ); + + const command = [ "qunit", "--watch", "watching/tests" ]; + const result = await executeIpc( + command, + execution => { + execution.once( "message", () => { + fixturify.writeSync( fixturePath, { + "x.cjs": "-", + "x.js": "-", + "x.json": "-", + "x.mjs": "-", + "x.ts": "-", + "x.txt": "-", + + "node_modules": { + "x": { + "y.js": "-" + } + }, + + "tests": { + "foo.js": "QUnit.test('foo2', function(assert) { assert.true(true); });", + "setup.js": "QUnit.on('runEnd', function() { process.send('runEnd2'); });" + } + } ); + + execution.once( "message", data => { + + // Ignore other re-runs + if ( data === "runEnd2" ) { + kill( execution ); + } + } ); + } ); + } + ); + + assert.equal( result.code, 0 ); + assert.equal( result.stderr, "" ); + assert.equal( result.stdout, expectedWatchOutput[ "file-extensions" ] ); + } ); + QUnit.test( "aborts and restarts when in middle of run", async assert => { // A proper abort finishes the currently running test and runs any remaining diff --git a/test/cli/fixtures/expected/watch-tap-outputs.js b/test/cli/fixtures/expected/watch-tap-outputs.js index 19ac14c29..8021e5229 100644 --- a/test/cli/fixtures/expected/watch-tap-outputs.js +++ b/test/cli/fixtures/expected/watch-tap-outputs.js @@ -62,6 +62,29 @@ ok 1 foo # skip 0 # todo 0 # fail 0 +Stopping QUnit...`, + + "file-extensions": `TAP version 13 +ok 1 foo +1..1 +# pass 1 +# skip 0 +# todo 0 +# fail 0 +File update: watching/x.cjs +File update: watching/x.js +File update: watching/x.json +File update: watching/x.mjs +File update: watching/tests/foo.js +File update: watching/tests/setup.js +Restarting... +TAP version 13 +ok 1 foo2 +1..1 +# pass 1 +# skip 0 +# todo 0 +# fail 0 Stopping QUnit...`, "change-file-mid-run": `TAP version 13 diff --git a/test/cli/helpers/execute.js b/test/cli/helpers/execute.js index 13784fdc3..6fbdb4145 100644 --- a/test/cli/helpers/execute.js +++ b/test/cli/helpers/execute.js @@ -110,7 +110,7 @@ module.exports.execute = async function execute( command, options = {}, hook ) { result.code = exitCode; const stderr = normalize( String( result.stderr ).trimEnd() ); if ( exitCode !== 0 ) { - reject( new Error( "Error code " + exitCode + "\n" + stderr ) ); + reject( new Error( "Error code " + exitCode + "\n" + ( stderr || result.stdout ) ) ); } else { resolve(); } From b031b0084dfd3d646000063d1ea446bcf14ce58d Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Tue, 15 Feb 2022 19:54:13 +0000 Subject: [PATCH 2/4] CLI: Reduce watch startup time by recursively skipping ignored directories Previously the callback would return true for "cwd/node_modules", then return false for all its immediate sub directories, and also for all its indirect subdirectories. While these directories were eventually watched, it could sometimes take considerable time to iterate over these. Use node-watch's skip feature to improve on this. And while at it, move it up a level by ignoring the top-level node_modules as well, not merely paths that we happen to see somewhere within it. Do both just in case we don't see the top level for some reason. --- src/cli/run.js | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/cli/run.js b/src/cli/run.js index c761f759b..c0511eb2c 100644 --- a/src/cli/run.js +++ b/src/cli/run.js @@ -187,14 +187,21 @@ run.watch = function watch() { // Include ".json" for test suites that use a data files, // and for changes to package.json that may affect how a file is parsed (e.g. type=module). const includeExts = [ ".js", ".json", ".cjs", ".mjs" ]; + const ignoreDirs = [ "node_modules" ]; const watcher = watch( baseDir, { persistent: true, recursive: true, - delay: 0, - filter: ( fullpath ) => { - return !/\/node_modules\//.test( fullpath ) && - includeExts.includes( path.extname( fullpath ) ); + + // Bare minimum delay, we have another debounce in run.restart(). + delay: 10, + filter: ( fullpath, skip ) => { + if ( /\/node_modules\//.test( fullpath ) || + ignoreDirs.includes( path.basename( fullpath ) ) + ) { + return skip; + } + return includeExts.includes( path.extname( fullpath ) ); } }, ( event, fullpath ) => { console.log( `File ${event}: ${path.relative( baseDir, fullpath )}` ); From c7a3cbf4296288bf0a8776131fb73eacedc0a6c9 Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Tue, 15 Feb 2022 19:58:06 +0000 Subject: [PATCH 3/4] CLI: Reduce watch startup time by ignoring ".git" directory --- src/cli/run.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cli/run.js b/src/cli/run.js index c0511eb2c..ae1c81556 100644 --- a/src/cli/run.js +++ b/src/cli/run.js @@ -187,7 +187,7 @@ run.watch = function watch() { // Include ".json" for test suites that use a data files, // and for changes to package.json that may affect how a file is parsed (e.g. type=module). const includeExts = [ ".js", ".json", ".cjs", ".mjs" ]; - const ignoreDirs = [ "node_modules" ]; + const ignoreDirs = [ ".git", "node_modules" ]; const watcher = watch( baseDir, { persistent: true, From b7c127c3ae6937f5279a2e2bf00b30f46bcdd03a Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Tue, 15 Feb 2022 19:58:33 +0000 Subject: [PATCH 4/4] CLI: Add support for watching `.ts` files when TypeScript is used Change `run.watch()` to also apply the `--require` file, so that we can observe any changes to `require.extensions`. Fixes https://github.com/qunitjs/qunit/issues/1669. --- src/cli/run.js | 24 +++++++++---- test/cli/cli-watch.js | 36 +++++++++++++++++++ .../fixtures/expected/watch-tap-outputs.js | 21 +++++++++++ 3 files changed, 75 insertions(+), 6 deletions(-) diff --git a/src/cli/run.js b/src/cli/run.js index ae1c81556..4e5fdb7f8 100644 --- a/src/cli/run.js +++ b/src/cli/run.js @@ -8,7 +8,8 @@ const requireQUnit = require( "./require-qunit" ); const utils = require( "./utils" ); const { findReporter } = require( "./find-reporter" ); -const RESTART_DEBOUNCE_LENGTH = 200; +const DEBOUNCE_WATCH_LENGTH = 60; +const DEBOUNCE_RESTART_LENGTH = 200 - DEBOUNCE_WATCH_LENGTH; const changedPendingPurge = []; @@ -158,7 +159,7 @@ run.restart = function( args ) { } run.abort( () => run.apply( null, args ) ); - }, RESTART_DEBOUNCE_LENGTH ); + }, DEBOUNCE_RESTART_LENGTH ); }; run.abort = function( callback ) { @@ -179,14 +180,25 @@ run.abort = function( callback ) { } }; -run.watch = function watch() { +run.watch = function watch( _, options ) { const watch = require( "node-watch" ); const args = Array.prototype.slice.call( arguments ); const baseDir = process.cwd(); - // Include ".json" for test suites that use a data files, + QUnit = requireQUnit(); + global.QUnit = QUnit; + options.requires.forEach( requireFromCWD ); + + // Include TypeScript when in use (automatically via require.extensions), + // https://github.com/qunitjs/qunit/issues/1669. + // + // Include ".json" (part of require.extensions) for test suites that use a data files, // and for changes to package.json that may affect how a file is parsed (e.g. type=module). - const includeExts = [ ".js", ".json", ".cjs", ".mjs" ]; + // + // Include ".cjs" and ".mjs", which Node.js doesn't expose via require.extensions by default. + // + // eslint-disable-next-line node/no-deprecated-api + const includeExts = Object.keys( require.extensions ).concat( [ ".cjs", ".mjs" ] ); const ignoreDirs = [ ".git", "node_modules" ]; const watcher = watch( baseDir, { @@ -194,7 +206,7 @@ run.watch = function watch() { recursive: true, // Bare minimum delay, we have another debounce in run.restart(). - delay: 10, + delay: DEBOUNCE_WATCH_LENGTH, filter: ( fullpath, skip ) => { if ( /\/node_modules\//.test( fullpath ) || ignoreDirs.includes( path.basename( fullpath ) ) diff --git a/test/cli/cli-watch.js b/test/cli/cli-watch.js index e2bce6292..7a1a9b5e5 100644 --- a/test/cli/cli-watch.js +++ b/test/cli/cli-watch.js @@ -225,6 +225,42 @@ QUnit.module( "CLI Watch", function( hooks ) { assert.equal( result.stdout, expectedWatchOutput[ "file-extensions" ] ); } ); + // Skip in coverage mode since NYC adds non-default extensions + QUnit[ process.env.NYC_PROCESS_ID ? "skip" : "test" ]( "TypeScript file extension", async assert => { + fixturify.writeSync( fixturePath, { + "register.js": "require.extensions['.ts'] = function() {};", + "tests": { + "setup.js": "QUnit.on('runEnd', function() { process.send('runEnd'); });", + "foo.js": "QUnit.test('foo', function(assert) { assert.true(true); });" + } + } ); + + const command = [ "qunit", "--watch", "--require", "./watching/register", "watching/tests" ]; + const result = await executeIpc( + command, + execution => { + execution.once( "message", () => { + fixturify.writeSync( fixturePath, { + "x.js": "-", + "x.ts": "-", + "tests": { + "foo.js": "QUnit.test('foo2', function(assert) { assert.true(true); });", + "setup.js": "QUnit.on('runEnd', function() { process.send('runEnd2'); });" + } + } ); + + execution.once( "message", () => { + kill( execution ); + } ); + } ); + } + ); + + assert.equal( result.code, 0 ); + assert.equal( result.stderr, "" ); + assert.equal( result.stdout, expectedWatchOutput[ "file-extension-ts" ] ); + } ); + QUnit.test( "aborts and restarts when in middle of run", async assert => { // A proper abort finishes the currently running test and runs any remaining diff --git a/test/cli/fixtures/expected/watch-tap-outputs.js b/test/cli/fixtures/expected/watch-tap-outputs.js index 8021e5229..5a5029e45 100644 --- a/test/cli/fixtures/expected/watch-tap-outputs.js +++ b/test/cli/fixtures/expected/watch-tap-outputs.js @@ -85,6 +85,27 @@ ok 1 foo2 # skip 0 # todo 0 # fail 0 +Stopping QUnit...`, + + "file-extension-ts": `TAP version 13 +ok 1 foo +1..1 +# pass 1 +# skip 0 +# todo 0 +# fail 0 +File update: watching/x.js +File update: watching/x.ts +File update: watching/tests/foo.js +File update: watching/tests/setup.js +Restarting... +TAP version 13 +ok 1 foo2 +1..1 +# pass 1 +# skip 0 +# todo 0 +# fail 0 Stopping QUnit...`, "change-file-mid-run": `TAP version 13