From 6ca316c80e8fc7f7a254eae8f82629ae5301633a Mon Sep 17 00:00:00 2001 From: Miles Johnson Date: Sat, 16 Dec 2023 17:22:13 -0800 Subject: [PATCH] new: Add tests and handle piping. (#343) --- Cargo.lock | 7 + crates/cli/Cargo.toml | 1 + crates/cli/src/main_shim.rs | 54 ++--- crates/cli/tests/fixtures/piped-data.txt | 1 + crates/cli/tests/fixtures/shim-code-0.mjs | 1 + crates/cli/tests/fixtures/shim-code-1.mjs | 1 + .../cli/tests/fixtures/shim-piped-stdin.mjs | 31 +++ crates/cli/tests/fixtures/shim-signal.mjs | 12 ++ crates/cli/tests/fixtures/shim-standard.mjs | 2 + crates/cli/tests/fixtures/shim-timeout.mjs | 7 + crates/cli/tests/fixtures/shim-tla.mjs | 10 + crates/cli/tests/shim_test.rs | 186 ++++++++++++++++++ ...st__shim_bin__handles_file_piped_data.snap | 10 + ...t__shim_bin__handles_stdin_piped_data.snap | 10 + .../shim_test__shim_bin__standard_output.snap | 8 + ...him_test__shim_bin__waits_for_timeout.snap | 9 + ...__shim_bin__waits_for_top_level_await.snap | 9 + crates/cli/tests/utils.rs | 9 +- 18 files changed, 324 insertions(+), 44 deletions(-) create mode 100644 crates/cli/tests/fixtures/piped-data.txt create mode 100644 crates/cli/tests/fixtures/shim-code-0.mjs create mode 100644 crates/cli/tests/fixtures/shim-code-1.mjs create mode 100644 crates/cli/tests/fixtures/shim-piped-stdin.mjs create mode 100644 crates/cli/tests/fixtures/shim-signal.mjs create mode 100644 crates/cli/tests/fixtures/shim-standard.mjs create mode 100644 crates/cli/tests/fixtures/shim-timeout.mjs create mode 100644 crates/cli/tests/fixtures/shim-tla.mjs create mode 100644 crates/cli/tests/shim_test.rs create mode 100644 crates/cli/tests/snapshots/shim_test__shim_bin__handles_file_piped_data.snap create mode 100644 crates/cli/tests/snapshots/shim_test__shim_bin__handles_stdin_piped_data.snap create mode 100644 crates/cli/tests/snapshots/shim_test__shim_bin__standard_output.snap create mode 100644 crates/cli/tests/snapshots/shim_test__shim_bin__waits_for_timeout.snap create mode 100644 crates/cli/tests/snapshots/shim_test__shim_bin__waits_for_top_level_await.snap diff --git a/Cargo.lock b/Cargo.lock index 06db6b35d..e8e66485d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2332,6 +2332,7 @@ dependencies = [ "semver", "serde", "shared_child", + "signal-child", "sigpipe", "starbase", "starbase_archive", @@ -2993,6 +2994,12 @@ dependencies = [ "dirs 4.0.0", ] +[[package]] +name = "signal-child" +version = "1.0.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b2a4eed4c5ae38438470ab8e0108bb751012f786f44ff585cfd837c9a5fe426f" + [[package]] name = "signal-hook-registry" version = "1.4.1" diff --git a/crates/cli/Cargo.toml b/crates/cli/Cargo.toml index 422df169a..932cf66eb 100644 --- a/crates/cli/Cargo.toml +++ b/crates/cli/Cargo.toml @@ -74,3 +74,4 @@ winreg = "0.52.0" [dev-dependencies] starbase_sandbox = { workspace = true } +signal-child = "1.0.5" diff --git a/crates/cli/src/main_shim.rs b/crates/cli/src/main_shim.rs index 8f46be978..8dd100fcc 100644 --- a/crates/cli/src/main_shim.rs +++ b/crates/cli/src/main_shim.rs @@ -7,11 +7,11 @@ use rust_json::{json_parse, JsonElem as Json}; use shared_child::SharedChild; use starbase::tracing::{self, trace, TracingOptions}; use std::collections::HashMap; -use std::io::{IsTerminal, Read, Write}; +use std::ffi::OsString; use std::path::{Path, PathBuf}; -use std::process::{Command, Stdio}; +use std::process::Command; use std::sync::Arc; -use std::{env, fs, io, process}; +use std::{env, fs, process}; fn get_proto_home() -> Result { if let Ok(root) = env::var("PROTO_HOME") { @@ -67,7 +67,7 @@ fn get_proto_binary(proto_home_dir: &Path, shim_exe_path: &Path) -> PathBuf { PathBuf::from(bin_name) } -fn create_command(args: Vec, shim_name: &str, shim_exe_path: &Path) -> Result { +fn create_command(args: Vec, shim_name: &str, shim_exe_path: &Path) -> Result { let proto_home_dir = get_proto_home()?; let registry_path = proto_home_dir.join("shims").join("registry.json"); let mut shim = Json::Object(HashMap::default()); @@ -92,13 +92,13 @@ fn create_command(args: Vec, shim_name: &str, shim_exe_path: &Path) -> R if let Json::Array(before_args) = &shim["before_args"] { for arg in before_args { if let Json::Str(arg) = arg { - passthrough_args.push(arg); + passthrough_args.push(OsString::from(arg)); } } } if args.len() > 1 { - for (i, arg) in args.iter().enumerate() { + for (i, arg) in args.into_iter().enumerate() { if i == 0 { continue; // The exe } @@ -110,7 +110,7 @@ fn create_command(args: Vec, shim_name: &str, shim_exe_path: &Path) -> R if let Json::Array(after_args) = &shim["after_args"] { for arg in after_args { if let Json::Str(arg) = arg { - passthrough_args.push(arg); + passthrough_args.push(OsString::from(arg)); } } } @@ -118,6 +118,7 @@ fn create_command(args: Vec, shim_name: &str, shim_exe_path: &Path) -> R // Create a command for local testing // let mut command = Command::new("node"); // command.arg("./docs/shim-test.mjs"); + // command.arg("--version"); // Create the command and handle alternate logic let mut command = Command::new(get_proto_binary(&proto_home_dir, shim_exe_path)); @@ -162,7 +163,7 @@ pub fn main() -> Result<()> { }); // Extract arguments to pass-through - let args = env::args().collect::>(); + let args = env::args_os().collect::>(); let exe_path = env::current_exe().unwrap_or_else(|_| PathBuf::from(&args[0])); let shim_name = exe_path @@ -179,29 +180,10 @@ pub fn main() -> Result<()> { )); } - // Capture any piped input - let input = { - let mut stdin = io::stdin(); - let mut buffer = String::new(); - - // Only read piped data when stdin is not a TTY, - // otherwise the process will hang indefinitely waiting for EOF - if !stdin.is_terminal() { - stdin.read_to_string(&mut buffer)?; - } - - buffer - }; - let has_piped_stdin = !input.is_empty(); - - // Create the actual command to execute + // Create the command to execute let mut command = create_command(args, &shim_name, &exe_path)?; command.env("PROTO_LOG", log_level); - if has_piped_stdin { - command.stdin(Stdio::piped()); - } - // Spawn a shareable child process trace!( shim = &shim_name, @@ -215,25 +197,13 @@ pub fn main() -> Result<()> { // Handle CTRL+C and kill the child ctrlc::set_handler(move || { - trace!("Received CTRL+C, killing child process"); + trace!("Received ctrl + c, killing child process"); let _ = child_clone.kill(); })?; - // If we have piped data, pass it through - if has_piped_stdin { - if let Some(mut stdin) = child.take_stdin() { - trace!( - shim = &shim_name, - input, - "Received piped input, passing through" - ); - stdin.write_all(input.as_bytes())?; - } - } - // Wait for the process to finish or be killed let status = child.wait()?; - let code = status.code().unwrap_or(0); + let code = status.code().unwrap_or(1); trace!(shim = &shim_name, code, "Received exit code"); diff --git a/crates/cli/tests/fixtures/piped-data.txt b/crates/cli/tests/fixtures/piped-data.txt new file mode 100644 index 000000000..a09776ede --- /dev/null +++ b/crates/cli/tests/fixtures/piped-data.txt @@ -0,0 +1 @@ +this data comes from a file diff --git a/crates/cli/tests/fixtures/shim-code-0.mjs b/crates/cli/tests/fixtures/shim-code-0.mjs new file mode 100644 index 000000000..517b13317 --- /dev/null +++ b/crates/cli/tests/fixtures/shim-code-0.mjs @@ -0,0 +1 @@ +process.exitCode = 0; diff --git a/crates/cli/tests/fixtures/shim-code-1.mjs b/crates/cli/tests/fixtures/shim-code-1.mjs new file mode 100644 index 000000000..6cee2e1e7 --- /dev/null +++ b/crates/cli/tests/fixtures/shim-code-1.mjs @@ -0,0 +1 @@ +process.exit(1); diff --git a/crates/cli/tests/fixtures/shim-piped-stdin.mjs b/crates/cli/tests/fixtures/shim-piped-stdin.mjs new file mode 100644 index 000000000..a86beb56e --- /dev/null +++ b/crates/cli/tests/fixtures/shim-piped-stdin.mjs @@ -0,0 +1,31 @@ +console.log("start"); + +async function getStdinBuffer() { + if (process.stdin.isTTY) { + return Buffer.alloc(0); + } + + const result = []; + let length = 0; + + for await (const chunk of process.stdin) { + result.push(chunk); + length += chunk.length; + } + + return Buffer.concat(result, length); +} + +getStdinBuffer().then((buffer) => { + let data = buffer.toString("utf8"); + + if (data) { + console.log("piped data =", data.trim()); + } +}); + +setTimeout(() => { + console.log("stop"); +}, 2500); + +console.log("running"); diff --git a/crates/cli/tests/fixtures/shim-signal.mjs b/crates/cli/tests/fixtures/shim-signal.mjs new file mode 100644 index 000000000..0beb238a4 --- /dev/null +++ b/crates/cli/tests/fixtures/shim-signal.mjs @@ -0,0 +1,12 @@ +console.log("start"); + +process.on("SIGINT", () => { + console.log("killed"); + process.exit(1); +}); + +setTimeout(() => { + console.log("stop"); +}, 5000); + +console.log("running"); diff --git a/crates/cli/tests/fixtures/shim-standard.mjs b/crates/cli/tests/fixtures/shim-standard.mjs new file mode 100644 index 000000000..591eeb533 --- /dev/null +++ b/crates/cli/tests/fixtures/shim-standard.mjs @@ -0,0 +1,2 @@ +console.log("stdout"); +console.error("stderr"); diff --git a/crates/cli/tests/fixtures/shim-timeout.mjs b/crates/cli/tests/fixtures/shim-timeout.mjs new file mode 100644 index 000000000..265cf4d59 --- /dev/null +++ b/crates/cli/tests/fixtures/shim-timeout.mjs @@ -0,0 +1,7 @@ +console.log("start"); + +setTimeout(() => { + console.log("stop"); +}, 2500); + +console.log("running"); diff --git a/crates/cli/tests/fixtures/shim-tla.mjs b/crates/cli/tests/fixtures/shim-tla.mjs new file mode 100644 index 000000000..09ed9189c --- /dev/null +++ b/crates/cli/tests/fixtures/shim-tla.mjs @@ -0,0 +1,10 @@ +console.log("start"); + +await new Promise((resolve) => { + setTimeout(() => { + console.log("running"); + resolve(); + }, 2500); +}); + +console.log("stop"); diff --git a/crates/cli/tests/shim_test.rs b/crates/cli/tests/shim_test.rs new file mode 100644 index 000000000..cd1dbabc3 --- /dev/null +++ b/crates/cli/tests/shim_test.rs @@ -0,0 +1,186 @@ +mod utils; + +use starbase_sandbox::{assert_snapshot, get_assert_output}; +use std::path::PathBuf; +use utils::*; + +mod shim_bin { + use super::*; + + fn get_fixture(name: &str) -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")).join(name) + } + + #[test] + fn standard_output() { + let sandbox = create_empty_sandbox(); + + let mut cmd = create_proto_command(sandbox.path()); + cmd.arg("install") + .arg("node") + .arg("--pin") + .arg("--") + .arg("--no-bundled-npm") + .assert() + .success(); + + let mut shim = create_shim_command(sandbox.path(), "node"); + shim.arg(get_fixture("tests/fixtures/shim-standard.mjs")); + shim.env_remove("PROTO_LOG"); + + let assert = shim.assert(); + + assert_snapshot!(get_assert_output(&assert)); + } + + #[test] + fn waits_for_timeout() { + let sandbox = create_empty_sandbox(); + + let mut cmd = create_proto_command(sandbox.path()); + cmd.arg("install") + .arg("node") + .arg("--pin") + .arg("--") + .arg("--no-bundled-npm") + .assert() + .success(); + + let mut shim = create_shim_command(sandbox.path(), "node"); + shim.arg(get_fixture("tests/fixtures/shim-timeout.mjs")); + shim.env_remove("PROTO_LOG"); + + let assert = shim.assert(); + + assert_snapshot!(get_assert_output(&assert)); + } + + #[test] + fn waits_for_top_level_await() { + let sandbox = create_empty_sandbox(); + + let mut cmd = create_proto_command(sandbox.path()); + cmd.arg("install") + .arg("node") + .arg("--pin") + .arg("--") + .arg("--no-bundled-npm") + .assert() + .success(); + + let mut shim = create_shim_command(sandbox.path(), "node"); + shim.arg(get_fixture("tests/fixtures/shim-tla.mjs")); + shim.env_remove("PROTO_LOG"); + + let assert = shim.assert(); + + assert_snapshot!(get_assert_output(&assert)); + } + + #[test] + fn handles_stdin_piped_data() { + let sandbox = create_empty_sandbox(); + + let mut cmd = create_proto_command(sandbox.path()); + cmd.arg("install") + .arg("node") + .arg("--pin") + .arg("--") + .arg("--no-bundled-npm") + .assert() + .success(); + + let mut shim = create_shim_command(sandbox.path(), "node"); + shim.arg(get_fixture("tests/fixtures/shim-piped-stdin.mjs")); + shim.env_remove("PROTO_LOG"); + shim.write_stdin("this data comes from stdin"); + + let assert = shim.assert(); + + assert_snapshot!(get_assert_output(&assert)); + } + + #[test] + fn handles_file_piped_data() { + let sandbox = create_empty_sandbox(); + + let mut cmd = create_proto_command(sandbox.path()); + cmd.arg("install") + .arg("node") + .arg("--pin") + .arg("--") + .arg("--no-bundled-npm") + .assert() + .success(); + + let mut shim = create_shim_command(sandbox.path(), "node"); + shim.arg(get_fixture("tests/fixtures/shim-piped-stdin.mjs")); + shim.env_remove("PROTO_LOG"); + shim.pipe_stdin("tests/fixtures/piped-data.txt").unwrap(); + + let assert = shim.assert(); + + assert_snapshot!(get_assert_output(&assert)); + } + + #[test] + fn handles_exit_codes() { + let sandbox = create_empty_sandbox(); + + let mut cmd = create_proto_command(sandbox.path()); + cmd.arg("install") + .arg("node") + .arg("--pin") + .arg("--") + .arg("--no-bundled-npm") + .assert() + .success(); + + let mut shim = create_shim_command(sandbox.path(), "node"); + shim.arg(get_fixture("tests/fixtures/shim-code-0.mjs")); + shim.assert().code(0); + + let mut shim = create_shim_command(sandbox.path(), "node"); + shim.arg(get_fixture("tests/fixtures/shim-code-1.mjs")); + shim.assert().code(1); + } + + #[test] + #[cfg(not(windows))] + fn handles_signals() { + use signal_child::Signalable; + + let sandbox = create_empty_sandbox(); + + let mut cmd = create_proto_command(sandbox.path()); + cmd.arg("install") + .arg("node") + .arg("--pin") + .arg("--") + .arg("--no-bundled-npm") + .assert() + .success(); + + let mut shim = create_shim_command_std(sandbox.path(), "node"); + shim.arg(get_fixture("tests/fixtures/shim-signal.mjs")); + shim.env_remove("PROTO_LOG"); + + // Interrupt / SIGINT + let mut child = shim.spawn().unwrap(); + child.interrupt().unwrap(); + + assert!(!child.wait().unwrap().success()); + + // Terminate / SIGTERM + let mut child = shim.spawn().unwrap(); + child.term().unwrap(); + + assert!(!child.wait().unwrap().success()); + + // Hangup / SIGHUP + // let mut child = shim.spawn().unwrap(); + // child.hangup().unwrap(); + + // assert!(!child.wait().unwrap().success()); + } +} diff --git a/crates/cli/tests/snapshots/shim_test__shim_bin__handles_file_piped_data.snap b/crates/cli/tests/snapshots/shim_test__shim_bin__handles_file_piped_data.snap new file mode 100644 index 000000000..187191b17 --- /dev/null +++ b/crates/cli/tests/snapshots/shim_test__shim_bin__handles_file_piped_data.snap @@ -0,0 +1,10 @@ +--- +source: crates/cli/tests/shim_test.rs +expression: get_assert_output(&assert) +--- +start +running +piped data = this data comes from a file +stop + + diff --git a/crates/cli/tests/snapshots/shim_test__shim_bin__handles_stdin_piped_data.snap b/crates/cli/tests/snapshots/shim_test__shim_bin__handles_stdin_piped_data.snap new file mode 100644 index 000000000..dd5f8e304 --- /dev/null +++ b/crates/cli/tests/snapshots/shim_test__shim_bin__handles_stdin_piped_data.snap @@ -0,0 +1,10 @@ +--- +source: crates/cli/tests/shim_test.rs +expression: get_assert_output(&assert) +--- +start +running +piped data = this data comes from stdin +stop + + diff --git a/crates/cli/tests/snapshots/shim_test__shim_bin__standard_output.snap b/crates/cli/tests/snapshots/shim_test__shim_bin__standard_output.snap new file mode 100644 index 000000000..9f3896673 --- /dev/null +++ b/crates/cli/tests/snapshots/shim_test__shim_bin__standard_output.snap @@ -0,0 +1,8 @@ +--- +source: crates/cli/tests/shim_test.rs +expression: get_assert_output(&assert) +--- +stdout +stderr + + diff --git a/crates/cli/tests/snapshots/shim_test__shim_bin__waits_for_timeout.snap b/crates/cli/tests/snapshots/shim_test__shim_bin__waits_for_timeout.snap new file mode 100644 index 000000000..3ffffeea2 --- /dev/null +++ b/crates/cli/tests/snapshots/shim_test__shim_bin__waits_for_timeout.snap @@ -0,0 +1,9 @@ +--- +source: crates/cli/tests/shim_test.rs +expression: get_assert_output(&assert) +--- +start +running +stop + + diff --git a/crates/cli/tests/snapshots/shim_test__shim_bin__waits_for_top_level_await.snap b/crates/cli/tests/snapshots/shim_test__shim_bin__waits_for_top_level_await.snap new file mode 100644 index 000000000..3ffffeea2 --- /dev/null +++ b/crates/cli/tests/snapshots/shim_test__shim_bin__waits_for_top_level_await.snap @@ -0,0 +1,9 @@ +--- +source: crates/cli/tests/shim_test.rs +expression: get_assert_output(&assert) +--- +start +running +stop + + diff --git a/crates/cli/tests/utils.rs b/crates/cli/tests/utils.rs index fcf2c4011..9fcf44d64 100644 --- a/crates/cli/tests/utils.rs +++ b/crates/cli/tests/utils.rs @@ -43,14 +43,19 @@ pub fn create_proto_command>(path: T) -> assert_cmd::Command { } pub fn create_shim_command>(path: T, name: &str) -> assert_cmd::Command { + let mut cmd = assert_cmd::Command::from_std(create_shim_command_std(path, name)); + cmd.timeout(std::time::Duration::from_secs(240)); + cmd +} + +pub fn create_shim_command_std>(path: T, name: &str) -> std::process::Command { let path = path.as_ref(); - let mut cmd = assert_cmd::Command::new(path.join(".proto/shims").join(if cfg!(windows) { + let mut cmd = std::process::Command::new(path.join(".proto/shims").join(if cfg!(windows) { format!("{name}.exe") } else { name.to_owned() })); - cmd.timeout(std::time::Duration::from_secs(240)); cmd.env("PROTO_LOG", "trace"); cmd.env("PROTO_HOME", path.join(".proto")); cmd.env("PROTO_NODE_VERSION", "latest"); // For package managers