From 9cdbf6bd6b9af38aed2c6a5e9c4c65a0d778e871 Mon Sep 17 00:00:00 2001
From: Joyee Cheung <joyeec9h3@gmail.com>
Date: Wed, 10 Aug 2022 02:01:02 +0800
Subject: [PATCH 1/7] bootstrap: check more metadata when loading the snapshot

This patch stores the metadata about the Node.js binary
into the SnapshotData and adds fields denoting how the
snapshot was generated, on what platform it was
generated as well as the V8 cached data version flag.
Instead of simply crashing when the metadata doesn't
match, Node.js now prints an error message and exit with
1 for the customized snapshot, or ignore the snapshot
and start from scratch if it's the default one.

PR-URL: https://github.com/nodejs/node/pull/44132
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
---
 doc/api/cli.md                              |   9 ++
 src/env.cc                                  |  15 +++
 src/env.h                                   |  20 ++-
 src/node.cc                                 |  14 +-
 src/node_internals.h                        |   1 +
 src/node_snapshotable.cc                    | 142 ++++++++++++++++++--
 test/parallel/test-snapshot-incompatible.js |  76 +++++++++++
 7 files changed, 260 insertions(+), 17 deletions(-)
 create mode 100644 test/parallel/test-snapshot-incompatible.js

diff --git a/doc/api/cli.md b/doc/api/cli.md
index 4f9d98a2344f0d..b14627d6fa7ce5 100644
--- a/doc/api/cli.md
+++ b/doc/api/cli.md
@@ -1177,6 +1177,15 @@ in the current working directory.
 When used without `--build-snapshot`, `--snapshot-blob` specifies the
 path to the blob that will be used to restore the application state.
 
+When loading a snapshot, Node.js checks that:
+
+1. The version, architecture and platform of the running Node.js binary
+   are exactly the same as that of the binary that generates the snapshot.
+2. The V8 flags and CPU features are compatible with that of the binary
+   that generates the snapshot.
+
+If they don't match, Node.js would refuse to load the snapshot and exit with 1.
+
 ### `--test`
 
 <!-- YAML
diff --git a/src/env.cc b/src/env.cc
index 46de28f6f44f2e..c05403558f3624 100644
--- a/src/env.cc
+++ b/src/env.cc
@@ -261,6 +261,21 @@ std::ostream& operator<<(std::ostream& output,
   return output;
 }
 
+std::ostream& operator<<(std::ostream& output, const SnapshotMetadata& i) {
+  output << "{\n"
+         << "  "
+         << (i.type == SnapshotMetadata::Type::kDefault
+                 ? "SnapshotMetadata::Type::kDefault"
+                 : "SnapshotMetadata::Type::kFullyCustomized")
+         << ", // type\n"
+         << "  \"" << i.node_version << "\", // node_version\n"
+         << "  \"" << i.node_arch << "\", // node_arch\n"
+         << "  \"" << i.node_platform << "\", // node_platform\n"
+         << "  " << i.v8_cache_version_tag << ", // v8_cache_version_tag\n"
+         << "}";
+  return output;
+}
+
 IsolateDataSerializeInfo IsolateData::Serialize(SnapshotCreator* creator) {
   Isolate* isolate = creator->GetIsolate();
   IsolateDataSerializeInfo info;
diff --git a/src/env.h b/src/env.h
index 7eefd8d59a1e62..ce5d932d6f37d2 100644
--- a/src/env.h
+++ b/src/env.h
@@ -984,6 +984,19 @@ struct EnvSerializeInfo {
   friend std::ostream& operator<<(std::ostream& o, const EnvSerializeInfo& i);
 };
 
+struct SnapshotMetadata {
+  // For now kFullyCustomized is only built with the --build-snapshot CLI flag.
+  // We might want to add more types of snapshots in the future.
+  enum class Type : uint8_t { kDefault, kFullyCustomized };
+
+  Type type;
+  std::string node_version;
+  std::string node_arch;
+  std::string node_platform;
+  // Result of v8::ScriptCompiler::CachedDataVersionTag().
+  uint32_t v8_cache_version_tag;
+};
+
 struct SnapshotData {
   enum class DataOwnership { kOwned, kNotOwned };
 
@@ -993,6 +1006,8 @@ struct SnapshotData {
 
   DataOwnership data_ownership = DataOwnership::kOwned;
 
+  SnapshotMetadata metadata;
+
   // The result of v8::SnapshotCreator::CreateBlob() during the snapshot
   // building process.
   v8::StartupData v8_snapshot_blob_data{nullptr, 0};
@@ -1009,7 +1024,10 @@ struct SnapshotData {
   std::vector<builtins::CodeCacheInfo> code_cache;
 
   void ToBlob(FILE* out) const;
-  static void FromBlob(SnapshotData* out, FILE* in);
+  // If returns false, the metadata doesn't match the current Node.js binary,
+  // and the caller should not consume the snapshot data.
+  bool Check() const;
+  static bool FromBlob(SnapshotData* out, FILE* in);
 
   ~SnapshotData();
 
diff --git a/src/node.cc b/src/node.cc
index 71b3f7dcfe3cdd..f679b2db3f4b57 100644
--- a/src/node.cc
+++ b/src/node.cc
@@ -1214,13 +1214,23 @@ int LoadSnapshotDataAndRun(const SnapshotData** snapshot_data_ptr,
       return result->exit_code;
     }
     std::unique_ptr<SnapshotData> read_data = std::make_unique<SnapshotData>();
-    SnapshotData::FromBlob(read_data.get(), fp);
+    if (!SnapshotData::FromBlob(read_data.get(), fp)) {
+      // If we fail to read the customized snapshot, simply exit with 1.
+      exit_code = 1;
+      return exit_code;
+    }
     *snapshot_data_ptr = read_data.release();
     fclose(fp);
   } else if (per_process::cli_options->node_snapshot) {
     // If --snapshot-blob is not specified, we are reading the embedded
     // snapshot, but we will skip it if --no-node-snapshot is specified.
-    *snapshot_data_ptr = SnapshotBuilder::GetEmbeddedSnapshotData();
+    const node::SnapshotData* read_data =
+        SnapshotBuilder::GetEmbeddedSnapshotData();
+    if (read_data != nullptr && read_data->Check()) {
+      // If we fail to read the embedded snapshot, treat it as if Node.js
+      // was built without one.
+      *snapshot_data_ptr = read_data;
+    }
   }
 
   if ((*snapshot_data_ptr) != nullptr) {
diff --git a/src/node_internals.h b/src/node_internals.h
index e12c77bdaf4b80..0be943273e163f 100644
--- a/src/node_internals.h
+++ b/src/node_internals.h
@@ -419,6 +419,7 @@ std::ostream& operator<<(std::ostream& output,
                          const TickInfo::SerializeInfo& d);
 std::ostream& operator<<(std::ostream& output,
                          const AsyncHooks::SerializeInfo& d);
+std::ostream& operator<<(std::ostream& output, const SnapshotMetadata& d);
 
 namespace performance {
 std::ostream& operator<<(std::ostream& output,
diff --git a/src/node_snapshotable.cc b/src/node_snapshotable.cc
index e48b4797803baa..a80d1435766133 100644
--- a/src/node_snapshotable.cc
+++ b/src/node_snapshotable.cc
@@ -679,6 +679,57 @@ size_t FileWriter::Write(const EnvSerializeInfo& data) {
   return written_total;
 }
 
+// Layout of SnapshotMetadata
+// [  1 byte   ]  type of the snapshot
+// [ 4/8 bytes ]  length of the node version string
+// [    ...    ]  |length| bytes of node version
+// [ 4/8 bytes ]  length of the node arch string
+// [    ...    ]  |length| bytes of node arch
+// [ 4/8 bytes ]  length of the node platform string
+// [    ...    ]  |length| bytes of node platform
+// [  4 bytes  ]  v8 cache version tag
+template <>
+SnapshotMetadata FileReader::Read() {
+  per_process::Debug(DebugCategory::MKSNAPSHOT, "Read<SnapshotMetadata>()\n");
+
+  SnapshotMetadata result;
+  result.type = static_cast<SnapshotMetadata::Type>(Read<uint8_t>());
+  result.node_version = ReadString();
+  result.node_arch = ReadString();
+  result.node_platform = ReadString();
+  result.v8_cache_version_tag = Read<uint32_t>();
+
+  if (is_debug) {
+    std::string str = ToStr(result);
+    Debug("Read<SnapshotMetadata>() %s\n", str.c_str());
+  }
+  return result;
+}
+
+template <>
+size_t FileWriter::Write(const SnapshotMetadata& data) {
+  if (is_debug) {
+    std::string str = ToStr(data);
+    Debug("\nWrite<SnapshotMetadata>() %s\n", str.c_str());
+  }
+  size_t written_total = 0;
+  // We need the Node.js version, platform and arch to match because
+  // Node.js may perform synchronizations that are platform-specific and they
+  // can be changed in semver-patches.
+  Debug("Write snapshot type %" PRIu8 "\n", static_cast<uint8_t>(data.type));
+  written_total += Write<uint8_t>(static_cast<uint8_t>(data.type));
+  Debug("Write Node.js version %s\n", data.node_version.c_str());
+  written_total += WriteString(data.node_version);
+  Debug("Write Node.js arch %s\n", data.node_arch);
+  written_total += WriteString(data.node_arch);
+  Debug("Write Node.js platform %s\n", data.node_platform);
+  written_total += WriteString(data.node_platform);
+  Debug("Write V8 cached data version tag %" PRIx32 "\n",
+        data.v8_cache_version_tag);
+  written_total += Write<uint32_t>(data.v8_cache_version_tag);
+  return written_total;
+}
+
 // Layout of the snapshot blob
 // [   4 bytes    ]  kMagic
 // [   4/8 bytes  ]  length of Node.js version string
@@ -695,13 +746,12 @@ void SnapshotData::ToBlob(FILE* out) const {
   w.Debug("SnapshotData::ToBlob()\n");
 
   size_t written_total = 0;
+
   // Metadata
   w.Debug("Write magic %" PRIx32 "\n", kMagic);
   written_total += w.Write<uint32_t>(kMagic);
-  w.Debug("Write version %s\n", NODE_VERSION);
-  written_total += w.WriteString(NODE_VERSION);
-  w.Debug("Write arch %s\n", NODE_ARCH);
-  written_total += w.WriteString(NODE_ARCH);
+  w.Debug("Write metadata\n");
+  written_total += w.Write<SnapshotMetadata>(metadata);
 
   written_total += w.Write<v8::StartupData>(v8_snapshot_blob_data);
   w.Debug("Write isolate_data_indices\n");
@@ -712,22 +762,22 @@ void SnapshotData::ToBlob(FILE* out) const {
   w.Debug("SnapshotData::ToBlob() Wrote %d bytes\n", written_total);
 }
 
-void SnapshotData::FromBlob(SnapshotData* out, FILE* in) {
+bool SnapshotData::FromBlob(SnapshotData* out, FILE* in) {
   FileReader r(in);
   r.Debug("SnapshotData::FromBlob()\n");
 
+  DCHECK_EQ(out->data_ownership, SnapshotData::DataOwnership::kOwned);
+
   // Metadata
   uint32_t magic = r.Read<uint32_t>();
-  r.Debug("Read magic %" PRIx64 "\n", magic);
+  r.Debug("Read magic %" PRIx32 "\n", magic);
   CHECK_EQ(magic, kMagic);
-  std::string version = r.ReadString();
-  r.Debug("Read version %s\n", version.c_str());
-  CHECK_EQ(version, NODE_VERSION);
-  std::string arch = r.ReadString();
-  r.Debug("Read arch %s\n", arch.c_str());
-  CHECK_EQ(arch, NODE_ARCH);
+  out->metadata = r.Read<SnapshotMetadata>();
+  r.Debug("Read metadata\n");
+  if (!out->Check()) {
+    return false;
+  }
 
-  DCHECK_EQ(out->data_ownership, SnapshotData::DataOwnership::kOwned);
   out->v8_snapshot_blob_data = r.Read<v8::StartupData>();
   r.Debug("Read isolate_data_info\n");
   out->isolate_data_info = r.Read<IsolateDataSerializeInfo>();
@@ -736,6 +786,54 @@ void SnapshotData::FromBlob(SnapshotData* out, FILE* in) {
   out->code_cache = r.ReadVector<builtins::CodeCacheInfo>();
 
   r.Debug("SnapshotData::FromBlob() read %d bytes\n", r.read_total);
+  return true;
+}
+
+bool SnapshotData::Check() const {
+  if (metadata.node_version != per_process::metadata.versions.node) {
+    fprintf(stderr,
+            "Failed to load the startup snapshot because it was built with"
+            "Node.js version %s and the current Node.js version is %s.\n",
+            metadata.node_version.c_str(),
+            NODE_VERSION);
+    return false;
+  }
+
+  if (metadata.node_arch != per_process::metadata.arch) {
+    fprintf(stderr,
+            "Failed to load the startup snapshot because it was built with"
+            "architecture %s and the architecture is %s.\n",
+            metadata.node_arch.c_str(),
+            NODE_ARCH);
+    return false;
+  }
+
+  if (metadata.node_platform != per_process::metadata.platform) {
+    fprintf(stderr,
+            "Failed to load the startup snapshot because it was built with"
+            "platform %s and the current platform is %s.\n",
+            metadata.node_platform.c_str(),
+            NODE_PLATFORM);
+    return false;
+  }
+
+  uint32_t current_cache_version = v8::ScriptCompiler::CachedDataVersionTag();
+  if (metadata.v8_cache_version_tag != current_cache_version &&
+      metadata.type == SnapshotMetadata::Type::kFullyCustomized) {
+    // For now we only do this check for the customized snapshots - we know
+    // that the flags we use in the default snapshot are limited and safe
+    // enough so we can relax the constraints for it.
+    fprintf(stderr,
+            "Failed to load the startup snapshot because it was built with "
+            "a different version of V8 or with different V8 configurations.\n"
+            "Expected tag %" PRIx32 ", read %" PRIx32 "\n",
+            current_cache_version,
+            metadata.v8_cache_version_tag);
+    return false;
+  }
+
+  // TODO(joyeecheung): check incompatible Node.js flags.
+  return true;
 }
 
 SnapshotData::~SnapshotData() {
@@ -822,6 +920,10 @@ static const int v8_snapshot_blob_size = )"
   // -- data_ownership begins --
   SnapshotData::DataOwnership::kNotOwned,
   // -- data_ownership ends --
+  // -- metadata begins --
+)" << data->metadata
+     << R"(,
+  // -- metadata ends --
   // -- v8_snapshot_blob_data begins --
   { v8_snapshot_blob_data, v8_snapshot_blob_size },
   // -- v8_snapshot_blob_data ends --
@@ -908,6 +1010,12 @@ int SnapshotBuilder::Generate(SnapshotData* out,
     per_process::v8_platform.Platform()->UnregisterIsolate(isolate);
   });
 
+  // It's only possible to be kDefault in node_mksnapshot.
+  SnapshotMetadata::Type snapshot_type =
+      per_process::cli_options->build_snapshot
+          ? SnapshotMetadata::Type::kFullyCustomized
+          : SnapshotMetadata::Type::kDefault;
+
   {
     HandleScope scope(isolate);
     TryCatch bootstrapCatch(isolate);
@@ -956,7 +1064,7 @@ int SnapshotBuilder::Generate(SnapshotData* out,
       // point (we currently only support this kind of entry point, but we
       // could also explore snapshotting other kinds of execution modes
       // in the future).
-      if (per_process::cli_options->build_snapshot) {
+      if (snapshot_type == SnapshotMetadata::Type::kFullyCustomized) {
 #if HAVE_INSPECTOR
         // TODO(joyeecheung): move this before RunBootstrapping().
         env->InitializeInspector({});
@@ -1020,6 +1128,12 @@ int SnapshotBuilder::Generate(SnapshotData* out,
     return SNAPSHOT_ERROR;
   }
 
+  out->metadata = SnapshotMetadata{snapshot_type,
+                                   per_process::metadata.versions.node,
+                                   per_process::metadata.arch,
+                                   per_process::metadata.platform,
+                                   v8::ScriptCompiler::CachedDataVersionTag()};
+
   // We cannot resurrect the handles from the snapshot, so make sure that
   // no handles are left open in the environment after the blob is created
   // (which should trigger a GC and close all handles that can be closed).
diff --git a/test/parallel/test-snapshot-incompatible.js b/test/parallel/test-snapshot-incompatible.js
new file mode 100644
index 00000000000000..44447df13c6f7c
--- /dev/null
+++ b/test/parallel/test-snapshot-incompatible.js
@@ -0,0 +1,76 @@
+'use strict';
+
+// This tests that Node.js refuses to load snapshots built with incompatible
+// V8 configurations.
+
+require('../common');
+const assert = require('assert');
+const { spawnSync } = require('child_process');
+const tmpdir = require('../common/tmpdir');
+const fixtures = require('../common/fixtures');
+const path = require('path');
+const fs = require('fs');
+
+tmpdir.refresh();
+const blobPath = path.join(tmpdir.path, 'snapshot.blob');
+const entry = fixtures.path('empty.js');
+
+// The flag used can be any flag that makes a difference in
+// v8::ScriptCompiler::CachedDataVersionTag(). --harmony
+// is chosen here because it's stable enough and makes a difference.
+{
+  // Build a snapshot with --harmony.
+  const child = spawnSync(process.execPath, [
+    '--harmony',
+    '--snapshot-blob',
+    blobPath,
+    '--build-snapshot',
+    entry,
+  ], {
+    cwd: tmpdir.path
+  });
+  if (child.status !== 0) {
+    console.log(child.stderr.toString());
+    console.log(child.stdout.toString());
+    assert.strictEqual(child.status, 0);
+  }
+  const stats = fs.statSync(path.join(tmpdir.path, 'snapshot.blob'));
+  assert(stats.isFile());
+}
+
+{
+  // Now load the snapshot without --harmony, which should fail.
+  const child = spawnSync(process.execPath, [
+    '--snapshot-blob',
+    blobPath,
+  ], {
+    cwd: tmpdir.path,
+    env: {
+      ...process.env,
+    }
+  });
+
+  const stderr = child.stderr.toString().trim();
+  assert.match(stderr, /Failed to load the startup snapshot/);
+  assert.strictEqual(child.status, 1);
+}
+
+{
+  // Load it again with --harmony and it should work.
+  const child = spawnSync(process.execPath, [
+    '--harmony',
+    '--snapshot-blob',
+    blobPath,
+  ], {
+    cwd: tmpdir.path,
+    env: {
+      ...process.env,
+    }
+  });
+
+  if (child.status !== 0) {
+    console.log(child.stderr.toString());
+    console.log(child.stdout.toString());
+    assert.strictEqual(child.status, 0);
+  }
+}

From 62036e0ae0327830f0357989ae14c409cfcad4d7 Mon Sep 17 00:00:00 2001
From: Joyee Cheung <joyeec9h3@gmail.com>
Date: Wed, 24 Aug 2022 21:52:54 +0800
Subject: [PATCH 2/7] debugger: decrease timeout used to wait for the port to
 be free

By default, the debugger would query the specified inspector
sever port to see if it's available before starting the server,
and it would keep retrying until a timeout (previously 9999 ms)
is reached. This timeout seems to be longer than necessary. This
patch decreases the timeout to 3 seconds.

PR-URL: https://github.com/nodejs/node/pull/44359
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Richard Lau <rlau@redhat.com>
---
 lib/internal/debugger/inspect.js | 7 +++++--
 test/common/debugger.js          | 8 +++++++-
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/lib/internal/debugger/inspect.js b/lib/internal/debugger/inspect.js
index 42b5c64ab87029..e006f513f9b93f 100644
--- a/lib/internal/debugger/inspect.js
+++ b/lib/internal/debugger/inspect.js
@@ -45,7 +45,7 @@ const debuglog = util.debuglog('inspect');
 
 const { ERR_DEBUGGER_STARTUP_ERROR } = require('internal/errors').codes;
 
-async function portIsFree(host, port, timeout = 9999) {
+async function portIsFree(host, port, timeout = 3000) {
   if (port === 0) return; // Binding to a random port.
 
   const retryDelay = 150;
@@ -64,7 +64,10 @@ async function portIsFree(host, port, timeout = 9999) {
     const error = await new Promise((resolve) => {
       const socket = net.connect(port, host);
       socket.on('error', resolve);
-      socket.on('connect', resolve);
+      socket.on('connect', () => {
+        socket.end();
+        resolve();
+      });
     });
     if (error?.code === 'ECONNREFUSED') {
       return;
diff --git a/test/common/debugger.js b/test/common/debugger.js
index d2ac4f3b6c5fbf..4aff5b9a0f74d9 100644
--- a/test/common/debugger.js
+++ b/test/common/debugger.js
@@ -7,7 +7,13 @@ const BREAK_MESSAGE = new RegExp('(?:' + [
   'exception', 'other', 'promiseRejection',
 ].join('|') + ') in', 'i');
 
-const TIMEOUT = common.platformTimeout(5000);
+let TIMEOUT = common.platformTimeout(5000);
+if (common.isWindows) {
+  // Some of the windows machines in the CI need more time to receive
+  // the outputs from the client.
+  // https://github.com/nodejs/build/issues/3014
+  TIMEOUT = common.platformTimeout(15000);
+}
 
 function isPreBreak(output) {
   return /Break on start/.test(output) && /1 \(function \(exports/.test(output);

From a897b500710b72efeda87ae0fd44f62f4c1ade15 Mon Sep 17 00:00:00 2001
From: Joyee Cheung <joyeec9h3@gmail.com>
Date: Thu, 25 Aug 2022 15:56:40 +0800
Subject: [PATCH 3/7] test: raise sleep times in child process tests

sequential/test-child-process-execsync and
parallel/test-child-process-spawnsync-timeout are both flaky
on azure Windows machines, where it may take longer for Node.js
to launch and receive output from child processes. These tests
work by spawning a child processes that is supposed to sleep
for a long time, but the option is configured so that Node.js
would terminate them early when a shorter timeout is reached.
Then the tests assert that the time taken for the whole thing
is shorter than the specified sleep time (meaning the process
don't actually get to sleep for that long). To make the tests
less brittle on azure Windows, this patch raises the sleep
times in those tests on Windows platform, so that the overhead
can be taken into account there.

PR-URL: https://github.com/nodejs/node/pull/44375
Refs: https://github.com/nodejs/build/issues/3014
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Nitzan Uziely <linkgoron@gmail.com>
---
 test/parallel/test-child-process-spawnsync-timeout.js | 9 ++++++++-
 test/sequential/test-child-process-execsync.js        | 8 +++++++-
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/test/parallel/test-child-process-spawnsync-timeout.js b/test/parallel/test-child-process-spawnsync-timeout.js
index aeb2be86608cf4..426ac05a43497c 100644
--- a/test/parallel/test-child-process-spawnsync-timeout.js
+++ b/test/parallel/test-child-process-spawnsync-timeout.js
@@ -28,7 +28,14 @@ const { debuglog, getSystemErrorName } = require('util');
 const debug = debuglog('test');
 
 const TIMER = 200;
-const SLEEP = common.platformTimeout(5000);
+let SLEEP = common.platformTimeout(5000);
+
+if (common.isWindows) {
+  // Some of the windows machines in the CI need more time to launch
+  // and receive output from child processes.
+  // https://github.com/nodejs/build/issues/3014
+  SLEEP = common.platformTimeout(15000);
+}
 
 switch (process.argv[2]) {
   case 'child':
diff --git a/test/sequential/test-child-process-execsync.js b/test/sequential/test-child-process-execsync.js
index 5512eaeed7af35..75acbc34a902bd 100644
--- a/test/sequential/test-child-process-execsync.js
+++ b/test/sequential/test-child-process-execsync.js
@@ -30,7 +30,13 @@ const { execFileSync, execSync, spawnSync } = require('child_process');
 const { getSystemErrorName } = require('util');
 
 const TIMER = 200;
-const SLEEP = 2000;
+let SLEEP = 2000;
+if (common.isWindows) {
+  // Some of the windows machines in the CI need more time to launch
+  // and receive output from child processes.
+  // https://github.com/nodejs/build/issues/3014
+  SLEEP = 10000;
+}
 
 const execOpts = { encoding: 'utf8', shell: true };
 

From e521284bd96fcd311e2c34d156e390fbca7e9a17 Mon Sep 17 00:00:00 2001
From: Joyee Cheung <joyeec9h3@gmail.com>
Date: Thu, 25 Aug 2022 12:37:15 +0800
Subject: [PATCH 4/7] test: split heap prof tests

The original heap prof tests can take too long to complete on
azure Windows machines, resulting in timeouts. Split them into
smaller tests and move them into the parallel directory to
speed up the execution.

PR-URL: https://github.com/nodejs/node/pull/44388
Refs: https://github.com/nodejs/reliability/issues/356
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 test/common/prof.js                          |  67 ++++
 test/parallel/test-heap-prof-basic.js        |  37 ++
 test/parallel/test-heap-prof-dir-absolute.js |  47 +++
 test/parallel/test-heap-prof-dir-name.js     |  49 +++
 test/parallel/test-heap-prof-dir-relative.js |  46 +++
 test/parallel/test-heap-prof-exec-argv.js    |  39 ++
 test/parallel/test-heap-prof-exit.js         |  41 ++
 test/parallel/test-heap-prof-interval.js     |  56 +++
 test/parallel/test-heap-prof-invalid-args.js |  82 ++++
 test/parallel/test-heap-prof-loop-drained.js |  41 ++
 test/parallel/test-heap-prof-name.js         |  44 +++
 test/parallel/test-heap-prof-sigint.js       |  43 +++
 test/sequential/test-heap-prof.js            | 375 -------------------
 13 files changed, 592 insertions(+), 375 deletions(-)
 create mode 100644 test/common/prof.js
 create mode 100644 test/parallel/test-heap-prof-basic.js
 create mode 100644 test/parallel/test-heap-prof-dir-absolute.js
 create mode 100644 test/parallel/test-heap-prof-dir-name.js
 create mode 100644 test/parallel/test-heap-prof-dir-relative.js
 create mode 100644 test/parallel/test-heap-prof-exec-argv.js
 create mode 100644 test/parallel/test-heap-prof-exit.js
 create mode 100644 test/parallel/test-heap-prof-interval.js
 create mode 100644 test/parallel/test-heap-prof-invalid-args.js
 create mode 100644 test/parallel/test-heap-prof-loop-drained.js
 create mode 100644 test/parallel/test-heap-prof-name.js
 create mode 100644 test/parallel/test-heap-prof-sigint.js
 delete mode 100644 test/sequential/test-heap-prof.js

diff --git a/test/common/prof.js b/test/common/prof.js
new file mode 100644
index 00000000000000..3eb288be1b9ca3
--- /dev/null
+++ b/test/common/prof.js
@@ -0,0 +1,67 @@
+'use strict';
+
+const assert = require('assert');
+const fs = require('fs');
+const path = require('path');
+
+function getHeapProfiles(dir) {
+  const list = fs.readdirSync(dir);
+  return list
+    .filter((file) => file.endsWith('.heapprofile'))
+    .map((file) => path.join(dir, file));
+}
+
+function findFirstFrameInNode(root, func) {
+  const first = root.children.find(
+    (child) => child.callFrame.functionName === func
+  );
+  if (first) {
+    return first;
+  }
+  for (const child of root.children) {
+    const first = findFirstFrameInNode(child, func);
+    if (first) {
+      return first;
+    }
+  }
+  return undefined;
+}
+
+function findFirstFrame(file, func) {
+  const data = fs.readFileSync(file, 'utf8');
+  const profile = JSON.parse(data);
+  const first = findFirstFrameInNode(profile.head, func);
+  return { frame: first, roots: profile.head.children };
+}
+
+function verifyFrames(output, file, func) {
+  const { frame, roots } = findFirstFrame(file, func);
+  if (!frame) {
+    // Show native debug output and the profile for debugging.
+    console.log(output.stderr.toString());
+    console.log(roots);
+  }
+  assert.notStrictEqual(frame, undefined);
+}
+
+// We need to set --heap-prof-interval to a small enough value to make
+// sure we can find our workload in the samples, so we need to set
+// TEST_ALLOCATION > kHeapProfInterval.
+const kHeapProfInterval = 128;
+const TEST_ALLOCATION = kHeapProfInterval * 2;
+
+const env = {
+  ...process.env,
+  TEST_ALLOCATION,
+  NODE_DEBUG_NATIVE: 'INSPECTOR_PROFILER'
+};
+
+// TODO(joyeecheung): share the fixutres with v8 coverage tests
+module.exports = {
+  getHeapProfiles,
+  verifyFrames,
+  findFirstFrame,
+  kHeapProfInterval,
+  TEST_ALLOCATION,
+  env
+};
diff --git a/test/parallel/test-heap-prof-basic.js b/test/parallel/test-heap-prof-basic.js
new file mode 100644
index 00000000000000..34d8af9a784073
--- /dev/null
+++ b/test/parallel/test-heap-prof-basic.js
@@ -0,0 +1,37 @@
+'use strict';
+
+// Tests --heap-prof without --heap-prof-interval. Here we just verify that
+// we manage to generate a profile.
+
+const common = require('../common');
+
+const fixtures = require('../common/fixtures');
+common.skipIfInspectorDisabled();
+
+const assert = require('assert');
+const { spawnSync } = require('child_process');
+
+const tmpdir = require('../common/tmpdir');
+
+const {
+  getHeapProfiles,
+  env
+} = require('../common/prof');
+
+{
+  tmpdir.refresh();
+  const output = spawnSync(process.execPath, [
+    '--heap-prof',
+    fixtures.path('workload', 'allocation.js'),
+  ], {
+    cwd: tmpdir.path,
+    env
+  });
+  if (output.status !== 0) {
+    console.log(output.stderr.toString());
+    console.log(output);
+  }
+  assert.strictEqual(output.status, 0);
+  const profiles = getHeapProfiles(tmpdir.path);
+  assert.strictEqual(profiles.length, 1);
+}
diff --git a/test/parallel/test-heap-prof-dir-absolute.js b/test/parallel/test-heap-prof-dir-absolute.js
new file mode 100644
index 00000000000000..5fa22470545624
--- /dev/null
+++ b/test/parallel/test-heap-prof-dir-absolute.js
@@ -0,0 +1,47 @@
+'use strict';
+
+// This tests that --heap-prof, --heap-prof-dir and --heap-prof-name works.
+
+const common = require('../common');
+
+const fixtures = require('../common/fixtures');
+common.skipIfInspectorDisabled();
+
+const assert = require('assert');
+const fs = require('fs');
+const path = require('path');
+const { spawnSync } = require('child_process');
+
+const tmpdir = require('../common/tmpdir');
+
+const {
+  getHeapProfiles,
+  verifyFrames,
+  kHeapProfInterval,
+  env
+} = require('../common/prof');
+
+// Tests absolute --heap-prof-dir
+{
+  tmpdir.refresh();
+  const dir = path.join(tmpdir.path, 'prof');
+  const output = spawnSync(process.execPath, [
+    '--heap-prof',
+    '--heap-prof-dir',
+    dir,
+    '--heap-prof-interval',
+    kHeapProfInterval,
+    fixtures.path('workload', 'allocation.js'),
+  ], {
+    cwd: tmpdir.path,
+    env
+  });
+  if (output.status !== 0) {
+    console.log(output.stderr.toString());
+  }
+  assert.strictEqual(output.status, 0);
+  assert(fs.existsSync(dir));
+  const profiles = getHeapProfiles(dir);
+  assert.strictEqual(profiles.length, 1);
+  verifyFrames(output, profiles[0], 'runAllocation');
+}
diff --git a/test/parallel/test-heap-prof-dir-name.js b/test/parallel/test-heap-prof-dir-name.js
new file mode 100644
index 00000000000000..ebc9374859663f
--- /dev/null
+++ b/test/parallel/test-heap-prof-dir-name.js
@@ -0,0 +1,49 @@
+'use strict';
+
+// Tests --heap-prof-dir and --heap-prof-name work together.
+
+const common = require('../common');
+
+const fixtures = require('../common/fixtures');
+common.skipIfInspectorDisabled();
+
+const assert = require('assert');
+const fs = require('fs');
+const path = require('path');
+const { spawnSync } = require('child_process');
+
+const tmpdir = require('../common/tmpdir');
+
+const {
+  getHeapProfiles,
+  verifyFrames,
+  kHeapProfInterval,
+  env
+} = require('../common/prof');
+
+{
+  tmpdir.refresh();
+  const dir = path.join(tmpdir.path, 'prof');
+  const file = path.join(dir, 'test.heapprofile');
+  const output = spawnSync(process.execPath, [
+    '--heap-prof',
+    '--heap-prof-name',
+    'test.heapprofile',
+    '--heap-prof-dir',
+    dir,
+    '--heap-prof-interval',
+    kHeapProfInterval,
+    fixtures.path('workload', 'allocation.js'),
+  ], {
+    cwd: tmpdir.path,
+    env
+  });
+  if (output.status !== 0) {
+    console.log(output.stderr.toString());
+  }
+  assert.strictEqual(output.status, 0);
+  assert(fs.existsSync(dir));
+  const profiles = getHeapProfiles(dir);
+  assert.deepStrictEqual(profiles, [file]);
+  verifyFrames(output, file, 'runAllocation');
+}
diff --git a/test/parallel/test-heap-prof-dir-relative.js b/test/parallel/test-heap-prof-dir-relative.js
new file mode 100644
index 00000000000000..8b71856cde99bc
--- /dev/null
+++ b/test/parallel/test-heap-prof-dir-relative.js
@@ -0,0 +1,46 @@
+'use strict';
+
+// Tests relative --heap-prof-dir works.
+
+const common = require('../common');
+
+const fixtures = require('../common/fixtures');
+common.skipIfInspectorDisabled();
+
+const assert = require('assert');
+const fs = require('fs');
+const path = require('path');
+const { spawnSync } = require('child_process');
+
+const tmpdir = require('../common/tmpdir');
+
+const {
+  getHeapProfiles,
+  verifyFrames,
+  kHeapProfInterval,
+  env
+} = require('../common/prof');
+
+{
+  tmpdir.refresh();
+  const output = spawnSync(process.execPath, [
+    '--heap-prof',
+    '--heap-prof-dir',
+    'prof',
+    '--heap-prof-interval',
+    kHeapProfInterval,
+    fixtures.path('workload', 'allocation.js'),
+  ], {
+    cwd: tmpdir.path,
+    env
+  });
+  if (output.status !== 0) {
+    console.log(output.stderr.toString());
+  }
+  assert.strictEqual(output.status, 0);
+  const dir = path.join(tmpdir.path, 'prof');
+  assert(fs.existsSync(dir));
+  const profiles = getHeapProfiles(dir);
+  assert.strictEqual(profiles.length, 1);
+  verifyFrames(output, profiles[0], 'runAllocation');
+}
diff --git a/test/parallel/test-heap-prof-exec-argv.js b/test/parallel/test-heap-prof-exec-argv.js
new file mode 100644
index 00000000000000..02ad4430dba769
--- /dev/null
+++ b/test/parallel/test-heap-prof-exec-argv.js
@@ -0,0 +1,39 @@
+'use strict';
+
+// Tests --heap-prof generates a heap profile from worker
+// when execArgv is set.
+
+const common = require('../common');
+
+const fixtures = require('../common/fixtures');
+common.skipIfInspectorDisabled();
+
+const assert = require('assert');
+const { spawnSync } = require('child_process');
+
+const tmpdir = require('../common/tmpdir');
+
+const {
+  getHeapProfiles,
+  verifyFrames,
+} = require('../common/prof');
+
+{
+  tmpdir.refresh();
+  const output = spawnSync(process.execPath, [
+    fixtures.path('workload', 'allocation-worker-argv.js'),
+  ], {
+    cwd: tmpdir.path,
+    env: {
+      ...process.env,
+      HEAP_PROF_INTERVAL: '128'
+    }
+  });
+  if (output.status !== 0) {
+    console.log(output.stderr.toString());
+  }
+  assert.strictEqual(output.status, 0);
+  const profiles = getHeapProfiles(tmpdir.path);
+  assert.strictEqual(profiles.length, 1);
+  verifyFrames(output, profiles[0], 'runAllocation');
+}
diff --git a/test/parallel/test-heap-prof-exit.js b/test/parallel/test-heap-prof-exit.js
new file mode 100644
index 00000000000000..ed5073c18f4785
--- /dev/null
+++ b/test/parallel/test-heap-prof-exit.js
@@ -0,0 +1,41 @@
+'use strict';
+
+// Tests --heap-prof generates a heap profile when process.exit(55) exits
+// process.
+
+const common = require('../common');
+
+const fixtures = require('../common/fixtures');
+common.skipIfInspectorDisabled();
+
+const assert = require('assert');
+const { spawnSync } = require('child_process');
+
+const tmpdir = require('../common/tmpdir');
+
+const {
+  getHeapProfiles,
+  verifyFrames,
+  kHeapProfInterval,
+  env
+} = require('../common/prof');
+
+{
+  tmpdir.refresh();
+  const output = spawnSync(process.execPath, [
+    '--heap-prof',
+    '--heap-prof-interval',
+    kHeapProfInterval,
+    fixtures.path('workload', 'allocation-exit.js'),
+  ], {
+    cwd: tmpdir.path,
+    env
+  });
+  if (output.status !== 55) {
+    console.log(output.stderr.toString());
+  }
+  assert.strictEqual(output.status, 55);
+  const profiles = getHeapProfiles(tmpdir.path);
+  assert.strictEqual(profiles.length, 1);
+  verifyFrames(output, profiles[0], 'runAllocation');
+}
diff --git a/test/parallel/test-heap-prof-interval.js b/test/parallel/test-heap-prof-interval.js
new file mode 100644
index 00000000000000..18841016a71371
--- /dev/null
+++ b/test/parallel/test-heap-prof-interval.js
@@ -0,0 +1,56 @@
+'use strict';
+
+// Tests multiple profiles generated by --heap-prof-interval are valid.
+
+const common = require('../common');
+
+const fixtures = require('../common/fixtures');
+common.skipIfInspectorDisabled();
+
+const assert = require('assert');
+const fs = require('fs');
+const path = require('path');
+const { spawnSync } = require('child_process');
+
+const tmpdir = require('../common/tmpdir');
+
+const {
+  getHeapProfiles,
+  findFirstFrame,
+  kHeapProfInterval,
+  env
+} = require('../common/prof');
+
+{
+  tmpdir.refresh();
+  const output = spawnSync(process.execPath, [
+    '--heap-prof-interval',
+    kHeapProfInterval,
+    '--heap-prof-dir',
+    'prof',
+    '--heap-prof',
+    fixtures.path('workload', 'allocation-worker.js'),
+  ], {
+    cwd: tmpdir.path,
+    env
+  });
+  if (output.status !== 0) {
+    console.log(output.stderr.toString());
+  }
+  assert.strictEqual(output.status, 0);
+  const dir = path.join(tmpdir.path, 'prof');
+  assert(fs.existsSync(dir));
+  const profiles = getHeapProfiles(dir);
+  assert.strictEqual(profiles.length, 2);
+  const profile1 = findFirstFrame(profiles[0], 'runAllocation');
+  const profile2 = findFirstFrame(profiles[1], 'runAllocation');
+  if (!profile1.frame && !profile2.frame) {
+    // Show native debug output and the profile for debugging.
+    console.log(output.stderr.toString());
+    console.log('heap path: ', profiles[0]);
+    console.log(profile1.roots);
+    console.log('heap path: ', profiles[1]);
+    console.log(profile2.roots);
+  }
+  assert(profile1.frame || profile2.frame);
+}
diff --git a/test/parallel/test-heap-prof-invalid-args.js b/test/parallel/test-heap-prof-invalid-args.js
new file mode 100644
index 00000000000000..e35376038ecc93
--- /dev/null
+++ b/test/parallel/test-heap-prof-invalid-args.js
@@ -0,0 +1,82 @@
+'use strict';
+
+// Tests invalid --heap-prof CLI arguments.
+
+const common = require('../common');
+
+const fixtures = require('../common/fixtures');
+common.skipIfInspectorDisabled();
+
+const assert = require('assert');
+const { spawnSync } = require('child_process');
+
+const tmpdir = require('../common/tmpdir');
+
+const {
+  kHeapProfInterval,
+  env
+} = require('../common/prof');
+
+// Tests --heap-prof-name without --heap-prof.
+{
+  tmpdir.refresh();
+  const output = spawnSync(process.execPath, [
+    '--heap-prof-name',
+    'test.heapprofile',
+    fixtures.path('workload', 'allocation.js'),
+  ], {
+    cwd: tmpdir.path,
+    env
+  });
+  const stderr = output.stderr.toString().trim();
+  if (output.status !== 9) {
+    console.log(stderr);
+  }
+  assert.strictEqual(output.status, 9);
+  assert.strictEqual(
+    stderr,
+    `${process.execPath}: --heap-prof-name must be used with --heap-prof`);
+}
+
+// Tests --heap-prof-dir without --heap-prof.
+{
+  tmpdir.refresh();
+  const output = spawnSync(process.execPath, [
+    '--heap-prof-dir',
+    'prof',
+    fixtures.path('workload', 'allocation.js'),
+  ], {
+    cwd: tmpdir.path,
+    env
+  });
+  const stderr = output.stderr.toString().trim();
+  if (output.status !== 9) {
+    console.log(stderr);
+  }
+  assert.strictEqual(output.status, 9);
+  assert.strictEqual(
+    stderr,
+    `${process.execPath}: --heap-prof-dir must be used with --heap-prof`);
+}
+
+// Tests --heap-prof-interval without --heap-prof.
+{
+  tmpdir.refresh();
+  const output = spawnSync(process.execPath, [
+    '--heap-prof-interval',
+    kHeapProfInterval,
+    fixtures.path('workload', 'allocation.js'),
+  ], {
+    cwd: tmpdir.path,
+    env
+  });
+  const stderr = output.stderr.toString().trim();
+  if (output.status !== 9) {
+    console.log(stderr);
+  }
+  assert.strictEqual(output.status, 9);
+  assert.strictEqual(
+    stderr,
+    `${process.execPath}: ` +
+    '--heap-prof-interval must be used with --heap-prof');
+}
diff --git a/test/parallel/test-heap-prof-loop-drained.js b/test/parallel/test-heap-prof-loop-drained.js
new file mode 100644
index 00000000000000..d0fc4c98784917
--- /dev/null
+++ b/test/parallel/test-heap-prof-loop-drained.js
@@ -0,0 +1,41 @@
+'use strict';
+
+// Tests that --heap-prof outputs heap profile when event loop is drained.
+
+const common = require('../common');
+
+const fixtures = require('../common/fixtures');
+common.skipIfInspectorDisabled();
+
+const assert = require('assert');
+const { spawnSync } = require('child_process');
+
+const tmpdir = require('../common/tmpdir');
+
+const {
+  getHeapProfiles,
+  verifyFrames,
+  kHeapProfInterval,
+  env,
+} = require('../common/prof');
+
+{
+  tmpdir.refresh();
+  const output = spawnSync(process.execPath, [
+    '--heap-prof',
+    '--heap-prof-interval',
+    kHeapProfInterval,
+    fixtures.path('workload', 'allocation.js'),
+  ], {
+    cwd: tmpdir.path,
+    env
+  });
+  if (output.status !== 0) {
+    console.log(output.stderr.toString());
+    console.log(output);
+  }
+  assert.strictEqual(output.status, 0);
+  const profiles = getHeapProfiles(tmpdir.path);
+  assert.strictEqual(profiles.length, 1);
+  verifyFrames(output, profiles[0], 'runAllocation');
+}
diff --git a/test/parallel/test-heap-prof-name.js b/test/parallel/test-heap-prof-name.js
new file mode 100644
index 00000000000000..a2bf6aae396203
--- /dev/null
+++ b/test/parallel/test-heap-prof-name.js
@@ -0,0 +1,44 @@
+'use strict';
+
+// Tests --heap-prof-name works.
+
+const common = require('../common');
+
+const fixtures = require('../common/fixtures');
+common.skipIfInspectorDisabled();
+
+const assert = require('assert');
+const path = require('path');
+const { spawnSync } = require('child_process');
+
+const tmpdir = require('../common/tmpdir');
+
+const {
+  getHeapProfiles,
+  verifyFrames,
+  kHeapProfInterval,
+  env
+} = require('../common/prof');
+
+{
+  tmpdir.refresh();
+  const file = path.join(tmpdir.path, 'test.heapprofile');
+  const output = spawnSync(process.execPath, [
+    '--heap-prof',
+    '--heap-prof-name',
+    'test.heapprofile',
+    '--heap-prof-interval',
+    kHeapProfInterval,
+    fixtures.path('workload', 'allocation.js'),
+  ], {
+    cwd: tmpdir.path,
+    env
+  });
+  if (output.status !== 0) {
+    console.log(output.stderr.toString());
+  }
+  assert.strictEqual(output.status, 0);
+  const profiles = getHeapProfiles(tmpdir.path);
+  assert.deepStrictEqual(profiles, [file]);
+  verifyFrames(output, file, 'runAllocation');
+}
diff --git a/test/parallel/test-heap-prof-sigint.js b/test/parallel/test-heap-prof-sigint.js
new file mode 100644
index 00000000000000..ec078256c2a8e9
--- /dev/null
+++ b/test/parallel/test-heap-prof-sigint.js
@@ -0,0 +1,43 @@
+'use strict';
+
+// Tests --heap-prof generates a heap profile when
+// process.kill(process.pid, "SIGINT"); exits process.
+
+const common = require('../common');
+
+const fixtures = require('../common/fixtures');
+common.skipIfInspectorDisabled();
+
+const assert = require('assert');
+const { spawnSync } = require('child_process');
+
+const tmpdir = require('../common/tmpdir');
+
+const {
+  getHeapProfiles,
+  verifyFrames,
+  kHeapProfInterval,
+  env
+} = require('../common/prof');
+
+{
+  tmpdir.refresh();
+  const output = spawnSync(process.execPath, [
+    '--heap-prof',
+    '--heap-prof-interval',
+    kHeapProfInterval,
+    fixtures.path('workload', 'allocation-sigint.js'),
+  ], {
+    cwd: tmpdir.path,
+    env
+  });
+  if (!common.isWindows) {
+    if (output.signal !== 'SIGINT') {
+      console.log(output.stderr.toString());
+    }
+    assert.strictEqual(output.signal, 'SIGINT');
+  }
+  const profiles = getHeapProfiles(tmpdir.path);
+  assert.strictEqual(profiles.length, 1);
+  verifyFrames(output, profiles[0], 'runAllocation');
+}
diff --git a/test/sequential/test-heap-prof.js b/test/sequential/test-heap-prof.js
deleted file mode 100644
index 1dda8f367433e8..00000000000000
--- a/test/sequential/test-heap-prof.js
+++ /dev/null
@@ -1,375 +0,0 @@
-'use strict';
-
-// This tests that --heap-prof, --heap-prof-dir and --heap-prof-name works.
-
-const common = require('../common');
-
-const fixtures = require('../common/fixtures');
-common.skipIfInspectorDisabled();
-
-const assert = require('assert');
-const fs = require('fs');
-const path = require('path');
-const { spawnSync } = require('child_process');
-
-const tmpdir = require('../common/tmpdir');
-
-function getHeapProfiles(dir) {
-  const list = fs.readdirSync(dir);
-  return list
-    .filter((file) => file.endsWith('.heapprofile'))
-    .map((file) => path.join(dir, file));
-}
-
-function findFirstFrameInNode(root, func) {
-  const first = root.children.find(
-    (child) => child.callFrame.functionName === func
-  );
-  if (first) {
-    return first;
-  }
-  for (const child of root.children) {
-    const first = findFirstFrameInNode(child, func);
-    if (first) {
-      return first;
-    }
-  }
-  return undefined;
-}
-
-function findFirstFrame(file, func) {
-  const data = fs.readFileSync(file, 'utf8');
-  const profile = JSON.parse(data);
-  const first = findFirstFrameInNode(profile.head, func);
-  return { frame: first, roots: profile.head.children };
-}
-
-function verifyFrames(output, file, func) {
-  const { frame, roots } = findFirstFrame(file, func);
-  if (!frame) {
-    // Show native debug output and the profile for debugging.
-    console.log(output.stderr.toString());
-    console.log(roots);
-  }
-  assert.notStrictEqual(frame, undefined);
-}
-
-// We need to set --heap-prof-interval to a small enough value to make
-// sure we can find our workload in the samples, so we need to set
-// TEST_ALLOCATION > kHeapProfInterval.
-const kHeapProfInterval = 128;
-const TEST_ALLOCATION = kHeapProfInterval * 2;
-
-const env = {
-  ...process.env,
-  TEST_ALLOCATION,
-  NODE_DEBUG_NATIVE: 'INSPECTOR_PROFILER'
-};
-
-// Test --heap-prof without --heap-prof-interval. Here we just verify that
-// we manage to generate a profile.
-{
-  tmpdir.refresh();
-  const output = spawnSync(process.execPath, [
-    '--heap-prof',
-    fixtures.path('workload', 'allocation.js'),
-  ], {
-    cwd: tmpdir.path,
-    env
-  });
-  if (output.status !== 0) {
-    console.log(output.stderr.toString());
-    console.log(output);
-  }
-  assert.strictEqual(output.status, 0);
-  const profiles = getHeapProfiles(tmpdir.path);
-  assert.strictEqual(profiles.length, 1);
-}
-
-// Outputs heap profile when event loop is drained.
-// TODO(joyeecheung): share the fixutres with v8 coverage tests
-{
-  tmpdir.refresh();
-  const output = spawnSync(process.execPath, [
-    '--heap-prof',
-    '--heap-prof-interval',
-    kHeapProfInterval,
-    fixtures.path('workload', 'allocation.js'),
-  ], {
-    cwd: tmpdir.path,
-    env
-  });
-  if (output.status !== 0) {
-    console.log(output.stderr.toString());
-    console.log(output);
-  }
-  assert.strictEqual(output.status, 0);
-  const profiles = getHeapProfiles(tmpdir.path);
-  assert.strictEqual(profiles.length, 1);
-  verifyFrames(output, profiles[0], 'runAllocation');
-}
-
-// Outputs heap profile when process.exit(55) exits process.
-{
-  tmpdir.refresh();
-  const output = spawnSync(process.execPath, [
-    '--heap-prof',
-    '--heap-prof-interval',
-    kHeapProfInterval,
-    fixtures.path('workload', 'allocation-exit.js'),
-  ], {
-    cwd: tmpdir.path,
-    env
-  });
-  if (output.status !== 55) {
-    console.log(output.stderr.toString());
-  }
-  assert.strictEqual(output.status, 55);
-  const profiles = getHeapProfiles(tmpdir.path);
-  assert.strictEqual(profiles.length, 1);
-  verifyFrames(output, profiles[0], 'runAllocation');
-}
-
-// Outputs heap profile when process.kill(process.pid, "SIGINT"); exits process.
-{
-  tmpdir.refresh();
-  const output = spawnSync(process.execPath, [
-    '--heap-prof',
-    '--heap-prof-interval',
-    kHeapProfInterval,
-    fixtures.path('workload', 'allocation-sigint.js'),
-  ], {
-    cwd: tmpdir.path,
-    env
-  });
-  if (!common.isWindows) {
-    if (output.signal !== 'SIGINT') {
-      console.log(output.stderr.toString());
-    }
-    assert.strictEqual(output.signal, 'SIGINT');
-  }
-  const profiles = getHeapProfiles(tmpdir.path);
-  assert.strictEqual(profiles.length, 1);
-  verifyFrames(output, profiles[0], 'runAllocation');
-}
-
-// Outputs heap profile from worker when execArgv is set.
-{
-  tmpdir.refresh();
-  const output = spawnSync(process.execPath, [
-    fixtures.path('workload', 'allocation-worker-argv.js'),
-  ], {
-    cwd: tmpdir.path,
-    env: {
-      ...process.env,
-      HEAP_PROF_INTERVAL: '128'
-    }
-  });
-  if (output.status !== 0) {
-    console.log(output.stderr.toString());
-  }
-  assert.strictEqual(output.status, 0);
-  const profiles = getHeapProfiles(tmpdir.path);
-  assert.strictEqual(profiles.length, 1);
-  verifyFrames(output, profiles[0], 'runAllocation');
-}
-
-// --heap-prof-name without --heap-prof
-{
-  tmpdir.refresh();
-  const output = spawnSync(process.execPath, [
-    '--heap-prof-name',
-    'test.heapprofile',
-    fixtures.path('workload', 'allocation.js'),
-  ], {
-    cwd: tmpdir.path,
-    env
-  });
-  const stderr = output.stderr.toString().trim();
-  if (output.status !== 9) {
-    console.log(stderr);
-  }
-  assert.strictEqual(output.status, 9);
-  assert.strictEqual(
-    stderr,
-    `${process.execPath}: --heap-prof-name must be used with --heap-prof`);
-}
-
-// --heap-prof-dir without --heap-prof
-{
-  tmpdir.refresh();
-  const output = spawnSync(process.execPath, [
-    '--heap-prof-dir',
-    'prof',
-    fixtures.path('workload', 'allocation.js'),
-  ], {
-    cwd: tmpdir.path,
-    env
-  });
-  const stderr = output.stderr.toString().trim();
-  if (output.status !== 9) {
-    console.log(stderr);
-  }
-  assert.strictEqual(output.status, 9);
-  assert.strictEqual(
-    stderr,
-    `${process.execPath}: --heap-prof-dir must be used with --heap-prof`);
-}
-
-// --heap-prof-interval without --heap-prof
-{
-  tmpdir.refresh();
-  const output = spawnSync(process.execPath, [
-    '--heap-prof-interval',
-    kHeapProfInterval,
-    fixtures.path('workload', 'allocation.js'),
-  ], {
-    cwd: tmpdir.path,
-    env
-  });
-  const stderr = output.stderr.toString().trim();
-  if (output.status !== 9) {
-    console.log(stderr);
-  }
-  assert.strictEqual(output.status, 9);
-  assert.strictEqual(
-    stderr,
-    `${process.execPath}: ` +
-    '--heap-prof-interval must be used with --heap-prof');
-}
-
-// --heap-prof-name
-{
-  tmpdir.refresh();
-  const file = path.join(tmpdir.path, 'test.heapprofile');
-  const output = spawnSync(process.execPath, [
-    '--heap-prof',
-    '--heap-prof-name',
-    'test.heapprofile',
-    '--heap-prof-interval',
-    kHeapProfInterval,
-    fixtures.path('workload', 'allocation.js'),
-  ], {
-    cwd: tmpdir.path,
-    env
-  });
-  if (output.status !== 0) {
-    console.log(output.stderr.toString());
-  }
-  assert.strictEqual(output.status, 0);
-  const profiles = getHeapProfiles(tmpdir.path);
-  assert.deepStrictEqual(profiles, [file]);
-  verifyFrames(output, file, 'runAllocation');
-}
-
-// relative --heap-prof-dir
-{
-  tmpdir.refresh();
-  const output = spawnSync(process.execPath, [
-    '--heap-prof',
-    '--heap-prof-dir',
-    'prof',
-    '--heap-prof-interval',
-    kHeapProfInterval,
-    fixtures.path('workload', 'allocation.js'),
-  ], {
-    cwd: tmpdir.path,
-    env
-  });
-  if (output.status !== 0) {
-    console.log(output.stderr.toString());
-  }
-  assert.strictEqual(output.status, 0);
-  const dir = path.join(tmpdir.path, 'prof');
-  assert(fs.existsSync(dir));
-  const profiles = getHeapProfiles(dir);
-  assert.strictEqual(profiles.length, 1);
-  verifyFrames(output, profiles[0], 'runAllocation');
-}
-
-// absolute --heap-prof-dir
-{
-  tmpdir.refresh();
-  const dir = path.join(tmpdir.path, 'prof');
-  const output = spawnSync(process.execPath, [
-    '--heap-prof',
-    '--heap-prof-dir',
-    dir,
-    '--heap-prof-interval',
-    kHeapProfInterval,
-    fixtures.path('workload', 'allocation.js'),
-  ], {
-    cwd: tmpdir.path,
-    env
-  });
-  if (output.status !== 0) {
-    console.log(output.stderr.toString());
-  }
-  assert.strictEqual(output.status, 0);
-  assert(fs.existsSync(dir));
-  const profiles = getHeapProfiles(dir);
-  assert.strictEqual(profiles.length, 1);
-  verifyFrames(output, profiles[0], 'runAllocation');
-}
-
-// --heap-prof-dir and --heap-prof-name
-{
-  tmpdir.refresh();
-  const dir = path.join(tmpdir.path, 'prof');
-  const file = path.join(dir, 'test.heapprofile');
-  const output = spawnSync(process.execPath, [
-    '--heap-prof',
-    '--heap-prof-name',
-    'test.heapprofile',
-    '--heap-prof-dir',
-    dir,
-    '--heap-prof-interval',
-    kHeapProfInterval,
-    fixtures.path('workload', 'allocation.js'),
-  ], {
-    cwd: tmpdir.path,
-    env
-  });
-  if (output.status !== 0) {
-    console.log(output.stderr.toString());
-  }
-  assert.strictEqual(output.status, 0);
-  assert(fs.existsSync(dir));
-  const profiles = getHeapProfiles(dir);
-  assert.deepStrictEqual(profiles, [file]);
-  verifyFrames(output, file, 'runAllocation');
-}
-
-{
-  tmpdir.refresh();
-  const output = spawnSync(process.execPath, [
-    '--heap-prof-interval',
-    kHeapProfInterval,
-    '--heap-prof-dir',
-    'prof',
-    '--heap-prof',
-    fixtures.path('workload', 'allocation-worker.js'),
-  ], {
-    cwd: tmpdir.path,
-    env
-  });
-  if (output.status !== 0) {
-    console.log(output.stderr.toString());
-  }
-  assert.strictEqual(output.status, 0);
-  const dir = path.join(tmpdir.path, 'prof');
-  assert(fs.existsSync(dir));
-  const profiles = getHeapProfiles(dir);
-  assert.strictEqual(profiles.length, 2);
-  const profile1 = findFirstFrame(profiles[0], 'runAllocation');
-  const profile2 = findFirstFrame(profiles[1], 'runAllocation');
-  if (!profile1.frame && !profile2.frame) {
-    // Show native debug output and the profile for debugging.
-    console.log(output.stderr.toString());
-    console.log('heap path: ', profiles[0]);
-    console.log(profile1.roots);
-    console.log('heap path: ', profiles[1]);
-    console.log(profile2.roots);
-  }
-  assert(profile1.frame || profile2.frame);
-}

From 5dd57efe52c349bd7fd4845df1bb346533475552 Mon Sep 17 00:00:00 2001
From: Joyee Cheung <joyeec9h3@gmail.com>
Date: Mon, 29 Aug 2022 18:27:40 +0800
Subject: [PATCH 5/7] test: make the vm timeout escape tests more lenient

Previously the tests required that Node.js finish the initialization
of the watchdog thread and fires the timeout within 100ms, which
can be difficult on certain systems under load. This patch
relaxes the requirement to 2000ms. If there is a bug and the
timeout can actually be escaped, raising the timeout to 2000ms
would not make a difference anyway.

PR-URL: https://github.com/nodejs/node/pull/44433
Refs: https://github.com/nodejs/reliability/issues/333
Refs: https://github.com/nodejs/reliability/issues/361
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
---
 test/parallel/test-vm-timeout-escape-promise-2.js      | 2 +-
 test/parallel/test-vm-timeout-escape-promise-module.js | 2 +-
 test/parallel/test-vm-timeout-escape-promise.js        | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/test/parallel/test-vm-timeout-escape-promise-2.js b/test/parallel/test-vm-timeout-escape-promise-2.js
index a3cb3dbc7fd442..db40d0ef1ccc47 100644
--- a/test/parallel/test-vm-timeout-escape-promise-2.js
+++ b/test/parallel/test-vm-timeout-escape-promise-2.js
@@ -16,7 +16,7 @@ function loop() {
   while (1) {
     const current = hrtime();
     const span = (current - start) / NS_PER_MS;
-    if (span >= 100n) {
+    if (span >= 2000n) {
       throw new Error(
         `escaped timeout at ${span} milliseconds!`);
     }
diff --git a/test/parallel/test-vm-timeout-escape-promise-module.js b/test/parallel/test-vm-timeout-escape-promise-module.js
index 3451c0af71c16f..0687b3b3c81f57 100644
--- a/test/parallel/test-vm-timeout-escape-promise-module.js
+++ b/test/parallel/test-vm-timeout-escape-promise-module.js
@@ -18,7 +18,7 @@ function loop() {
   while (1) {
     const current = hrtime();
     const span = (current - start) / NS_PER_MS;
-    if (span >= 100n) {
+    if (span >= 2000n) {
       throw new Error(
         `escaped timeout at ${span} milliseconds!`);
     }
diff --git a/test/parallel/test-vm-timeout-escape-promise.js b/test/parallel/test-vm-timeout-escape-promise.js
index 0e82ab8c86a935..36a76001af348a 100644
--- a/test/parallel/test-vm-timeout-escape-promise.js
+++ b/test/parallel/test-vm-timeout-escape-promise.js
@@ -17,7 +17,7 @@ function loop() {
   while (1) {
     const current = hrtime();
     const span = (current - start) / NS_PER_MS;
-    if (span >= 100n) {
+    if (span >= 2000n) {
       throw new Error(
         `escaped timeout at ${span} milliseconds!`);
     }

From 36b60c6566d870edb42921efbd516ca2fee1944a Mon Sep 17 00:00:00 2001
From: Joyee Cheung <joyeec9h3@gmail.com>
Date: Thu, 25 Aug 2022 18:08:53 +0800
Subject: [PATCH 6/7] test: deflake child process exec timeout tests

On Windows it might take too long for the parent to start the
communication with a child process, so by the time the parent
starts its own timer, the child process might have already
completed running, and the parent in those tests won't have a
chance to terminate these child processes after the timeout.
To address this issue, raise the time for which the child is
supposed to run to make sure that the parent starts
its own timer before the child terminates in the tests.
Also, split the test into smaller ones to reduce the overhead.

PR-URL: https://github.com/nodejs/node/pull/44390
Refs: https://github.com/nodejs/reliability/issues/356
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
---
 test/common/child_process.js                  | 45 ++++++++++++
 .../test-child-process-exec-timeout-expire.js | 50 +++++++++++++
 .../test-child-process-exec-timeout-kill.js   | 39 ++++++++++
 ...-child-process-exec-timeout-not-expired.js | 34 +++++++++
 .../test-child-process-exec-timeout.js        | 73 -------------------
 5 files changed, 168 insertions(+), 73 deletions(-)
 create mode 100644 test/common/child_process.js
 create mode 100644 test/parallel/test-child-process-exec-timeout-expire.js
 create mode 100644 test/parallel/test-child-process-exec-timeout-kill.js
 create mode 100644 test/parallel/test-child-process-exec-timeout-not-expired.js
 delete mode 100644 test/parallel/test-child-process-exec-timeout.js

diff --git a/test/common/child_process.js b/test/common/child_process.js
new file mode 100644
index 00000000000000..30669821bf4165
--- /dev/null
+++ b/test/common/child_process.js
@@ -0,0 +1,45 @@
+'use strict';
+
+const assert = require('assert');
+const common = require('./');
+
+// Workaround for Windows Server 2008R2
+// When CMD is used to launch a process and CMD is killed too quickly, the
+// process can stay behind running in suspended state, never completing.
+function cleanupStaleProcess(filename) {
+  if (!common.isWindows) {
+    return;
+  }
+  process.once('beforeExit', () => {
+    const basename = filename.replace(/.*[/\\]/g, '');
+    require('child_process')
+      .execFileSync(`${process.env.SystemRoot}\\System32\\wbem\\WMIC.exe`, [
+        'process',
+        'where',
+        `commandline like '%${basename}%child'`,
+        'delete',
+        '/nointeractive',
+      ]);
+  });
+}
+
+// This should keep the child process running long enough to expire
+// the timeout.
+const kExpiringChildRunTime = common.platformTimeout(20 * 1000);
+const kExpiringParentTimer = 1;
+assert(kExpiringChildRunTime > kExpiringParentTimer);
+
+function logAfterTime(time) {
+  setTimeout(() => {
+    // The following console statements are part of the test.
+    console.log('child stdout');
+    console.error('child stderr');
+  }, time);
+}
+
+module.exports = {
+  cleanupStaleProcess,
+  logAfterTime,
+  kExpiringChildRunTime,
+  kExpiringParentTimer
+};
diff --git a/test/parallel/test-child-process-exec-timeout-expire.js b/test/parallel/test-child-process-exec-timeout-expire.js
new file mode 100644
index 00000000000000..08e54544836d98
--- /dev/null
+++ b/test/parallel/test-child-process-exec-timeout-expire.js
@@ -0,0 +1,50 @@
+'use strict';
+
+// Test exec() with a timeout that expires.
+
+const common = require('../common');
+const assert = require('assert');
+const cp = require('child_process');
+
+const {
+  cleanupStaleProcess,
+  logAfterTime,
+  kExpiringChildRunTime,
+  kExpiringParentTimer
+} = require('../common/child_process');
+
+if (process.argv[2] === 'child') {
+  logAfterTime(kExpiringChildRunTime);
+  return;
+}
+
+const cmd = `"${process.execPath}" "${__filename}" child`;
+
+cp.exec(cmd, {
+  timeout: kExpiringParentTimer,
+}, common.mustCall((err, stdout, stderr) => {
+  console.log('[stdout]', stdout.trim());
+  console.log('[stderr]', stderr.trim());
+
+  let sigterm = 'SIGTERM';
+  assert.strictEqual(err.killed, true);
+  // TODO OpenBSD returns a null signal and 143 for code
+  if (common.isOpenBSD) {
+    assert.strictEqual(err.code, 143);
+    sigterm = null;
+  } else {
+    assert.strictEqual(err.code, null);
+  }
+  // At least starting with Darwin Kernel Version 16.4.0, sending a SIGTERM to a
+  // process that is still starting up kills it with SIGKILL instead of SIGTERM.
+  // See: https://github.com/libuv/libuv/issues/1226
+  if (common.isOSX)
+    assert.ok(err.signal === 'SIGTERM' || err.signal === 'SIGKILL');
+  else
+    assert.strictEqual(err.signal, sigterm);
+  assert.strictEqual(err.cmd, cmd);
+  assert.strictEqual(stdout.trim(), '');
+  assert.strictEqual(stderr.trim(), '');
+}));
+
+cleanupStaleProcess(__filename);
diff --git a/test/parallel/test-child-process-exec-timeout-kill.js b/test/parallel/test-child-process-exec-timeout-kill.js
new file mode 100644
index 00000000000000..0be04e8607f3d0
--- /dev/null
+++ b/test/parallel/test-child-process-exec-timeout-kill.js
@@ -0,0 +1,39 @@
+'use strict';
+
+// Test exec() with both a timeout and a killSignal.
+
+const common = require('../common');
+const assert = require('assert');
+const cp = require('child_process');
+
+const {
+  cleanupStaleProcess,
+  logInTimeout,
+  kExpiringChildRunTime,
+  kExpiringParentTimer,
+} = require('../common/child_process');
+
+if (process.argv[2] === 'child') {
+  logInTimeout(kExpiringChildRunTime);
+  return;
+}
+
+const cmd = `"${process.execPath}" "${__filename}" child`;
+
+// Test with a different kill signal.
+cp.exec(cmd, {
+  timeout: kExpiringParentTimer,
+  killSignal: 'SIGKILL'
+}, common.mustCall((err, stdout, stderr) => {
+  console.log('[stdout]', stdout.trim());
+  console.log('[stderr]', stderr.trim());
+
+  assert.strictEqual(err.killed, true);
+  assert.strictEqual(err.code, null);
+  assert.strictEqual(err.signal, 'SIGKILL');
+  assert.strictEqual(err.cmd, cmd);
+  assert.strictEqual(stdout.trim(), '');
+  assert.strictEqual(stderr.trim(), '');
+}));
+
+cleanupStaleProcess(__filename);
diff --git a/test/parallel/test-child-process-exec-timeout-not-expired.js b/test/parallel/test-child-process-exec-timeout-not-expired.js
new file mode 100644
index 00000000000000..fb0af5fa8f59d5
--- /dev/null
+++ b/test/parallel/test-child-process-exec-timeout-not-expired.js
@@ -0,0 +1,34 @@
+'use strict';
+
+// Test exec() when a timeout is set, but not expired.
+
+const common = require('../common');
+const assert = require('assert');
+const cp = require('child_process');
+
+const {
+  cleanupStaleProcess,
+  logAfterTime
+} = require('../common/child_process');
+
+const kTimeoutNotSupposedToExpire = 2 ** 30;
+const childRunTime = common.platformTimeout(100);
+
+// The time spent in the child should be smaller than the timeout below.
+assert(childRunTime < kTimeoutNotSupposedToExpire);
+
+if (process.argv[2] === 'child') {
+  logAfterTime(childRunTime);
+  return;
+}
+
+const cmd = `"${process.execPath}" "${__filename}" child`;
+
+cp.exec(cmd, {
+  timeout: kTimeoutNotSupposedToExpire
+}, common.mustSucceed((stdout, stderr) => {
+  assert.strictEqual(stdout.trim(), 'child stdout');
+  assert.strictEqual(stderr.trim(), 'child stderr');
+}));
+
+cleanupStaleProcess(__filename);
diff --git a/test/parallel/test-child-process-exec-timeout.js b/test/parallel/test-child-process-exec-timeout.js
deleted file mode 100644
index 64648d62bb3284..00000000000000
--- a/test/parallel/test-child-process-exec-timeout.js
+++ /dev/null
@@ -1,73 +0,0 @@
-'use strict';
-const common = require('../common');
-const assert = require('assert');
-const cp = require('child_process');
-
-if (process.argv[2] === 'child') {
-  setTimeout(() => {
-    // The following console statements are part of the test.
-    console.log('child stdout');
-    console.error('child stderr');
-  }, common.platformTimeout(1000));
-  return;
-}
-
-const cmd = `"${process.execPath}" "${__filename}" child`;
-
-// Test the case where a timeout is set, and it expires.
-cp.exec(cmd, { timeout: 1 }, common.mustCall((err, stdout, stderr) => {
-  let sigterm = 'SIGTERM';
-  assert.strictEqual(err.killed, true);
-  // TODO OpenBSD returns a null signal and 143 for code
-  if (common.isOpenBSD) {
-    assert.strictEqual(err.code, 143);
-    sigterm = null;
-  } else {
-    assert.strictEqual(err.code, null);
-  }
-  // At least starting with Darwin Kernel Version 16.4.0, sending a SIGTERM to a
-  // process that is still starting up kills it with SIGKILL instead of SIGTERM.
-  // See: https://github.com/libuv/libuv/issues/1226
-  if (common.isOSX)
-    assert.ok(err.signal === 'SIGTERM' || err.signal === 'SIGKILL');
-  else
-    assert.strictEqual(err.signal, sigterm);
-  assert.strictEqual(err.cmd, cmd);
-  assert.strictEqual(stdout.trim(), '');
-  assert.strictEqual(stderr.trim(), '');
-}));
-
-// Test with a different kill signal.
-cp.exec(cmd, {
-  timeout: 1,
-  killSignal: 'SIGKILL'
-}, common.mustCall((err, stdout, stderr) => {
-  assert.strictEqual(err.killed, true);
-  assert.strictEqual(err.code, null);
-  assert.strictEqual(err.signal, 'SIGKILL');
-  assert.strictEqual(err.cmd, cmd);
-  assert.strictEqual(stdout.trim(), '');
-  assert.strictEqual(stderr.trim(), '');
-}));
-
-// Test the case where a timeout is set, but not expired.
-cp.exec(cmd, { timeout: 2 ** 30 }, common.mustSucceed((stdout, stderr) => {
-  assert.strictEqual(stdout.trim(), 'child stdout');
-  assert.strictEqual(stderr.trim(), 'child stderr');
-}));
-
-// Workaround for Windows Server 2008R2
-// When CMD is used to launch a process and CMD is killed too quickly, the
-// process can stay behind running in suspended state, never completing.
-if (common.isWindows) {
-  process.once('beforeExit', () => {
-    const basename = __filename.replace(/.*[/\\]/g, '');
-    cp.execFileSync(`${process.env.SystemRoot}\\System32\\wbem\\WMIC.exe`, [
-      'process',
-      'where',
-      `commandline like '%${basename}%child'`,
-      'delete',
-      '/nointeractive',
-    ]);
-  });
-}

From 5aad6912d4eafb8a1e15bbf5683f30f546481ad0 Mon Sep 17 00:00:00 2001
From: Joyee Cheung <joyeec9h3@gmail.com>
Date: Thu, 25 Aug 2022 02:16:56 +0800
Subject: [PATCH 7/7] test: avoid race in file write stream handle tests

The test previously created two fs.promises.open calls on the
same file with w+ back-to-back, and one of them could fail
when checking the contents of that file if the other happened
to be opening the file for write. Split them into different
tests (with different tmpdir) to avoid the race.

PR-URL: https://github.com/nodejs/node/pull/44380
Refs: https://github.com/nodejs/reliability/issues/354
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
---
 .../test-fs-write-stream-file-handle-2.js     | 33 +++++++++++++++++++
 .../test-fs-write-stream-file-handle.js       | 23 -------------
 2 files changed, 33 insertions(+), 23 deletions(-)
 create mode 100644 test/parallel/test-fs-write-stream-file-handle-2.js

diff --git a/test/parallel/test-fs-write-stream-file-handle-2.js b/test/parallel/test-fs-write-stream-file-handle-2.js
new file mode 100644
index 00000000000000..25f68b14da70b3
--- /dev/null
+++ b/test/parallel/test-fs-write-stream-file-handle-2.js
@@ -0,0 +1,33 @@
+'use strict';
+const common = require('../common');
+const fs = require('fs');
+const path = require('path');
+const assert = require('assert');
+const tmpdir = require('../common/tmpdir');
+const file = path.join(tmpdir.path, 'write_stream_filehandle_test.txt');
+const input = 'hello world';
+
+tmpdir.refresh();
+
+fs.promises.open(file, 'w+').then((handle) => {
+  let calls = 0;
+  const {
+    write: originalWriteFunction,
+    writev: originalWritevFunction
+  } = handle;
+  handle.write = function write() {
+    calls++;
+    return Reflect.apply(originalWriteFunction, this, arguments);
+  };
+  handle.writev = function writev() {
+    calls++;
+    return Reflect.apply(originalWritevFunction, this, arguments);
+  };
+  const stream = fs.createWriteStream(null, { fd: handle });
+
+  stream.end(input);
+  stream.on('close', common.mustCall(() => {
+    assert(calls > 0, 'expected at least one call to fileHandle.write or ' +
+    'fileHandle.writev, got 0');
+  }));
+}).then(common.mustCall());
diff --git a/test/parallel/test-fs-write-stream-file-handle.js b/test/parallel/test-fs-write-stream-file-handle.js
index 23ddf21c50f3f2..01c490ef5d0da1 100644
--- a/test/parallel/test-fs-write-stream-file-handle.js
+++ b/test/parallel/test-fs-write-stream-file-handle.js
@@ -19,26 +19,3 @@ fs.promises.open(file, 'w+').then((handle) => {
     assert.strictEqual(output, input);
   }));
 }).then(common.mustCall());
-
-fs.promises.open(file, 'w+').then((handle) => {
-  let calls = 0;
-  const {
-    write: originalWriteFunction,
-    writev: originalWritevFunction
-  } = handle;
-  handle.write = function write() {
-    calls++;
-    return Reflect.apply(originalWriteFunction, this, arguments);
-  };
-  handle.writev = function writev() {
-    calls++;
-    return Reflect.apply(originalWritevFunction, this, arguments);
-  };
-  const stream = fs.createWriteStream(null, { fd: handle });
-
-  stream.end(input);
-  stream.on('close', common.mustCall(() => {
-    assert(calls > 0, 'expected at least one call to fileHandle.write or ' +
-    'fileHandle.writev, got 0');
-  }));
-}).then(common.mustCall());