From 2ea7dcb2c6854b7c70c4d48786e419f92792d482 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= Date: Tue, 30 Aug 2022 14:57:22 +0200 Subject: [PATCH 1/4] src: remove v8abbr.h The definitions in v8abbr.h, except for NODE_OFF_EXTSTR_DATA, were only used for dtrace, which has been removed. Refs: https://github.com/nodejs/node/pull/43652 PR-URL: https://github.com/nodejs/node/pull/44402 Reviewed-By: Joyee Cheung Reviewed-By: Anna Henningsen Reviewed-By: Jiawen Geng Reviewed-By: Ben Noordhuis Reviewed-By: James M Snell Reviewed-By: Darshan Sen --- .github/label-pr-config.yml | 2 +- src/node_postmortem_metadata.cc | 9 +- src/v8abbr.h | 123 ------------------- test/cctest/test_node_postmortem_metadata.cc | 3 +- 4 files changed, 8 insertions(+), 129 deletions(-) delete mode 100644 src/v8abbr.h diff --git a/.github/label-pr-config.yml b/.github/label-pr-config.yml index ac998eeb9d20a7..702cabfc7e1e65 100644 --- a/.github/label-pr-config.yml +++ b/.github/label-pr-config.yml @@ -21,7 +21,7 @@ subSystemLabels: /^src\/tty_/: c++, tty /^src\/node_url/: c++, whatwg-url /^src\/node_util/: c++, util - /^src\/(?:node_v8|v8abbr)/: c++, v8 engine + /^src\/node_v8/: c++, v8 engine /^src\/node_contextify/: c++, vm /^src\/.*win32.*/: c++, windows /^src\/node_zlib/: c++, zlib diff --git a/src/node_postmortem_metadata.cc b/src/node_postmortem_metadata.cc index 2dc534f59bf80a..ea7396e54c8107 100644 --- a/src/node_postmortem_metadata.cc +++ b/src/node_postmortem_metadata.cc @@ -1,10 +1,11 @@ -#include "env.h" #include "base_object-inl.h" +#include "env.h" #include "handle_wrap.h" -#include "util-inl.h" -#include "req_wrap.h" -#include "v8abbr.h" #include "node_context_data.h" +#include "req_wrap.h" +#include "util-inl.h" + +#define NODE_OFF_EXTSTR_DATA sizeof(void*) #define NODEDBG_SYMBOL(Name) nodedbg_ ## Name diff --git a/src/v8abbr.h b/src/v8abbr.h deleted file mode 100644 index 50145ae4d30fa2..00000000000000 --- a/src/v8abbr.h +++ /dev/null @@ -1,123 +0,0 @@ -/** Copyright Joyent, Inc. and other Node contributors. -* -* Permission is hereby granted, free of charge, to any person obtaining a -* copy of this software and associated documentation files (the -* "Software"), to deal in the Software without restriction, including -* without limitation the rights to use, copy, modify, merge, publish, -* distribute, sublicense, and/or sell copies of the Software, and to permit -* persons to whom the Software is furnished to do so, subject to the -* following conditions: -* -* The above copyright notice and this permission notice shall be included -* in all copies or substantial portions of the Software. -* -* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS -* OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF -* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN -* NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, -* DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR -* OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE -* USE OR OTHER DEALINGS IN THE SOFTWARE. -*/ - -/* - * This header defines short names for V8 constants for use by the ustack - * helper. - */ - -#ifndef SRC_V8ABBR_H_ -#define SRC_V8ABBR_H_ - -/* Frame pointer offsets */ -#define V8_OFF_FP_FUNC V8DBG_OFF_FP_FUNCTION -#define V8_OFF_FP_CONTEXT V8DBG_OFF_FP_CONTEXT -#define V8_OFF_FP_MARKER V8DBG_OFF_FP_MARKER - -/* Stack frame types */ -#define V8_FT_ENTRY V8DBG_FRAMETYPE_ENTRYFRAME -#define V8_FT_ENTRYCONSTRUCT V8DBG_FRAMETYPE_CONSTRUCTENTRYFRAME -#define V8_FT_EXIT V8DBG_FRAMETYPE_EXITFRAME -#define V8_FT_JAVASCRIPT V8DBG_FRAMETYPE_JAVASCRIPTFRAME -#define V8_FT_OPTIMIZED V8DBG_FRAMETYPE_OPTIMIZEDFRAME -#define V8_FT_INTERNAL V8DBG_FRAMETYPE_INTERNALFRAME -#define V8_FT_CONSTRUCT V8DBG_FRAMETYPE_CONSTRUCTFRAME -#define V8_FT_STUB V8DBG_FRAMETYPE_STUBFRAME - -/* Identification masks and tags */ -#define V8_SmiTagMask (V8DBG_SMITAGMASK) -#define V8_SmiTag (V8DBG_SMITAG) -#define V8_SmiValueShift (V8DBG_SMISHIFTSIZE + V8DBG_SMITAGMASK) - -#define V8_HeapObjectTagMask V8DBG_HEAPOBJECTTAGMASK -#define V8_HeapObjectTag V8DBG_HEAPOBJECTTAG - -#define V8_IsNotStringMask V8DBG_ISNOTSTRINGMASK -#define V8_StringTag V8DBG_STRINGTAG - -#define V8_StringEncodingMask V8DBG_STRINGENCODINGMASK -#define V8_AsciiStringTag V8DBG_ONEBYTESTRINGTAG - -#define V8_StringRepresentationMask V8DBG_STRINGREPRESENTATIONMASK -#define V8_SeqStringTag V8DBG_SEQSTRINGTAG -#define V8_ConsStringTag V8DBG_CONSSTRINGTAG -#define V8_ExternalStringTag V8DBG_EXTERNALSTRINGTAG - -/* Instance types */ -#define V8_IT_FIXEDARRAY V8DBG_TYPE_FIXEDARRAY__FIXED_ARRAY_TYPE -#define V8_IT_CODE V8DBG_TYPE_CODE__CODE_TYPE -#define V8_IT_SCRIPT V8DBG_TYPE_SCRIPT__SCRIPT_TYPE - -/* Node-specific offsets */ -#define NODE_OFF_EXTSTR_DATA sizeof(void*) - -/* - * Not all versions of V8 have the offset for the "chars" array in the - * SeqTwoByteString class, but it's the same as the one for SeqOneByteString. - */ -#ifndef V8DBG_CLASS_SEQTWOBYTESTRING__CHARS__CHAR -#define V8DBG_CLASS_SEQTWOBYTESTRING__CHARS__CHAR \ - V8DBG_CLASS_SEQONEBYTESTRING__CHARS__CHAR -#endif - -/* Heap class->field offsets */ -#define V8_OFF_HEAP(off) ((off) - 1) - -#define V8_OFF_FUNC_SHARED \ - V8_OFF_HEAP(V8DBG_CLASS_JSFUNCTION__SHARED__SHAREDFUNCTIONINFO) -#define V8_OFF_RAW_NAME \ - V8_OFF_HEAP(V8DBG_CLASS_SHAREDFUNCTIONINFO__NAME_OR_SCOPE_INFO__OBJECT) -#define V8_OFF_SHARED_IDENT \ - V8_OFF_HEAP( \ - V8DBG_CLASS_SHAREDFUNCTIONINFO__NAME_OR_SCOPE_INFO__OBJECT) -#define V8_OFF_SHARED_SCRIPT \ - V8_OFF_HEAP( \ - V8DBG_CLASS_SHAREDFUNCTIONINFO__SCRIPT_OR_DEBUG_INFO__HEAPOBJECT) -#define V8_OFF_SHARED_FUNIDENT \ - V8_OFF_HEAP( \ - V8DBG_CLASS_SHAREDFUNCTIONINFO__NAME_OR_SCOPE_INFO__OBJECT) -#define V8_OFF_SCRIPT_NAME \ - V8_OFF_HEAP(V8DBG_CLASS_SCRIPT__NAME__OBJECT) -#define V8_OFF_SCRIPT_LENDS \ - V8_OFF_HEAP(V8DBG_CLASS_SCRIPT__LINE_ENDS__OBJECT) -#define V8_OFF_STR_LENGTH \ - V8_OFF_HEAP(V8DBG_CLASS_STRING__LENGTH__INT32_T) -#define V8_OFF_STR_CHARS \ - V8_OFF_HEAP(V8DBG_CLASS_SEQONEBYTESTRING__CHARS__CHAR) -#define V8_OFF_CONSSTR_CAR \ - V8_OFF_HEAP(V8DBG_CLASS_CONSSTRING__FIRST__STRING) -#define V8_OFF_CONSSTR_CDR \ - V8_OFF_HEAP(V8DBG_CLASS_CONSSTRING__SECOND__STRING) -#define V8_OFF_EXTSTR_RSRC \ - V8_OFF_HEAP(V8DBG_CLASS_EXTERNALSTRING__RESOURCE__OBJECT) -#define V8_OFF_FA_SIZE \ - V8_OFF_HEAP(V8DBG_CLASS_FIXEDARRAYBASE__LENGTH__SMI) -#define V8_OFF_FA_DATA \ - V8_OFF_HEAP(V8DBG_CLASS_FIXEDARRAY__DATA__UINTPTR_T) -#define V8_OFF_HEAPOBJ_MAP \ - V8_OFF_HEAP(V8DBG_CLASS_HEAPOBJECT__MAP__MAP) -#define V8_OFF_MAP_ATTRS \ - V8_OFF_HEAP(V8DBG_CLASS_MAP__INSTANCE_TYPE__UINT16_T) -#define V8_OFF_TWOBYTESTR_CHARS \ - V8_OFF_HEAP(V8DBG_CLASS_SEQTWOBYTESTRING__CHARS__CHAR) - -#endif /* SRC_V8ABBR_H_ */ diff --git a/test/cctest/test_node_postmortem_metadata.cc b/test/cctest/test_node_postmortem_metadata.cc index 4cee7db4c8ee5b..d4faec35adf638 100644 --- a/test/cctest/test_node_postmortem_metadata.cc +++ b/test/cctest/test_node_postmortem_metadata.cc @@ -5,7 +5,8 @@ #include "req_wrap-inl.h" #include "tracing/agent.h" #include "v8.h" -#include "v8abbr.h" + +#define NODE_OFF_EXTSTR_DATA sizeof(void*) extern "C" { extern uintptr_t From 23e1b59e4c63ddbc15fa8298636ebbf7b3b95581 Mon Sep 17 00:00:00 2001 From: theanarkh Date: Wed, 31 Aug 2022 01:08:33 +0800 Subject: [PATCH 2/4] lib: add diagnostics channel for process and worker PR-URL: https://github.com/nodejs/node/pull/44045 Reviewed-By: James M Snell --- doc/api/diagnostics_channel.md | 29 +++++++++++++++++++ lib/internal/child_process.js | 7 +++++ lib/internal/worker.js | 8 +++++ .../test-diagnostics-channel-process.js | 21 ++++++++++++++ ...test-diagnostics-channel-worker-threads.js | 11 +++++++ 5 files changed, 76 insertions(+) create mode 100644 test/parallel/test-diagnostics-channel-process.js create mode 100644 test/parallel/test-diagnostics-channel-worker-threads.js diff --git a/doc/api/diagnostics_channel.md b/doc/api/diagnostics_channel.md index 761a0567a1435b..1b9f54237337c0 100644 --- a/doc/api/diagnostics_channel.md +++ b/doc/api/diagnostics_channel.md @@ -428,6 +428,8 @@ Emitted when server receives a request. Emitted when server sends a response. +#### NET + `net.client.socket` * `socket` {net.Socket} @@ -440,13 +442,40 @@ Emitted when a new TCP or pipe client socket is created. Emitted when a new TCP or pipe connection is received. +#### UDP + `udp.socket` * `socket` {dgram.Socket} Emitted when a new UDP socket is created. +#### Process + + + +`child_process` + +* `process` {ChildProcess} + +Emitted when a new process is created. + +#### Worker Thread + + + +`worker_threads` + +* `worker` [`Worker`][] + +Emitted when a new thread is created. + [`'uncaughtException'`]: process.md#event-uncaughtexception +[`Worker`]: worker_threads.md#class-worker [`channel.subscribe(onMessage)`]: #channelsubscribeonmessage [`diagnostics_channel.channel(name)`]: #diagnostics_channelchannelname [`diagnostics_channel.subscribe(name, onMessage)`]: #diagnostics_channelsubscribename-onmessage diff --git a/lib/internal/child_process.js b/lib/internal/child_process.js index 9ad7bf8fd8593c..f2fe406a4a27d9 100644 --- a/lib/internal/child_process.js +++ b/lib/internal/child_process.js @@ -59,6 +59,8 @@ const { convertToValidSignal, deprecate } = require('internal/util'); const { isArrayBufferView } = require('internal/util/types'); const spawn_sync = internalBinding('spawn_sync'); const { kStateSymbol } = require('internal/dgram'); +const dc = require('diagnostics_channel'); +const childProcessChannel = dc.channel('child_process'); const { UV_EACCES, @@ -301,6 +303,11 @@ function ChildProcess() { maybeClose(this); }; + if (childProcessChannel.hasSubscribers) { + childProcessChannel.publish({ + process: this, + }); + } } ObjectSetPrototypeOf(ChildProcess.prototype, EventEmitter.prototype); ObjectSetPrototypeOf(ChildProcess, EventEmitter); diff --git a/lib/internal/worker.js b/lib/internal/worker.js index 8e396195209b83..75287e81446779 100644 --- a/lib/internal/worker.js +++ b/lib/internal/worker.js @@ -87,6 +87,9 @@ let debug = require('internal/util/debuglog').debuglog('worker', (fn) => { debug = fn; }); +const dc = require('diagnostics_channel'); +const workerThreadsChannel = dc.channel('worker_threads'); + let cwdCounter; const environmentData = new SafeMap(); @@ -260,6 +263,11 @@ class Worker extends EventEmitter { this[kHandle].startThread(); process.nextTick(() => process.emit('worker', this)); + if (workerThreadsChannel.hasSubscribers) { + workerThreadsChannel.publish({ + worker: this, + }); + } } [kOnExit](code, customErr, customErrReason) { diff --git a/test/parallel/test-diagnostics-channel-process.js b/test/parallel/test-diagnostics-channel-process.js new file mode 100644 index 00000000000000..3ca6e2cd4f243a --- /dev/null +++ b/test/parallel/test-diagnostics-channel-process.js @@ -0,0 +1,21 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const cluster = require('cluster'); +const { ChildProcess } = require('child_process'); +const dc = require('diagnostics_channel'); + +if (cluster.isPrimary) { + dc.subscribe('child_process', common.mustCall(({ process }) => { + assert.strictEqual(process instanceof ChildProcess, true); + })); + const worker = cluster.fork(); + worker.on('online', common.mustCall(() => { + worker.send('disconnect'); + })); +} else { + process.on('message', common.mustCall((msg) => { + assert.strictEqual(msg, 'disconnect'); + process.disconnect(); + })); +} diff --git a/test/parallel/test-diagnostics-channel-worker-threads.js b/test/parallel/test-diagnostics-channel-worker-threads.js new file mode 100644 index 00000000000000..786b77da1709d3 --- /dev/null +++ b/test/parallel/test-diagnostics-channel-worker-threads.js @@ -0,0 +1,11 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const { Worker } = require('worker_threads'); +const dc = require('diagnostics_channel'); + +dc.subscribe('worker_threads', common.mustCall(({ worker }) => { + assert.strictEqual(worker instanceof Worker, true); +})); + +new Worker('const a = 1;', { eval: true }); From 4b96a0cc1a3696458607fc7e27893549879e2763 Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Tue, 23 Aug 2022 20:50:21 +0300 Subject: [PATCH 3/4] cli: add `--watch` PR-URL: https://github.com/nodejs/node/pull/44366 Fixes: https://github.com/nodejs/node/issues/40429 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Antoine du Hamel --- doc/api/cli.md | 49 +++ lib/internal/assert/assertion_error.js | 49 +-- lib/internal/main/watch_mode.js | 132 ++++++++ lib/internal/modules/cjs/loader.js | 10 + lib/internal/modules/esm/loader.js | 4 + lib/internal/util/colors.js | 23 ++ lib/internal/watch_mode/files_watcher.js | 133 ++++++++ node.gypi | 6 + src/env-inl.h | 3 +- src/inspector_agent.cc | 3 + src/node.cc | 4 + src/node_options.cc | 37 ++- src/node_options.h | 5 + test/common/inspector-helper.js | 28 +- test/fixtures/watch-mode/dependant.js | 2 + test/fixtures/watch-mode/dependant.mjs | 2 + test/fixtures/watch-mode/dependency.js | 1 + test/fixtures/watch-mode/dependency.mjs | 1 + test/fixtures/watch-mode/failing.js | 1 + test/fixtures/watch-mode/graceful-sigterm.js | 17 + test/fixtures/watch-mode/infinite-loop.js | 2 + test/fixtures/watch-mode/inspect.js | 2 + .../watch-mode/inspect_with_signal.js | 2 + test/fixtures/watch-mode/ipc.js | 12 + test/fixtures/watch-mode/parse_args.js | 4 + test/fixtures/watch-mode/process_exit.js | 1 + test/fixtures/watch-mode/subdir/file.js | 1 + .../test-watch-mode-files_watcher.mjs | 162 ++++++++++ test/parallel/test-watch-mode.mjs | 300 ++++++++++++++++++ 29 files changed, 954 insertions(+), 42 deletions(-) create mode 100644 lib/internal/main/watch_mode.js create mode 100644 lib/internal/util/colors.js create mode 100644 lib/internal/watch_mode/files_watcher.js create mode 100644 test/fixtures/watch-mode/dependant.js create mode 100644 test/fixtures/watch-mode/dependant.mjs create mode 100644 test/fixtures/watch-mode/dependency.js create mode 100644 test/fixtures/watch-mode/dependency.mjs create mode 100644 test/fixtures/watch-mode/failing.js create mode 100644 test/fixtures/watch-mode/graceful-sigterm.js create mode 100644 test/fixtures/watch-mode/infinite-loop.js create mode 100644 test/fixtures/watch-mode/inspect.js create mode 100644 test/fixtures/watch-mode/inspect_with_signal.js create mode 100644 test/fixtures/watch-mode/ipc.js create mode 100644 test/fixtures/watch-mode/parse_args.js create mode 100644 test/fixtures/watch-mode/process_exit.js create mode 100644 test/fixtures/watch-mode/subdir/file.js create mode 100644 test/parallel/test-watch-mode-files_watcher.mjs create mode 100644 test/parallel/test-watch-mode.mjs diff --git a/doc/api/cli.md b/doc/api/cli.md index d252aba1612106..ff145e000343c9 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -1529,6 +1529,53 @@ on the number of online processors. If the value provided is larger than V8's maximum, then the largest value will be chosen. +### `--watch` + + + +> Stability: 1 - Experimental + +Starts Node.js in watch mode. +When in watch mode, changes in the watched files cause the Node.js process to +restart. +By default, watch mode will watch the entry point +and any required or imported module. +Use `--watch-path` to specify what paths to watch. + +This flag cannot be combined with +`--check`, `--eval`, `--interactive`, or the REPL. + +```console +$ node --watch index.js +``` + +### `--watch-path` + + + +> Stability: 1 - Experimental + +Starts Node.js in watch mode and specifies what paths to watch. +When in watch mode, changes in the watched paths cause the Node.js process to +restart. +This will turn off watching of required or imported modules, even when used in +combination with `--watch`. + +This flag cannot be combined with +`--check`, `--eval`, `--interactive`, or the REPL. + +```console +$ node --watch-path=./src --watch-path=./tests index.js +``` + +This option is only supported on macOS and Windows. +An `ERR_FEATURE_UNAVAILABLE_ON_PLATFORM` exception will be thrown +when the option is used on a platform that does not support it. + ### `--zero-fill-buffers` diff --git a/lib/internal/assert/assertion_error.js b/lib/internal/assert/assertion_error.js index 3deb8185229d7f..17e261bd4cb1ef 100644 --- a/lib/internal/assert/assertion_error.js +++ b/lib/internal/assert/assertion_error.js @@ -21,15 +21,12 @@ const { inspect } = require('internal/util/inspect'); const { removeColors, } = require('internal/util'); +const colors = require('internal/util/colors'); const { validateObject, } = require('internal/validators'); const { isErrorStackTraceLimitWritable } = require('internal/errors'); -let blue = ''; -let green = ''; -let red = ''; -let white = ''; const kReadableOperator = { deepStrictEqual: 'Expected values to be strictly deep-equal:', @@ -169,7 +166,7 @@ function createErrDiff(actual, expected, operator) { // Only remove lines in case it makes sense to collapse those. // TODO: Accept env to always show the full error. if (actualLines.length > 50) { - actualLines[46] = `${blue}...${white}`; + actualLines[46] = `${colors.blue}...${colors.white}`; while (actualLines.length > 47) { ArrayPrototypePop(actualLines); } @@ -182,7 +179,7 @@ function createErrDiff(actual, expected, operator) { // There were at least five identical lines at the end. Mark a couple of // skipped. if (i >= 5) { - end = `\n${blue}...${white}${end}`; + end = `\n${colors.blue}...${colors.white}${end}`; skipped = true; } if (other !== '') { @@ -193,15 +190,15 @@ function createErrDiff(actual, expected, operator) { let printedLines = 0; let identical = 0; const msg = kReadableOperator[operator] + - `\n${green}+ actual${white} ${red}- expected${white}`; - const skippedMsg = ` ${blue}...${white} Lines skipped`; + `\n${colors.green}+ actual${colors.white} ${colors.red}- expected${colors.white}`; + const skippedMsg = ` ${colors.blue}...${colors.white} Lines skipped`; let lines = actualLines; - let plusMinus = `${green}+${white}`; + let plusMinus = `${colors.green}+${colors.white}`; let maxLength = expectedLines.length; if (actualLines.length < maxLines) { lines = expectedLines; - plusMinus = `${red}-${white}`; + plusMinus = `${colors.red}-${colors.white}`; maxLength = actualLines.length; } @@ -216,7 +213,7 @@ function createErrDiff(actual, expected, operator) { res += `\n ${lines[i - 3]}`; printedLines++; } else { - res += `\n${blue}...${white}`; + res += `\n${colors.blue}...${colors.white}`; skipped = true; } } @@ -272,7 +269,7 @@ function createErrDiff(actual, expected, operator) { res += `\n ${actualLines[i - 3]}`; printedLines++; } else { - res += `\n${blue}...${white}`; + res += `\n${colors.blue}...${colors.white}`; skipped = true; } } @@ -286,8 +283,8 @@ function createErrDiff(actual, expected, operator) { identical = 0; // Add the actual line to the result and cache the expected diverging // line so consecutive diverging lines show up as +++--- and not +-+-+-. - res += `\n${green}+${white} ${actualLine}`; - other += `\n${red}-${white} ${expectedLine}`; + res += `\n${colors.green}+${colors.white} ${actualLine}`; + other += `\n${colors.red}-${colors.white} ${expectedLine}`; printedLines += 2; // Lines are identical } else { @@ -306,8 +303,8 @@ function createErrDiff(actual, expected, operator) { } // Inspected object to big (Show ~50 rows max) if (printedLines > 50 && i < maxLines - 2) { - return `${msg}${skippedMsg}\n${res}\n${blue}...${white}${other}\n` + - `${blue}...${white}`; + return `${msg}${skippedMsg}\n${res}\n${colors.blue}...${colors.white}${other}\n` + + `${colors.blue}...${colors.white}`; } } @@ -347,21 +344,9 @@ class AssertionError extends Error { if (message != null) { super(String(message)); } else { - if (process.stderr.isTTY) { - // Reset on each call to make sure we handle dynamically set environment - // variables correct. - if (process.stderr.hasColors()) { - blue = '\u001b[34m'; - green = '\u001b[32m'; - white = '\u001b[39m'; - red = '\u001b[31m'; - } else { - blue = ''; - green = ''; - white = ''; - red = ''; - } - } + // Reset colors on each call to make sure we handle dynamically set environment + // variables correct. + colors.refresh(); // Prevent the error stack from being visible by duplicating the error // in a very close way to the original in case both sides are actually // instances of Error. @@ -393,7 +378,7 @@ class AssertionError extends Error { // Only remove lines in case it makes sense to collapse those. // TODO: Accept env to always show the full error. if (res.length > 50) { - res[46] = `${blue}...${white}`; + res[46] = `${colors.blue}...${colors.white}`; while (res.length > 47) { ArrayPrototypePop(res); } diff --git a/lib/internal/main/watch_mode.js b/lib/internal/main/watch_mode.js new file mode 100644 index 00000000000000..93aa42a1e7b95a --- /dev/null +++ b/lib/internal/main/watch_mode.js @@ -0,0 +1,132 @@ +'use strict'; +const { + ArrayPrototypeFilter, + ArrayPrototypeForEach, + ArrayPrototypeJoin, + ArrayPrototypeMap, + ArrayPrototypePushApply, + ArrayPrototypeSlice, +} = primordials; + +const { + prepareMainThreadExecution, + markBootstrapComplete +} = require('internal/process/pre_execution'); +const { triggerUncaughtException } = internalBinding('errors'); +const { getOptionValue } = require('internal/options'); +const { emitExperimentalWarning } = require('internal/util'); +const { FilesWatcher } = require('internal/watch_mode/files_watcher'); +const { green, blue, red, white, clear } = require('internal/util/colors'); + +const { spawn } = require('child_process'); +const { inspect } = require('util'); +const { setTimeout, clearTimeout } = require('timers'); +const { resolve } = require('path'); +const { once, on } = require('events'); + + +prepareMainThreadExecution(false, false); +markBootstrapComplete(); + +// TODO(MoLow): Make kill signal configurable +const kKillSignal = 'SIGTERM'; +const kShouldFilterModules = getOptionValue('--watch-path').length === 0; +const kWatchedPaths = ArrayPrototypeMap(getOptionValue('--watch-path'), (path) => resolve(path)); +const kCommand = ArrayPrototypeSlice(process.argv, 1); +const kCommandStr = inspect(ArrayPrototypeJoin(kCommand, ' ')); +const args = ArrayPrototypeFilter(process.execArgv, (arg, i, arr) => + arg !== '--watch-path' && arr[i - 1] !== '--watch-path' && arg !== '--watch'); +ArrayPrototypePushApply(args, kCommand); + +const watcher = new FilesWatcher({ throttle: 500, mode: kShouldFilterModules ? 'filter' : 'all' }); +ArrayPrototypeForEach(kWatchedPaths, (p) => watcher.watchPath(p)); + +let graceTimer; +let child; +let exited; + +function start() { + exited = false; + const stdio = kShouldFilterModules ? ['inherit', 'inherit', 'inherit', 'ipc'] : undefined; + child = spawn(process.execPath, args, { stdio, env: { ...process.env, WATCH_REPORT_DEPENDENCIES: '1' } }); + watcher.watchChildProcessModules(child); + child.once('exit', (code) => { + exited = true; + if (code === 0) { + process.stdout.write(`${blue}Completed running ${kCommandStr}${white}\n`); + } else { + process.stdout.write(`${red}Failed running ${kCommandStr}${white}\n`); + } + }); +} + +async function killAndWait(signal = kKillSignal, force = false) { + child?.removeAllListeners(); + if (!child) { + return; + } + if ((child.killed || exited) && !force) { + return; + } + const onExit = once(child, 'exit'); + child.kill(signal); + const { 0: exitCode } = await onExit; + return exitCode; +} + +function reportGracefulTermination() { + // Log if process takes more than 500ms to stop. + let reported = false; + clearTimeout(graceTimer); + graceTimer = setTimeout(() => { + reported = true; + process.stdout.write(`${blue}Waiting for graceful termination...${white}\n`); + }, 500).unref(); + return () => { + clearTimeout(graceTimer); + if (reported) { + process.stdout.write(`${clear}${green}Gracefully restarted ${kCommandStr}${white}\n`); + } + }; +} + +async function stop() { + watcher.clearFileFilters(); + const clearGraceReport = reportGracefulTermination(); + await killAndWait(); + clearGraceReport(); +} + +async function restart() { + process.stdout.write(`${clear}${green}Restarting ${kCommandStr}${white}\n`); + await stop(); + start(); +} + +(async () => { + emitExperimentalWarning('Watch mode'); + + try { + start(); + + // eslint-disable-next-line no-unused-vars + for await (const _ of on(watcher, 'changed')) { + await restart(); + } + } catch (error) { + triggerUncaughtException(error, true /* fromPromise */); + } +})(); + +// Exiting gracefully to avoid stdout/stderr getting written after +// parent process is killed. +// this is fairly safe since user code cannot run in this process +function signalHandler(signal) { + return async () => { + watcher.clear(); + const exitCode = await killAndWait(signal, true); + process.exit(exitCode ?? 0); + }; +} +process.on('SIGTERM', signalHandler('SIGTERM')); +process.on('SIGINT', signalHandler('SIGINT')); diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 796c527b011139..102bf90953b65b 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -100,6 +100,7 @@ const { const { getOptionValue } = require('internal/options'); const preserveSymlinks = getOptionValue('--preserve-symlinks'); const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main'); +const shouldReportRequiredModules = process.env.WATCH_REPORT_DEPENDENCIES; // Do not eagerly grab .manifest, it may be in TDZ const policy = getOptionValue('--experimental-policy') ? require('internal/process/policy') : @@ -168,6 +169,12 @@ function updateChildren(parent, child, scan) { ArrayPrototypePush(children, child); } +function reportModuleToWatchMode(filename) { + if (shouldReportRequiredModules && process.send) { + process.send({ 'watch:require': filename }); + } +} + const moduleParentCache = new SafeWeakMap(); function Module(id = '', parent) { this.id = id; @@ -776,6 +783,7 @@ Module._load = function(request, parent, isMain) { // cache key names. relResolveCacheIdentifier = `${parent.path}\x00${request}`; const filename = relativeResolveCache[relResolveCacheIdentifier]; + reportModuleToWatchMode(filename); if (filename !== undefined) { const cachedModule = Module._cache[filename]; if (cachedModule !== undefined) { @@ -828,6 +836,8 @@ Module._load = function(request, parent, isMain) { module.id = '.'; } + reportModuleToWatchMode(filename); + Module._cache[filename] = module; if (parent !== undefined) { relativeResolveCache[relResolveCacheIdentifier] = filename; diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index cc7f64a9980d86..8f1d3b8bf8ba8f 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -474,6 +474,10 @@ class ESMLoader { getOptionValue('--inspect-brk') ); + if (process.env.WATCH_REPORT_DEPENDENCIES && process.send) { + process.send({ 'watch:import': url }); + } + const job = new ModuleJob( this, url, diff --git a/lib/internal/util/colors.js b/lib/internal/util/colors.js new file mode 100644 index 00000000000000..5622a88467d038 --- /dev/null +++ b/lib/internal/util/colors.js @@ -0,0 +1,23 @@ +'use strict'; + +module.exports = { + blue: '', + green: '', + white: '', + red: '', + clear: '', + hasColors: false, + refresh() { + if (process.stderr.isTTY) { + const hasColors = process.stderr.hasColors(); + module.exports.blue = hasColors ? '\u001b[34m' : ''; + module.exports.green = hasColors ? '\u001b[32m' : ''; + module.exports.white = hasColors ? '\u001b[39m' : ''; + module.exports.red = hasColors ? '\u001b[31m' : ''; + module.exports.clear = hasColors ? '\u001bc' : ''; + module.exports.hasColors = hasColors; + } + } +}; + +module.exports.refresh(); diff --git a/lib/internal/watch_mode/files_watcher.js b/lib/internal/watch_mode/files_watcher.js new file mode 100644 index 00000000000000..6c6c0f27fd8f8d --- /dev/null +++ b/lib/internal/watch_mode/files_watcher.js @@ -0,0 +1,133 @@ +'use strict'; + +const { + SafeMap, + SafeSet, + StringPrototypeStartsWith, +} = primordials; + +const { validateNumber, validateOneOf } = require('internal/validators'); +const { kEmptyObject } = require('internal/util'); +const { TIMEOUT_MAX } = require('internal/timers'); + +const EventEmitter = require('events'); +const { watch } = require('fs'); +const { fileURLToPath } = require('url'); +const { resolve, dirname } = require('path'); +const { setTimeout } = require('timers'); + + +const supportsRecursiveWatching = process.platform === 'win32' || + process.platform === 'darwin'; + +class FilesWatcher extends EventEmitter { + #watchers = new SafeMap(); + #filteredFiles = new SafeSet(); + #throttling = new SafeSet(); + #throttle; + #mode; + + constructor({ throttle = 500, mode = 'filter' } = kEmptyObject) { + super(); + + validateNumber(throttle, 'options.throttle', 0, TIMEOUT_MAX); + validateOneOf(mode, 'options.mode', ['filter', 'all']); + this.#throttle = throttle; + this.#mode = mode; + } + + #isPathWatched(path) { + if (this.#watchers.has(path)) { + return true; + } + + for (const { 0: watchedPath, 1: watcher } of this.#watchers.entries()) { + if (watcher.recursive && StringPrototypeStartsWith(path, watchedPath)) { + return true; + } + } + + return false; + } + + #removeWatchedChildren(path) { + for (const { 0: watchedPath, 1: watcher } of this.#watchers.entries()) { + if (path !== watchedPath && StringPrototypeStartsWith(watchedPath, path)) { + this.#unwatch(watcher); + this.#watchers.delete(watchedPath); + } + } + } + + #unwatch(watcher) { + watcher.handle.removeAllListeners(); + watcher.handle.close(); + } + + #onChange(trigger) { + if (this.#throttling.has(trigger)) { + return; + } + if (this.#mode === 'filter' && !this.#filteredFiles.has(trigger)) { + return; + } + this.#throttling.add(trigger); + this.emit('changed'); + setTimeout(() => this.#throttling.delete(trigger), this.#throttle).unref(); + } + + get watchedPaths() { + return [...this.#watchers.keys()]; + } + + watchPath(path, recursive = true) { + if (this.#isPathWatched(path)) { + return; + } + const watcher = watch(path, { recursive }); + watcher.on('change', (eventType, fileName) => this + .#onChange(recursive ? resolve(path, fileName) : path)); + this.#watchers.set(path, { handle: watcher, recursive }); + if (recursive) { + this.#removeWatchedChildren(path); + } + } + + filterFile(file) { + if (supportsRecursiveWatching) { + this.watchPath(dirname(file)); + } else { + // Having multiple FSWatcher's seems to be slower + // than a single recursive FSWatcher + this.watchPath(file, false); + } + this.#filteredFiles.add(file); + } + watchChildProcessModules(child) { + if (this.#mode !== 'filter') { + return; + } + child.on('message', (message) => { + try { + if (message['watch:require']) { + this.filterFile(message['watch:require']); + } + if (message['watch:import']) { + this.filterFile(fileURLToPath(message['watch:import'])); + } + } catch { + // Failed watching file. ignore + } + }); + } + clearFileFilters() { + this.#filteredFiles.clear(); + } + clear() { + this.#watchers.forEach(this.#unwatch); + this.#watchers.clear(); + this.#filteredFiles.clear(); + } +} + +module.exports = { FilesWatcher }; diff --git a/node.gypi b/node.gypi index d24928df8b29c9..b73ec5f35b76d0 100644 --- a/node.gypi +++ b/node.gypi @@ -317,6 +317,12 @@ }], ], }], + [ 'coverage=="true"', { + 'defines': [ + 'ALLOW_ATTACHING_DEBUGGER_IN_WATCH_MODE', + 'ALLOW_ATTACHING_DEBUGGER_IN_TEST_RUNNER', + ], + }], [ 'OS=="sunos"', { 'ldflags': [ '-Wl,-M,/usr/lib/ld/map.noexstk' ], }], diff --git a/src/env-inl.h b/src/env-inl.h index 59dee7761d4ea5..abd69b02e0dffc 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -655,7 +655,8 @@ inline bool Environment::owns_inspector() const { } inline bool Environment::should_create_inspector() const { - return (flags_ & EnvironmentFlags::kNoCreateInspector) == 0; + return (flags_ & EnvironmentFlags::kNoCreateInspector) == 0 && + !options_->test_runner && !options_->watch_mode; } inline bool Environment::tracks_unmanaged_fds() const { diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc index 34bb11e7d7122c..9ee779fb597b42 100644 --- a/src/inspector_agent.cc +++ b/src/inspector_agent.cc @@ -676,6 +676,9 @@ bool Agent::Start(const std::string& path, const DebugOptions& options, std::shared_ptr> host_port, bool is_main) { + if (!options.allow_attaching_debugger) { + return false; + } path_ = path; debug_options_ = options; CHECK_NOT_NULL(host_port); diff --git a/src/node.cc b/src/node.cc index 71b3f7dcfe3cdd..f75e2ffa754440 100644 --- a/src/node.cc +++ b/src/node.cc @@ -498,6 +498,10 @@ MaybeLocal StartExecution(Environment* env, StartExecutionCallback cb) { return StartExecution(env, "internal/main/test_runner"); } + if (env->options()->watch_mode && !first_argv.empty()) { + return StartExecution(env, "internal/main/watch_mode"); + } + if (!first_argv.empty() && first_argv != "-") { return StartExecution(env, "internal/main/run_main_module"); } diff --git a/src/node_options.cc b/src/node_options.cc index bd4ad2f6408ca8..d08184f3da5001 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -156,9 +156,36 @@ void EnvironmentOptions::CheckOptions(std::vector* errors) { errors->push_back("either --test or --interactive can be used, not both"); } + if (watch_mode) { + // TODO(MoLow): Support (incremental?) watch mode within test runner + errors->push_back("either --test or --watch can be used, not both"); + } + if (debug_options_.inspector_enabled) { errors->push_back("the inspector cannot be used with --test"); } +#ifndef ALLOW_ATTACHING_DEBUGGER_IN_TEST_RUNNER + debug_options_.allow_attaching_debugger = false; +#endif + } + + if (watch_mode) { + if (syntax_check_only) { + errors->push_back("either --watch or --check can be used, not both"); + } + + if (has_eval_string) { + errors->push_back("either --watch or --eval can be used, not both"); + } + + if (force_repl) { + errors->push_back("either --watch or --interactive " + "can be used, not both"); + } + +#ifndef ALLOW_ATTACHING_DEBUGGER_IN_WATCH_MODE + debug_options_.allow_attaching_debugger = false; +#endif } #if HAVE_INSPECTOR @@ -586,7 +613,15 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() { "", /* undocumented, only for debugging */ &EnvironmentOptions::verify_base_objects, kAllowedInEnvironment); - + AddOption("--watch", + "run in watch mode", + &EnvironmentOptions::watch_mode, + kAllowedInEnvironment); + AddOption("--watch-path", + "path to watch", + &EnvironmentOptions::watch_mode_paths, + kAllowedInEnvironment); + Implies("--watch-path", "--watch"); AddOption("--check", "syntax check script without executing", &EnvironmentOptions::syntax_check_only); diff --git a/src/node_options.h b/src/node_options.h index b20cfae141956a..2f1142429783ce 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -71,6 +71,7 @@ class DebugOptions : public Options { DebugOptions(DebugOptions&&) = default; DebugOptions& operator=(DebugOptions&&) = default; + bool allow_attaching_debugger = true; // --inspect bool inspector_enabled = false; // --debug @@ -172,6 +173,10 @@ class EnvironmentOptions : public Options { false; #endif // DEBUG + bool watch_mode = false; + bool watch_mode_report_to_parent = false; + std::vector watch_mode_paths; + bool syntax_check_only = false; bool has_eval_string = false; bool experimental_wasi = false; diff --git a/test/common/inspector-helper.js b/test/common/inspector-helper.js index cebc048362ef40..ed3a6c90989664 100644 --- a/test/common/inspector-helper.js +++ b/test/common/inspector-helper.js @@ -151,6 +151,7 @@ class InspectorSession { }); } + waitForServerDisconnect() { return this._terminationPromise; } @@ -326,13 +327,15 @@ class InspectorSession { class NodeInstance extends EventEmitter { constructor(inspectorFlags = ['--inspect-brk=0', '--expose-internals'], scriptContents = '', - scriptFile = _MAINSCRIPT) { + scriptFile = _MAINSCRIPT, + logger = console) { super(); + this._logger = logger; this._scriptPath = scriptFile; this._script = scriptFile ? null : scriptContents; this._portCallback = null; - this.portPromise = new Promise((resolve) => this._portCallback = resolve); + this.resetPort(); this._process = spawnChildProcess(inspectorFlags, scriptContents, scriptFile); this._running = true; @@ -342,7 +345,7 @@ class NodeInstance extends EventEmitter { this._process.stdout.on('data', makeBufferingDataCallback( (line) => { this.emit('stdout', line); - console.log('[out]', line); + this._logger.log('[out]', line); })); this._process.stderr.on('data', makeBufferingDataCallback( @@ -351,7 +354,7 @@ class NodeInstance extends EventEmitter { this._shutdownPromise = new Promise((resolve) => { this._process.once('exit', (exitCode, signal) => { if (signal) { - console.error(`[err] child process crashed, signal ${signal}`); + this._logger.error(`[err] child process crashed, signal ${signal}`); } resolve({ exitCode, signal }); this._running = false; @@ -359,6 +362,14 @@ class NodeInstance extends EventEmitter { }); } + get pid() { + return this._process.pid; + } + + resetPort() { + this.portPromise = new Promise((resolve) => this._portCallback = resolve); + } + static async startViaSignal(scriptContents) { const instance = new NodeInstance( ['--expose-internals'], @@ -370,7 +381,8 @@ class NodeInstance extends EventEmitter { } onStderrLine(line) { - console.log('[err]', line); + this.emit('stderr', line); + this._logger.log('[err]', line); if (this._portCallback) { const matches = line.match(/Debugger listening on ws:\/\/.+:(\d+)\/.+/); if (matches) { @@ -387,7 +399,7 @@ class NodeInstance extends EventEmitter { } httpGet(host, path, hostHeaderValue) { - console.log('[test]', `Testing ${path}`); + this._logger.log('[test]', `Testing ${path}`); const headers = hostHeaderValue ? { 'Host': hostHeaderValue } : null; return this.portPromise.then((port) => new Promise((resolve, reject) => { const req = http.get({ host, port, family: 4, path, headers }, (res) => { @@ -428,7 +440,7 @@ class NodeInstance extends EventEmitter { } async connectInspectorSession() { - console.log('[test]', 'Connecting to a child Node process'); + this._logger.log('[test]', 'Connecting to a child Node process'); const upgradeRequest = await this.sendUpgradeRequest(); return new Promise((resolve) => { upgradeRequest @@ -439,7 +451,7 @@ class NodeInstance extends EventEmitter { } async expectConnectionDeclined() { - console.log('[test]', 'Checking upgrade is not possible'); + this._logger.log('[test]', 'Checking upgrade is not possible'); const upgradeRequest = await this.sendUpgradeRequest(); return new Promise((resolve) => { upgradeRequest diff --git a/test/fixtures/watch-mode/dependant.js b/test/fixtures/watch-mode/dependant.js new file mode 100644 index 00000000000000..25a0ec095d7cf2 --- /dev/null +++ b/test/fixtures/watch-mode/dependant.js @@ -0,0 +1,2 @@ +const dependency = require('./dependency'); +console.log(dependency); diff --git a/test/fixtures/watch-mode/dependant.mjs b/test/fixtures/watch-mode/dependant.mjs new file mode 100644 index 00000000000000..dff99bb2714728 --- /dev/null +++ b/test/fixtures/watch-mode/dependant.mjs @@ -0,0 +1,2 @@ +import dependency from './dependency.mjs'; +console.log(dependency); diff --git a/test/fixtures/watch-mode/dependency.js b/test/fixtures/watch-mode/dependency.js new file mode 100644 index 00000000000000..f053ebf7976e37 --- /dev/null +++ b/test/fixtures/watch-mode/dependency.js @@ -0,0 +1 @@ +module.exports = {}; diff --git a/test/fixtures/watch-mode/dependency.mjs b/test/fixtures/watch-mode/dependency.mjs new file mode 100644 index 00000000000000..ff8b4c56321a33 --- /dev/null +++ b/test/fixtures/watch-mode/dependency.mjs @@ -0,0 +1 @@ +export default {}; diff --git a/test/fixtures/watch-mode/failing.js b/test/fixtures/watch-mode/failing.js new file mode 100644 index 00000000000000..d1e87944d9f33c --- /dev/null +++ b/test/fixtures/watch-mode/failing.js @@ -0,0 +1 @@ +throw new Error('fails'); diff --git a/test/fixtures/watch-mode/graceful-sigterm.js b/test/fixtures/watch-mode/graceful-sigterm.js new file mode 100644 index 00000000000000..d099b47b76f730 --- /dev/null +++ b/test/fixtures/watch-mode/graceful-sigterm.js @@ -0,0 +1,17 @@ + +setInterval(() => {}, 1000); +console.log('running'); + +process.on('SIGTERM', () => { + setTimeout(() => { + console.log('exiting gracefully'); + process.exit(0); + }, 1000); +}); + +process.on('SIGINT', () => { + setTimeout(() => { + console.log('exiting gracefully'); + process.exit(0); + }, 1000); +}); diff --git a/test/fixtures/watch-mode/infinite-loop.js b/test/fixtures/watch-mode/infinite-loop.js new file mode 100644 index 00000000000000..56e92666e7cb1c --- /dev/null +++ b/test/fixtures/watch-mode/infinite-loop.js @@ -0,0 +1,2 @@ +console.log('running'); +while(true) {}; diff --git a/test/fixtures/watch-mode/inspect.js b/test/fixtures/watch-mode/inspect.js new file mode 100644 index 00000000000000..f836b77e8a85e1 --- /dev/null +++ b/test/fixtures/watch-mode/inspect.js @@ -0,0 +1,2 @@ +console.log('safe to debug now'); +setInterval(() => {}, 1000); diff --git a/test/fixtures/watch-mode/inspect_with_signal.js b/test/fixtures/watch-mode/inspect_with_signal.js new file mode 100644 index 00000000000000..6abf3ab2b5888a --- /dev/null +++ b/test/fixtures/watch-mode/inspect_with_signal.js @@ -0,0 +1,2 @@ +console.log('pid is', process.pid); +setInterval(() => {}, 1000); diff --git a/test/fixtures/watch-mode/ipc.js b/test/fixtures/watch-mode/ipc.js new file mode 100644 index 00000000000000..31e1bd0e5c589f --- /dev/null +++ b/test/fixtures/watch-mode/ipc.js @@ -0,0 +1,12 @@ +const path = require('node:path'); +const url = require('node:url'); +const os = require('node:os'); +const fs = require('node:fs'); + +const tmpfile = path.join(os.tmpdir(), 'file'); +fs.writeFileSync(tmpfile, ''); + +process.send({ 'watch:require': path.resolve(__filename) }); +process.send({ 'watch:import': url.pathToFileURL(path.resolve(__filename)).toString() }); +process.send({ 'watch:import': url.pathToFileURL(tmpfile).toString() }); +process.send({ 'watch:import': new URL('http://invalid.com').toString() }); diff --git a/test/fixtures/watch-mode/parse_args.js b/test/fixtures/watch-mode/parse_args.js new file mode 100644 index 00000000000000..06c7227cee5933 --- /dev/null +++ b/test/fixtures/watch-mode/parse_args.js @@ -0,0 +1,4 @@ +const { parseArgs } = require('node:util'); + +const { values } = parseArgs({ options: { random: { type: 'string' } } }); +console.log(values.random); diff --git a/test/fixtures/watch-mode/process_exit.js b/test/fixtures/watch-mode/process_exit.js new file mode 100644 index 00000000000000..cbe6cdd84cc073 --- /dev/null +++ b/test/fixtures/watch-mode/process_exit.js @@ -0,0 +1 @@ +setImmediate(() => process.exit(0)); diff --git a/test/fixtures/watch-mode/subdir/file.js b/test/fixtures/watch-mode/subdir/file.js new file mode 100644 index 00000000000000..8b137891791fe9 --- /dev/null +++ b/test/fixtures/watch-mode/subdir/file.js @@ -0,0 +1 @@ + diff --git a/test/parallel/test-watch-mode-files_watcher.mjs b/test/parallel/test-watch-mode-files_watcher.mjs new file mode 100644 index 00000000000000..1c3088800bd5d9 --- /dev/null +++ b/test/parallel/test-watch-mode-files_watcher.mjs @@ -0,0 +1,162 @@ +// Flags: --expose-internals +import * as common from '../common/index.mjs'; +import * as fixtures from '../common/fixtures.mjs'; +import tmpdir from '../common/tmpdir.js'; +import path from 'node:path'; +import assert from 'node:assert'; +import process from 'node:process'; +import os from 'node:os'; +import { describe, it, beforeEach, afterEach } from 'node:test'; +import { writeFileSync, mkdirSync } from 'node:fs'; +import { setTimeout } from 'node:timers/promises'; +import { once } from 'node:events'; +import { spawn } from 'node:child_process'; +import watcher from 'internal/watch_mode/files_watcher'; + +if (common.isIBMi) + common.skip('IBMi does not support `fs.watch()`'); + +const supportsRecursiveWatching = common.isOSX || common.isWindows; + +const { FilesWatcher } = watcher; +tmpdir.refresh(); + +describe('watch mode file watcher', () => { + let watcher; + let changesCount; + + beforeEach(() => { + changesCount = 0; + watcher = new FilesWatcher({ throttle: 100 }); + watcher.on('changed', () => changesCount++); + }); + + afterEach(() => watcher.clear()); + + let counter = 0; + function writeAndWaitForChanges(watcher, file) { + return new Promise((resolve) => { + const interval = setInterval(() => writeFileSync(file, `write ${counter++}`), 100); + watcher.once('changed', () => { + clearInterval(interval); + resolve(); + }); + }); + } + + it('should watch changed files', async () => { + const file = path.join(tmpdir.path, 'file1'); + writeFileSync(file, 'written'); + watcher.filterFile(file); + await writeAndWaitForChanges(watcher, file); + assert.strictEqual(changesCount, 1); + }); + + it('should throttle changes', async () => { + const file = path.join(tmpdir.path, 'file2'); + writeFileSync(file, 'written'); + watcher.filterFile(file); + await writeAndWaitForChanges(watcher, file); + + writeFileSync(file, '1'); + writeFileSync(file, '2'); + writeFileSync(file, '3'); + writeFileSync(file, '4'); + await setTimeout(200); // throttle * 2 + writeFileSync(file, '5'); + const changed = once(watcher, 'changed'); + writeFileSync(file, 'after'); + await changed; + // Unfortunately testing that changesCount === 2 is flaky + assert.ok(changesCount < 5); + }); + + it('should ignore files in watched directory if they are not filtered', + { skip: !supportsRecursiveWatching }, async () => { + watcher.on('changed', common.mustNotCall()); + watcher.watchPath(tmpdir.path); + writeFileSync(path.join(tmpdir.path, 'file3'), '1'); + // Wait for this long to make sure changes are not triggered + await setTimeout(1000); + }); + + it('should allow clearing filters', async () => { + const file = path.join(tmpdir.path, 'file4'); + writeFileSync(file, 'written'); + watcher.filterFile(file); + await writeAndWaitForChanges(watcher, file); + + writeFileSync(file, '1'); + + await setTimeout(200); // avoid throttling + watcher.clearFileFilters(); + writeFileSync(file, '2'); + // Wait for this long to make sure changes are triggered only once + await setTimeout(1000); + assert.strictEqual(changesCount, 1); + }); + + it('should watch all files in watched path when in "all" mode', + { skip: !supportsRecursiveWatching }, async () => { + watcher = new FilesWatcher({ throttle: 100, mode: 'all' }); + watcher.on('changed', () => changesCount++); + + const file = path.join(tmpdir.path, 'file5'); + watcher.watchPath(tmpdir.path); + + const changed = once(watcher, 'changed'); + writeFileSync(file, 'changed'); + await changed; + assert.strictEqual(changesCount, 1); + }); + + it('should ruse existing watcher if it exists', + { skip: !supportsRecursiveWatching }, () => { + assert.deepStrictEqual(watcher.watchedPaths, []); + watcher.watchPath(tmpdir.path); + assert.deepStrictEqual(watcher.watchedPaths, [tmpdir.path]); + watcher.watchPath(tmpdir.path); + assert.deepStrictEqual(watcher.watchedPaths, [tmpdir.path]); + }); + + it('should ruse existing watcher of a parent directory', + { skip: !supportsRecursiveWatching }, () => { + assert.deepStrictEqual(watcher.watchedPaths, []); + watcher.watchPath(tmpdir.path); + assert.deepStrictEqual(watcher.watchedPaths, [tmpdir.path]); + watcher.watchPath(path.join(tmpdir.path, 'subdirectory')); + assert.deepStrictEqual(watcher.watchedPaths, [tmpdir.path]); + }); + + it('should remove existing watcher if adding a parent directory watcher', + { skip: !supportsRecursiveWatching }, () => { + assert.deepStrictEqual(watcher.watchedPaths, []); + const subdirectory = path.join(tmpdir.path, 'subdirectory'); + mkdirSync(subdirectory); + watcher.watchPath(subdirectory); + assert.deepStrictEqual(watcher.watchedPaths, [subdirectory]); + watcher.watchPath(tmpdir.path); + assert.deepStrictEqual(watcher.watchedPaths, [tmpdir.path]); + }); + + it('should clear all watchers when calling clear', + { skip: !supportsRecursiveWatching }, () => { + assert.deepStrictEqual(watcher.watchedPaths, []); + watcher.watchPath(tmpdir.path); + assert.deepStrictEqual(watcher.watchedPaths, [tmpdir.path]); + watcher.clear(); + assert.deepStrictEqual(watcher.watchedPaths, []); + }); + + it('should watch files from subprocess IPC events', async () => { + const file = fixtures.path('watch-mode/ipc.js'); + const child = spawn(process.execPath, [file], { stdio: ['pipe', 'pipe', 'pipe', 'ipc'], encoding: 'utf8' }); + watcher.watchChildProcessModules(child); + await once(child, 'exit'); + let expected = [file, path.join(os.tmpdir(), 'file')]; + if (supportsRecursiveWatching) { + expected = expected.map((file) => path.dirname(file)); + } + assert.deepStrictEqual(watcher.watchedPaths, expected); + }); +}); diff --git a/test/parallel/test-watch-mode.mjs b/test/parallel/test-watch-mode.mjs new file mode 100644 index 00000000000000..83226c96ca5362 --- /dev/null +++ b/test/parallel/test-watch-mode.mjs @@ -0,0 +1,300 @@ +import * as common from '../common/index.mjs'; +import * as fixtures from '../common/fixtures.mjs'; +import tmpdir from '../common/tmpdir.js'; +import assert from 'node:assert'; +import path from 'node:path'; +import { execPath } from 'node:process'; +import { describe, it } from 'node:test'; +import { spawn } from 'node:child_process'; +import { writeFileSync, readFileSync } from 'node:fs'; +import { inspect } from 'node:util'; +import { once } from 'node:events'; +import { setTimeout } from 'node:timers/promises'; +import { NodeInstance } from '../common/inspector-helper.js'; + + +if (common.isIBMi) + common.skip('IBMi does not support `fs.watch()`'); + +async function spawnWithRestarts({ + args, + file, + restarts, + startedPredicate, + restartMethod, +}) { + args ??= [file]; + const printedArgs = inspect(args.slice(args.indexOf(file)).join(' ')); + startedPredicate ??= (data) => Boolean(data.match(new RegExp(`(Failed|Completed) running ${printedArgs.replace(/\\/g, '\\\\')}`, 'g'))?.length); + restartMethod ??= () => writeFileSync(file, readFileSync(file)); + + let stderr = ''; + let stdout = ''; + let restartCount = 0; + let completedStart = false; + let finished = false; + + const child = spawn(execPath, ['--watch', '--no-warnings', ...args], { encoding: 'utf8' }); + child.stderr.on('data', (data) => { + stderr += data; + }); + child.stdout.on('data', async (data) => { + if (finished) return; + stdout += data; + const restartMessages = stdout.match(new RegExp(`Restarting ${printedArgs.replace(/\\/g, '\\\\')}`, 'g'))?.length ?? 0; + completedStart = completedStart || startedPredicate(data.toString()); + if (restartMessages >= restarts && completedStart) { + finished = true; + child.kill(); + return; + } + if (restartCount <= restartMessages && completedStart) { + await setTimeout(restartCount > 0 ? 1000 : 50, { ref: false }); // Prevent throttling + restartCount++; + completedStart = false; + restartMethod(); + } + }); + + await Promise.race([once(child, 'exit'), once(child, 'error')]); + return { stderr, stdout }; +} + +let tmpFiles = 0; +function createTmpFile(content = 'console.log("running");') { + const file = path.join(tmpdir.path, `${tmpFiles++}.js`); + writeFileSync(file, content); + return file; +} + +function removeGraceMessage(stdout, file) { + // Remove the message in case restart took long to avoid flakiness + return stdout + .replaceAll('Waiting for graceful termination...', '') + .replaceAll(`Gracefully restarted ${inspect(file)}`, ''); +} + +tmpdir.refresh(); + +// Warning: this suite can run safely with concurrency: true +// only if tests do not watch/depend on the same files +describe('watch mode', { concurrency: false, timeout: 60_0000 }, () => { + it('should watch changes to a file - event loop ended', async () => { + const file = createTmpFile(); + const { stderr, stdout } = await spawnWithRestarts({ file, restarts: 1 }); + + assert.strictEqual(stderr, ''); + assert.strictEqual(removeGraceMessage(stdout, file), [ + 'running', `Completed running ${inspect(file)}`, `Restarting ${inspect(file)}`, + 'running', `Completed running ${inspect(file)}`, '', + ].join('\n')); + }); + + it('should watch changes to a failing file', async () => { + const file = fixtures.path('watch-mode/failing.js'); + const { stderr, stdout } = await spawnWithRestarts({ file, restarts: 1 }); + + assert.match(stderr, /Error: fails\r?\n/); + assert.strictEqual(stderr.match(/Error: fails\r?\n/g).length, 2); + assert.strictEqual(removeGraceMessage(stdout, file), [`Failed running ${inspect(file)}`, `Restarting ${inspect(file)}`, + `Failed running ${inspect(file)}`, ''].join('\n')); + }); + + it('should not watch when running an non-existing file', async () => { + const file = fixtures.path('watch-mode/non-existing.js'); + const { stderr, stdout } = await spawnWithRestarts({ file, restarts: 0, restartMethod: () => {} }); + + assert.match(stderr, /code: 'MODULE_NOT_FOUND'/); + assert.strictEqual(stdout, [`Failed running ${inspect(file)}`, ''].join('\n')); + }); + + it('should watch when running an non-existing file - when specified under --watch-path', { + skip: !common.isOSX && !common.isWindows + }, async () => { + const file = fixtures.path('watch-mode/subdir/non-existing.js'); + const watched = fixtures.path('watch-mode/subdir/file.js'); + const { stderr, stdout } = await spawnWithRestarts({ + file, + args: ['--watch-path', fixtures.path('./watch-mode/subdir/'), file], + restarts: 1, + restartMethod: () => writeFileSync(watched, readFileSync(watched)) + }); + + assert.strictEqual(stderr, ''); + assert.strictEqual(removeGraceMessage(stdout, file), [`Failed running ${inspect(file)}`, `Restarting ${inspect(file)}`, + `Failed running ${inspect(file)}`, ''].join('\n')); + }); + + it('should watch changes to a file - event loop blocked', async () => { + const file = fixtures.path('watch-mode/infinite-loop.js'); + const { stderr, stdout } = await spawnWithRestarts({ + file, + restarts: 2, + startedPredicate: (data) => data.startsWith('running'), + }); + + assert.strictEqual(stderr, ''); + assert.strictEqual(removeGraceMessage(stdout, file), + ['running', `Restarting ${inspect(file)}`, 'running', `Restarting ${inspect(file)}`, 'running', ''].join('\n')); + }); + + it('should watch changes to dependencies - cjs', async () => { + const file = fixtures.path('watch-mode/dependant.js'); + const dependency = fixtures.path('watch-mode/dependency.js'); + const { stderr, stdout } = await spawnWithRestarts({ + file, + restarts: 1, + restartMethod: () => writeFileSync(dependency, readFileSync(dependency)), + }); + + assert.strictEqual(stderr, ''); + assert.strictEqual(removeGraceMessage(stdout, file), [ + '{}', `Completed running ${inspect(file)}`, `Restarting ${inspect(file)}`, + '{}', `Completed running ${inspect(file)}`, '', + ].join('\n')); + }); + + it('should watch changes to dependencies - esm', async () => { + const file = fixtures.path('watch-mode/dependant.mjs'); + const dependency = fixtures.path('watch-mode/dependency.mjs'); + const { stderr, stdout } = await spawnWithRestarts({ + file, + restarts: 1, + restartMethod: () => writeFileSync(dependency, readFileSync(dependency)), + }); + + assert.strictEqual(stderr, ''); + assert.strictEqual(removeGraceMessage(stdout, file), [ + '{}', `Completed running ${inspect(file)}`, `Restarting ${inspect(file)}`, + '{}', `Completed running ${inspect(file)}`, '', + ].join('\n')); + }); + + it('should restart multiple times', async () => { + const file = createTmpFile(); + const { stderr, stdout } = await spawnWithRestarts({ file, restarts: 3 }); + + assert.strictEqual(stderr, ''); + assert.strictEqual(stdout.match(new RegExp(`Restarting ${inspect(file).replace(/\\/g, '\\\\')}`, 'g')).length, 3); + }); + + it('should gracefully wait when restarting', { skip: common.isWindows }, async () => { + const file = fixtures.path('watch-mode/graceful-sigterm.js'); + const { stderr, stdout } = await spawnWithRestarts({ + file, + restarts: 1, + startedPredicate: (data) => data.startsWith('running'), + }); + + // This message appearing is very flaky depending on a race between the + // inner process and the outer process. it is acceptable for the message not to appear + // as long as the SIGTERM handler is respected. + if (stdout.includes('Waiting for graceful termination...')) { + assert.strictEqual(stdout, ['running', `Restarting ${inspect(file)}`, 'Waiting for graceful termination...', + 'exiting gracefully', `Gracefully restarted ${inspect(file)}`, 'running', ''].join('\n')); + } else { + assert.strictEqual(stdout, ['running', `Restarting ${inspect(file)}`, 'exiting gracefully', 'running', ''].join('\n')); + } + assert.strictEqual(stderr, ''); + }); + + it('should pass arguments to file', async () => { + const file = fixtures.path('watch-mode/parse_args.js'); + const random = Date.now().toString(); + const args = [file, '--random', random]; + const { stderr, stdout } = await spawnWithRestarts({ file, args, restarts: 1 }); + + assert.strictEqual(stderr, ''); + assert.strictEqual(removeGraceMessage(stdout, args.join(' ')), [ + random, `Completed running ${inspect(args.join(' '))}`, `Restarting ${inspect(args.join(' '))}`, + random, `Completed running ${inspect(args.join(' '))}`, '', + ].join('\n')); + }); + + it('should not load --require modules in main process', async () => { + const file = createTmpFile(''); + const required = fixtures.path('watch-mode/process_exit.js'); + const args = ['--require', required, file]; + const { stderr, stdout } = await spawnWithRestarts({ file, args, restarts: 1 }); + + assert.strictEqual(stderr, ''); + assert.strictEqual(removeGraceMessage(stdout, file), [ + `Completed running ${inspect(file)}`, `Restarting ${inspect(file)}`, `Completed running ${inspect(file)}`, '', + ].join('\n')); + }); + + it('should not load --import modules in main process', async () => { + const file = createTmpFile(''); + const imported = fixtures.fileURL('watch-mode/process_exit.js'); + const args = ['--import', imported, file]; + const { stderr, stdout } = await spawnWithRestarts({ file, args, restarts: 1 }); + + assert.strictEqual(stderr, ''); + assert.strictEqual(removeGraceMessage(stdout, file), [ + `Completed running ${inspect(file)}`, `Restarting ${inspect(file)}`, `Completed running ${inspect(file)}`, '', + ].join('\n')); + }); + + describe('inspect', { + skip: Boolean(process.config.variables.coverage || !process.features.inspector), + }, () => { + const silentLogger = { log: () => {}, error: () => {} }; + async function getDebuggedPid(instance, waitForLog = true) { + const session = await instance.connectInspectorSession(); + await session.send({ method: 'Runtime.enable' }); + if (waitForLog) { + await session.waitForConsoleOutput('log', 'safe to debug now'); + } + const { value: innerPid } = (await session.send({ + 'method': 'Runtime.evaluate', 'params': { 'expression': 'process.pid' } + })).result; + session.disconnect(); + return innerPid; + } + + it('should start debugger on inner process', async () => { + const file = fixtures.path('watch-mode/inspect.js'); + const instance = new NodeInstance(['--inspect=0', '--watch'], undefined, file, silentLogger); + let stderr = ''; + instance.on('stderr', (data) => { stderr += data; }); + + const pids = [instance.pid]; + pids.push(await getDebuggedPid(instance)); + instance.resetPort(); + writeFileSync(file, readFileSync(file)); + pids.push(await getDebuggedPid(instance)); + + await instance.kill(); + + // There should be 3 pids (one parent + 2 restarts). + // Message about Debugger should only appear twice. + assert.strictEqual(stderr.match(/Debugger listening on ws:\/\//g).length, 2); + assert.strictEqual(new Set(pids).size, 3); + }); + + it('should prevent attaching debugger with SIGUSR1 to outer process', { skip: common.isWindows }, async () => { + const file = fixtures.path('watch-mode/inspect_with_signal.js'); + const instance = new NodeInstance(['--inspect-port=0', '--watch'], undefined, file, silentLogger); + let stderr = ''; + instance.on('stderr', (data) => { stderr += data; }); + + const loggedPid = await new Promise((resolve) => { + instance.on('stdout', (data) => { + const matches = data.match(/pid is (\d+)/); + if (matches) resolve(Number(matches[1])); + }); + }); + + + process.kill(instance.pid, 'SIGUSR1'); + process.kill(loggedPid, 'SIGUSR1'); + const debuggedPid = await getDebuggedPid(instance, false); + + await instance.kill(); + + // Message about Debugger should only appear once in inner process. + assert.strictEqual(stderr.match(/Debugger listening on ws:\/\//g).length, 1); + assert.strictEqual(loggedPid, debuggedPid); + }); + }); +}); From 39bdc8e95154857de549efa6b154ef6f601b7b84 Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Tue, 6 Sep 2022 08:10:44 +0300 Subject: [PATCH 4/4] test: skip testing `--watch` with `--import` --- test/parallel/test-watch-mode.mjs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/parallel/test-watch-mode.mjs b/test/parallel/test-watch-mode.mjs index 83226c96ca5362..76ab33507f90fa 100644 --- a/test/parallel/test-watch-mode.mjs +++ b/test/parallel/test-watch-mode.mjs @@ -223,7 +223,9 @@ describe('watch mode', { concurrency: false, timeout: 60_0000 }, () => { ].join('\n')); }); - it('should not load --import modules in main process', async () => { + it('should not load --import modules in main process', { + skip: 'enable once --import is backported', + }, async () => { const file = createTmpFile(''); const imported = fixtures.fileURL('watch-mode/process_exit.js'); const args = ['--import', imported, file];