From dcbbcd23f5dd8601e2851aded4cabc6557164363 Mon Sep 17 00:00:00 2001
From: Matt Mastracci <matthew@mastracci.com>
Date: Fri, 9 Feb 2024 13:33:05 -0700
Subject: [PATCH] refactor: split integration tests from CLI (part 1) (#22308)

This PR separates integration tests from CLI tests into a new project
named `cli_tests`. This is a prerequisite for an integration test runner
that can work with either the CLI binary in the current project, or one
that is built ahead of time.

## Background

Rust does not have the concept of artifact dependencies yet
(https://github.com/rust-lang/cargo/issues/9096). Because of this, the
only way we can ensure a binary is built before running associated tests
is by hanging tests off the crate with the binary itself.

Unfortunately this means that to run those tests, you _must_ build the
binary and in the case of the deno executable that might be a 10 minute
wait in release mode.

## Implementation

To allow for tests to run with and without the requirement that the
binary is up-to-date, we split the integration tests into a project of
their own. As these tests would not require the binary to build itself
before being run as-is, we add a stub integration `[[test]]` target in
the `cli` project that invokes these tests using `cargo test`.

The stub test runner we add has `harness = false` so that we can get
access to a `main` function. This `main` function's sole job is to
`execvp` the command `cargo test -p deno_cli`, effectively "calling"
another cargo target.

This ensures that the deno executable is always correctly rebuilt before
running the stub test runner from `cli`, and gets us closer to be able
to run the entire integration test suite on arbitrary deno executables
(and therefore split the build into multiple phases).

The new `cli_tests` project lives within `cli` to avoid a large PR. In
later PRs, the test data will be split from the `cli` project. As there
are a few thousand files, it'll be better to do this as a completely
separate PR to avoid noise.
---
 .github/workflows/ci.generate.ts         |  2 +-
 .github/workflows/ci.yml                 |  2 +-
 Cargo.lock                               | 75 ++++++++++++++++++++----
 Cargo.toml                               |  1 +
 cli/Cargo.toml                           | 17 ++----
 cli/tests/Cargo.toml                     | 51 ++++++++++++++++
 cli/tests/integration/cert_tests.rs      |  8 +--
 cli/tests/integration/inspector_tests.rs |  2 +-
 cli/tests/integration/js_unit_tests.rs   |  2 +-
 cli/tests/integration/node_unit_tests.rs |  2 +-
 cli/tests/integration/run_tests.rs       |  2 +-
 cli/tests/integration_tests.rs           |  1 +
 cli/tests/integration_tests_runner.rs    | 18 ++++++
 cli/tests/lib.rs                         |  1 +
 ext/tls/Cargo.toml                       |  1 +
 ext/tls/lib.rs                           |  1 +
 test_util/Cargo.toml                     |  1 +
 test_util/src/lib.rs                     |  1 +
 test_util/src/spawn.rs                   | 71 ++++++++++++++++++++++
 19 files changed, 228 insertions(+), 31 deletions(-)
 create mode 100644 cli/tests/Cargo.toml
 create mode 100644 cli/tests/integration_tests_runner.rs
 create mode 100644 cli/tests/lib.rs
 create mode 100644 test_util/src/spawn.rs

diff --git a/.github/workflows/ci.generate.ts b/.github/workflows/ci.generate.ts
index e47435b7e0..26ca893162 100755
--- a/.github/workflows/ci.generate.ts
+++ b/.github/workflows/ci.generate.ts
@@ -791,7 +791,7 @@ const ci = {
             // Run unit then integration tests. Skip doc tests here
             // since they are sometimes very slow on Mac.
             "cargo test --locked --lib",
-            "cargo test --locked --test '*'",
+            "cargo test --locked --tests",
           ].join("\n"),
           env: { CARGO_PROFILE_DEV_DEBUG: 0 },
         },
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 63a87d0595..79edd24c56 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -480,7 +480,7 @@ jobs:
           (startsWith(github.ref, 'refs/tags/') || matrix.os != 'linux'))
         run: |-
           cargo test --locked --lib
-          cargo test --locked --test '*'
+          cargo test --locked --tests
         env:
           CARGO_PROFILE_DEV_DEBUG: 0
       - name: Test (release)
diff --git a/Cargo.lock b/Cargo.lock
index 279efee968..a935bda405 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -607,6 +607,38 @@ version = "0.6.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "702fc72eb24e5a1e48ce58027a675bc24edd52096d5397d4aea7c6dd9eca0bd1"
 
+[[package]]
+name = "cli_tests"
+version = "0.0.0"
+dependencies = [
+ "bytes",
+ "deno_ast",
+ "deno_bench_util",
+ "deno_core",
+ "deno_fetch",
+ "deno_lockfile",
+ "deno_tls",
+ "fastwebsockets",
+ "flaky_test",
+ "http 1.0.0",
+ "http-body-util",
+ "hyper 1.1.0",
+ "hyper-util",
+ "nix 0.26.2",
+ "once_cell",
+ "os_pipe",
+ "pretty_assertions",
+ "serde",
+ "serde_repr",
+ "test_util",
+ "tokio",
+ "tokio-util",
+ "tower-lsp",
+ "trust-dns-client",
+ "trust-dns-server",
+ "url",
+]
+
 [[package]]
 name = "clipboard-win"
 version = "5.0.0"
@@ -1005,18 +1037,12 @@ dependencies = [
  "env_logger",
  "eszip",
  "fancy-regex",
- "fastwebsockets",
- "flaky_test",
  "flate2",
  "fs3",
  "fwdansi",
  "glibc_version",
  "glob",
  "hex",
- "http 1.0.0",
- "http-body-util",
- "hyper 1.1.0",
- "hyper-util",
  "import_map",
  "indexmap",
  "jsonc-parser",
@@ -1057,8 +1083,6 @@ dependencies = [
  "tokio",
  "tokio-util",
  "tower-lsp",
- "trust-dns-client",
- "trust-dns-server",
  "twox-hash",
  "typed-arena",
  "unicode-width",
@@ -1776,6 +1800,7 @@ dependencies = [
  "once_cell",
  "rustls",
  "rustls-pemfile",
+ "rustls-tokio-stream",
  "rustls-webpki",
  "serde",
  "webpki-roots",
@@ -2838,7 +2863,7 @@ dependencies = [
  "presser",
  "thiserror",
  "winapi",
- "windows",
+ "windows 0.51.1",
 ]
 
 [[package]]
@@ -6309,6 +6334,7 @@ dependencies = [
  "termcolor",
  "tokio",
  "url",
+ "win32job",
  "winapi",
 ]
 
@@ -7178,6 +7204,16 @@ version = "1.0.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "653f141f39ec16bba3c5abe400a0c60da7468261cc2cbf36805022876bc721a8"
 
+[[package]]
+name = "win32job"
+version = "2.0.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "5b2b1bf557d947847a30eb73f79aa6cdb3eaf3ce02f5e9599438f77896a62b3c"
+dependencies = [
+ "thiserror",
+ "windows 0.52.0",
+]
+
 [[package]]
 name = "winapi"
 version = "0.3.9"
@@ -7215,10 +7251,20 @@ version = "0.51.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "ca229916c5ee38c2f2bc1e9d8f04df975b4bd93f9955dc69fabb5d91270045c9"
 dependencies = [
- "windows-core",
+ "windows-core 0.51.1",
  "windows-targets 0.48.5",
 ]
 
+[[package]]
+name = "windows"
+version = "0.52.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "e48a53791691ab099e5e2ad123536d0fff50652600abaf43bbf952894110d0be"
+dependencies = [
+ "windows-core 0.52.0",
+ "windows-targets 0.52.0",
+]
+
 [[package]]
 name = "windows-core"
 version = "0.51.1"
@@ -7228,6 +7274,15 @@ dependencies = [
  "windows-targets 0.48.5",
 ]
 
+[[package]]
+name = "windows-core"
+version = "0.52.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "33ab640c8d7e35bf8ba19b884ba838ceb4fba93a4e8c65a9059d08afcfc683d9"
+dependencies = [
+ "windows-targets 0.52.0",
+]
+
 [[package]]
 name = "windows-sys"
 version = "0.48.0"
diff --git a/Cargo.toml b/Cargo.toml
index 505364efe7..f53ceef2db 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -6,6 +6,7 @@ members = [
   "bench_util",
   "cli",
   "cli/napi/sym",
+  "cli/tests",
   "ext/broadcast_channel",
   "ext/cache",
   "ext/canvas",
diff --git a/cli/Cargo.toml b/cli/Cargo.toml
index f9e59abc3a..cb52d3966c 100644
--- a/cli/Cargo.toml
+++ b/cli/Cargo.toml
@@ -4,6 +4,7 @@
 name = "deno"
 version = "1.40.4"
 authors.workspace = true
+autotests = false
 default-run = "deno"
 edition.workspace = true
 exclude = ["tests/testdata/npm/registry/*"]
@@ -16,6 +17,11 @@ name = "deno"
 path = "main.rs"
 doc = false
 
+[[test]]
+name = "integration"
+path = "tests/integration_tests_runner.rs"
+harness = false
+
 [[bench]]
 name = "deno_bench"
 harness = false
@@ -149,19 +155,8 @@ nix.workspace = true
 
 [dev-dependencies]
 deno_bench_util.workspace = true
-deno_core = { workspace = true, features = ["include_js_files_for_snapshotting", "unsafe_use_unprotected_platform"] }
-fastwebsockets = { workspace = true, features = ["upgrade", "unstable-split"] }
-flaky_test = "=0.1.0"
-http.workspace = true
-http-body-util.workspace = true
-hyper.workspace = true
-hyper-util.workspace = true
-once_cell.workspace = true
-os_pipe.workspace = true
 pretty_assertions.workspace = true
 test_util.workspace = true
-trust-dns-client = "=0.22.0"
-trust-dns-server = "=0.22.1"
 
 [package.metadata.winres]
 # This section defines the metadata that appears in the deno.exe PE header.
diff --git a/cli/tests/Cargo.toml b/cli/tests/Cargo.toml
new file mode 100644
index 0000000000..6ad5984e2e
--- /dev/null
+++ b/cli/tests/Cargo.toml
@@ -0,0 +1,51 @@
+# Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
+
+[package]
+name = "cli_tests"
+version = "0.0.0"
+authors.workspace = true
+autotests = false
+edition.workspace = true
+license.workspace = true
+repository.workspace = true
+
+[lib]
+path = "lib.rs"
+
+[features]
+run = []
+
+[[test]]
+name = "integration_tests"
+path = "integration_tests.rs"
+required-features = ["run"]
+
+[dev-dependencies]
+bytes.workspace = true
+deno_ast.workspace = true
+deno_bench_util.workspace = true
+deno_core = { workspace = true, features = ["include_js_files_for_snapshotting", "unsafe_use_unprotected_platform"] }
+deno_fetch.workspace = true
+deno_lockfile.workspace = true
+deno_tls.workspace = true
+fastwebsockets = { workspace = true, features = ["upgrade", "unstable-split"] }
+flaky_test = "=0.1.0"
+http.workspace = true
+http-body-util.workspace = true
+hyper.workspace = true
+hyper-util.workspace = true
+once_cell.workspace = true
+os_pipe.workspace = true
+pretty_assertions.workspace = true
+serde.workspace = true
+serde_repr.workspace = true
+test_util.workspace = true
+tokio.workspace = true
+tokio-util.workspace = true
+tower-lsp.workspace = true
+trust-dns-client = "=0.22.0"
+trust-dns-server = "=0.22.1"
+url.workspace = true
+
+[target.'cfg(unix)'.dev-dependencies]
+nix.workspace = true
diff --git a/cli/tests/integration/cert_tests.rs b/cli/tests/integration/cert_tests.rs
index 484d053f81..9e89a9f3ed 100644
--- a/cli/tests/integration/cert_tests.rs
+++ b/cli/tests/integration/cert_tests.rs
@@ -1,14 +1,14 @@
 // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
 
-use deno_runtime::deno_net::ops_tls::TlsStream;
-use deno_runtime::deno_tls::rustls;
-use deno_runtime::deno_tls::rustls_pemfile;
-use lsp_types::Url;
+use deno_tls::rustls;
+use deno_tls::rustls_pemfile;
+use deno_tls::rustls_tokio_stream::TlsStream;
 use std::io::BufReader;
 use std::io::Cursor;
 use std::io::Read;
 use std::sync::Arc;
 use test_util as util;
+use url::Url;
 use util::testdata_path;
 use util::TestContext;
 
diff --git a/cli/tests/integration/inspector_tests.rs b/cli/tests/integration/inspector_tests.rs
index 872f9e3d2d..bbe70ae5e0 100644
--- a/cli/tests/integration/inspector_tests.rs
+++ b/cli/tests/integration/inspector_tests.rs
@@ -6,7 +6,7 @@ use deno_core::error::AnyError;
 use deno_core::serde_json;
 use deno_core::serde_json::json;
 use deno_core::url;
-use deno_runtime::deno_fetch::reqwest;
+use deno_fetch::reqwest;
 use fastwebsockets::FragmentCollector;
 use fastwebsockets::Frame;
 use fastwebsockets::WebSocket;
diff --git a/cli/tests/integration/js_unit_tests.rs b/cli/tests/integration/js_unit_tests.rs
index 748c0fe5a6..0e3a1a1182 100644
--- a/cli/tests/integration/js_unit_tests.rs
+++ b/cli/tests/integration/js_unit_tests.rs
@@ -7,7 +7,7 @@ use test_util as util;
 
 util::unit_test_factory!(
   js_unit_test,
-  "tests/unit",
+  "../tests/unit",
   "*.ts",
   [
     abort_controller_test,
diff --git a/cli/tests/integration/node_unit_tests.rs b/cli/tests/integration/node_unit_tests.rs
index b0259663d6..5afaf48c55 100644
--- a/cli/tests/integration/node_unit_tests.rs
+++ b/cli/tests/integration/node_unit_tests.rs
@@ -8,7 +8,7 @@ use util::env_vars_for_npm_tests;
 
 util::unit_test_factory!(
   node_unit_test,
-  "tests/unit_node",
+  "../tests/unit_node",
   "**/*_test.ts",
   [
     _fs_access_test = _fs / _fs_access_test,
diff --git a/cli/tests/integration/run_tests.rs b/cli/tests/integration/run_tests.rs
index 3d14bb0bb6..428de32389 100644
--- a/cli/tests/integration/run_tests.rs
+++ b/cli/tests/integration/run_tests.rs
@@ -3,7 +3,7 @@
 use bytes::Bytes;
 use deno_core::serde_json::json;
 use deno_core::url;
-use deno_runtime::deno_fetch::reqwest;
+use deno_fetch::reqwest;
 use pretty_assertions::assert_eq;
 use std::io::Read;
 use std::io::Write;
diff --git a/cli/tests/integration_tests.rs b/cli/tests/integration_tests.rs
index 967cf6afea..8469b5416a 100644
--- a/cli/tests/integration_tests.rs
+++ b/cli/tests/integration_tests.rs
@@ -3,4 +3,5 @@
 // The tests exist in a sub folder instead of as separate files in
 // this directory so that cargo doesn't compile each file as a new crate.
 
+#[cfg(test)]
 mod integration;
diff --git a/cli/tests/integration_tests_runner.rs b/cli/tests/integration_tests_runner.rs
new file mode 100644
index 0000000000..12e83a0194
--- /dev/null
+++ b/cli/tests/integration_tests_runner.rs
@@ -0,0 +1,18 @@
+// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
+pub fn main() {
+  let mut args = vec!["cargo", "test", "-p", "cli_tests", "--features", "run"];
+
+  if !cfg!(debug_assertions) {
+    args.push("--release");
+  }
+
+  args.push("--");
+
+  // If any args were passed to this process, pass them through to the child
+  let orig_args = std::env::args().skip(1).collect::<Vec<_>>();
+  let orig_args: Vec<&str> =
+    orig_args.iter().map(|x| x.as_ref()).collect::<Vec<_>>();
+  args.extend(orig_args);
+
+  test_util::spawn::exec_replace("cargo", &args).unwrap();
+}
diff --git a/cli/tests/lib.rs b/cli/tests/lib.rs
new file mode 100644
index 0000000000..0a39b9f87d
--- /dev/null
+++ b/cli/tests/lib.rs
@@ -0,0 +1 @@
+// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
diff --git a/ext/tls/Cargo.toml b/ext/tls/Cargo.toml
index 675a979da2..34a34f3387 100644
--- a/ext/tls/Cargo.toml
+++ b/ext/tls/Cargo.toml
@@ -19,6 +19,7 @@ deno_native_certs = "0.2.0"
 once_cell.workspace = true
 rustls = { workspace = true, features = ["dangerous_configuration"] }
 rustls-pemfile.workspace = true
+rustls-tokio-stream.workspace = true
 rustls-webpki.workspace = true
 serde.workspace = true
 webpki-roots.workspace = true
diff --git a/ext/tls/lib.rs b/ext/tls/lib.rs
index 6ea4a68294..9ed8a5a1f5 100644
--- a/ext/tls/lib.rs
+++ b/ext/tls/lib.rs
@@ -3,6 +3,7 @@
 pub use deno_native_certs;
 pub use rustls;
 pub use rustls_pemfile;
+pub use rustls_tokio_stream;
 pub use webpki;
 pub use webpki_roots;
 
diff --git a/test_util/Cargo.toml b/test_util/Cargo.toml
index 178dd5d543..b2ff102779 100644
--- a/test_util/Cargo.toml
+++ b/test_util/Cargo.toml
@@ -53,6 +53,7 @@ tempfile.workspace = true
 termcolor.workspace = true
 tokio.workspace = true
 url.workspace = true
+win32job = "2"
 
 [target.'cfg(windows)'.dependencies]
 winapi = { workspace = true, features = ["consoleapi", "synchapi", "handleapi", "namedpipeapi", "winbase", "winerror"] }
diff --git a/test_util/src/lib.rs b/test_util/src/lib.rs
index 9bf68cbc2d..dda62a5914 100644
--- a/test_util/src/lib.rs
+++ b/test_util/src/lib.rs
@@ -32,6 +32,7 @@ pub mod lsp;
 mod npm;
 pub mod pty;
 pub mod servers;
+pub mod spawn;
 
 pub use builders::DenoChild;
 pub use builders::TestCommandBuilder;
diff --git a/test_util/src/spawn.rs b/test_util/src/spawn.rs
new file mode 100644
index 0000000000..bfd83e9b26
--- /dev/null
+++ b/test_util/src/spawn.rs
@@ -0,0 +1,71 @@
+// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
+use anyhow::Error;
+use std::convert::Infallible;
+
+/// For unix targets, we just replace our current process with the desired cargo process.
+#[cfg(unix)]
+pub fn exec_replace_inner(
+  cmd: &str,
+  args: &[&str],
+) -> Result<Infallible, Error> {
+  use std::ffi::CStr;
+  use std::ffi::CString;
+
+  let args = args
+    .iter()
+    .map(|arg| CString::new(*arg).unwrap())
+    .collect::<Vec<_>>();
+  let args: Vec<&CStr> =
+    args.iter().map(|arg| arg.as_ref()).collect::<Vec<_>>();
+
+  let err = nix::unistd::execvp(&CString::new(cmd).unwrap(), &args)
+    .expect_err("Impossible");
+  Err(err.into())
+}
+
+#[cfg(windows)]
+pub fn exec_replace_inner(
+  cmd: &str,
+  args: &[&str],
+) -> Result<Infallible, Error> {
+  use std::os::windows::io::AsRawHandle;
+  use std::process::Command;
+  use win32job::ExtendedLimitInfo;
+  use win32job::Job;
+
+  // Use a job to ensure the child process's lifetime does not exceed the current process's lifetime.
+  // This ensures that if the current process is terminated (e.g., via ctrl+c or task manager),
+  // the child process is automatically reaped.
+
+  // For more information about this technique, see Raymond Chen's blog post:
+  // https://devblogs.microsoft.com/oldnewthing/20131209-00/?p=2433
+  // Note: While our implementation is not perfect, it serves its purpose for test code.
+
+  // In the future, we may directly obtain the main thread's handle from Rust code and use it
+  // to create a suspended process that we can then resume:
+  // https://github.com/rust-lang/rust/issues/96723
+
+  // Creates a child process and assigns it to our current job.
+  // A more reliable approach would be to create the child suspended and then assign it to the job.
+  // For now, we create the child, create the job, and then assign both us and the child to the job.
+  let mut child = Command::new(cmd).args(&args[1..]).spawn()?;
+
+  let mut info = ExtendedLimitInfo::default();
+  info.limit_kill_on_job_close();
+  let job = Job::create_with_limit_info(&info)?;
+  job.assign_current_process()?;
+  let handle = child.as_raw_handle();
+  job.assign_process(handle as _)?;
+
+  let exit = child.wait()?;
+  std::process::exit(exit.code().unwrap_or(1));
+}
+
+/// Runs a command, replacing the current process on Unix. On Windows, this function blocks and
+/// exits.
+///
+/// In either case, the only way this function returns is if it fails to launch the child
+/// process.
+pub fn exec_replace(command: &str, args: &[&str]) -> Result<Infallible, Error> {
+  exec_replace_inner(command, args)
+}