diff options
| -rw-r--r-- | cli/tests/integration/bench_tests.rs | 2 | ||||
| -rw-r--r-- | runtime/ops/process.rs | 5 | ||||
| -rw-r--r-- | test_util/src/builders.rs | 221 |
3 files changed, 181 insertions, 47 deletions
diff --git a/cli/tests/integration/bench_tests.rs b/cli/tests/integration/bench_tests.rs index 6da30857f..b2cd38475 100644 --- a/cli/tests/integration/bench_tests.rs +++ b/cli/tests/integration/bench_tests.rs @@ -196,7 +196,7 @@ fn recursive_permissions_pledge() { .run(); output.assert_exit_code(1); assert_contains!( - output.text(), + output.combined_output(), "pledge test permissions called before restoring previous pledge" ); } diff --git a/runtime/ops/process.rs b/runtime/ops/process.rs index ad14ef2e4..20db96af9 100644 --- a/runtime/ops/process.rs +++ b/runtime/ops/process.rs @@ -1,7 +1,6 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. use super::check_unstable; -use super::signal; use crate::permissions::PermissionsContainer; use deno_core::error::AnyError; use deno_core::op; @@ -565,7 +564,7 @@ mod deprecated { #[cfg(unix)] pub fn kill(pid: i32, signal: &str) -> Result<(), AnyError> { - let signo = super::signal::signal_str_to_int(signal)?; + let signo = super::super::signal::signal_str_to_int(signal)?; use nix::sys::signal::kill as unix_kill; use nix::sys::signal::Signal; use nix::unistd::Pid; @@ -593,8 +592,8 @@ mod deprecated { } else if pid <= 0 { Err(type_error("Invalid pid")) } else { - // SAFETY: winapi call let handle = + // SAFETY: winapi call unsafe { OpenProcess(PROCESS_TERMINATE, FALSE, pid as DWORD) }; if handle.is_null() { diff --git a/test_util/src/builders.rs b/test_util/src/builders.rs index 6ccb45f50..da331b62a 100644 --- a/test_util/src/builders.rs +++ b/test_util/src/builders.rs @@ -168,6 +168,7 @@ impl TestContext { envs: Default::default(), env_clear: Default::default(), cwd: Default::default(), + split_output: false, context: self.clone(), } } @@ -181,6 +182,7 @@ pub struct TestCommandBuilder { envs: HashMap<String, String>, env_clear: bool, cwd: Option<String>, + split_output: bool, context: TestContext, } @@ -205,6 +207,15 @@ impl TestCommandBuilder { self } + /// Splits the output into stdout and stderr rather than having them combined. + pub fn split_output(&mut self) -> &mut Self { + // Note: it was previously attempted to capture stdout & stderr separately + // then forward the output to a combined pipe, but this was found to be + // too racy compared to providing the same combined pipe to both. + self.split_output = true; + self + } + pub fn env( &mut self, key: impl AsRef<str>, @@ -227,6 +238,23 @@ impl TestCommandBuilder { } pub fn run(&self) -> TestCommandOutput { + fn read_pipe_to_string(mut pipe: os_pipe::PipeReader) -> String { + let mut output = String::new(); + pipe.read_to_string(&mut output).unwrap(); + output + } + + fn sanitize_output(text: String, args: &[String]) -> String { + let mut text = strip_ansi_codes(&text).to_string(); + // deno test's output capturing flushes with a zero-width space in order to + // synchronize the output pipes. Occassionally this zero width space + // might end up in the output so strip it from the output comparison here. + if args.first().map(|s| s.as_str()) == Some("test") { + text = text.replace('\u{200B}', ""); + } + text + } + let cwd = self.cwd.as_ref().or(self.context.cwd.as_ref()); let cwd = if self.context.use_temp_cwd { assert!(cwd.is_none()); @@ -256,7 +284,6 @@ impl TestCommandBuilder { arg.replace("$TESTDATA", &self.context.testdata_dir.to_string_lossy()) }) .collect::<Vec<_>>(); - let (mut reader, writer) = pipe().unwrap(); let command_name = self .command_name .as_ref() @@ -284,9 +311,25 @@ impl TestCommandBuilder { }); command.current_dir(cwd); command.stdin(Stdio::piped()); - let writer_clone = writer.try_clone().unwrap(); - command.stderr(writer_clone); - command.stdout(writer); + + let (combined_reader, std_out_err_handle) = if self.split_output { + let (stdout_reader, stdout_writer) = pipe().unwrap(); + let (stderr_reader, stderr_writer) = pipe().unwrap(); + command.stdout(stdout_writer); + command.stderr(stderr_writer); + ( + None, + Some(( + std::thread::spawn(move || read_pipe_to_string(stdout_reader)), + std::thread::spawn(move || read_pipe_to_string(stderr_reader)), + )), + ) + } else { + let (combined_reader, combined_writer) = pipe().unwrap(); + command.stdout(combined_writer.try_clone().unwrap()); + command.stderr(combined_writer); + (Some(combined_reader), None) + }; let mut process = command.spawn().unwrap(); @@ -301,10 +344,16 @@ impl TestCommandBuilder { // and dropping it closes them. drop(command); - let mut actual = String::new(); - reader.read_to_string(&mut actual).unwrap(); + let combined = combined_reader + .map(|pipe| sanitize_output(read_pipe_to_string(pipe), &args)); - let status = process.wait().expect("failed to finish process"); + let status = process.wait().unwrap(); + let std_out_err = std_out_err_handle.map(|(stdout, stderr)| { + ( + sanitize_output(stdout.join().unwrap(), &args), + sanitize_output(stderr.join().unwrap(), &args), + ) + }); let exit_code = status.code(); #[cfg(unix)] let signal = { @@ -314,33 +363,30 @@ impl TestCommandBuilder { #[cfg(not(unix))] let signal = None; - actual = strip_ansi_codes(&actual).to_string(); - - // deno test's output capturing flushes with a zero-width space in order to - // synchronize the output pipes. Occassionally this zero width space - // might end up in the output so strip it from the output comparison here. - if args.first().map(|s| s.as_str()) == Some("test") { - actual = actual.replace('\u{200B}', ""); - } - TestCommandOutput { exit_code, signal, - text: actual, + combined, + std_out_err, testdata_dir: self.context.testdata_dir.clone(), asserted_exit_code: RefCell::new(false), - asserted_text: RefCell::new(false), + asserted_stdout: RefCell::new(false), + asserted_stderr: RefCell::new(false), + asserted_combined: RefCell::new(false), _test_context: self.context.clone(), } } } pub struct TestCommandOutput { - text: String, + combined: Option<String>, + std_out_err: Option<(String, String)>, exit_code: Option<i32>, signal: Option<i32>, testdata_dir: PathBuf, - asserted_text: RefCell<bool>, + asserted_stdout: RefCell<bool>, + asserted_stderr: RefCell<bool>, + asserted_combined: RefCell<bool>, asserted_exit_code: RefCell<bool>, // keep alive for the duration of the output reference _test_context: TestContext, @@ -348,6 +394,17 @@ pub struct TestCommandOutput { impl Drop for TestCommandOutput { fn drop(&mut self) { + fn panic_unasserted_output(text: &str) { + println!("OUTPUT\n{text}\nOUTPUT"); + panic!( + concat!( + "The non-empty text of the command was not asserted at {}. ", + "Call `output.skip_output_check()` to skip if necessary.", + ), + failed_position() + ); + } + if std::thread::panicking() { return; } @@ -359,15 +416,20 @@ impl Drop for TestCommandOutput { failed_position(), ) } - if !*self.asserted_text.borrow() && !self.text.is_empty() { - println!("OUTPUT\n{}\nOUTPUT", self.text); - panic!( - concat!( - "The non-empty text of the command was not asserted. ", - "Call `output.skip_output_check()` to skip if necessary at {}.", - ), - failed_position() - ); + + // either the combined output needs to be asserted or both stdout and stderr + if let Some(combined) = &self.combined { + if !*self.asserted_combined.borrow() && !combined.is_empty() { + panic_unasserted_output(combined); + } + } + if let Some((stdout, stderr)) = &self.std_out_err { + if !*self.asserted_stdout.borrow() && !stdout.is_empty() { + panic_unasserted_output(stdout); + } + if !*self.asserted_stderr.borrow() && !stderr.is_empty() { + panic_unasserted_output(stderr); + } } } } @@ -378,7 +440,9 @@ impl TestCommandOutput { } pub fn skip_output_check(&self) { - *self.asserted_text.borrow_mut() = true; + *self.asserted_combined.borrow_mut() = true; + *self.asserted_stdout.borrow_mut() = true; + *self.asserted_stderr.borrow_mut() = true; } pub fn skip_exit_code_check(&self) { @@ -394,9 +458,30 @@ impl TestCommandOutput { self.signal } - pub fn text(&self) -> &str { + pub fn combined_output(&self) -> &str { self.skip_output_check(); - &self.text + self + .combined + .as_deref() + .expect("not available since .split_output() was called") + } + + pub fn stdout(&self) -> &str { + *self.asserted_stdout.borrow_mut() = true; + self + .std_out_err + .as_ref() + .map(|(stdout, _)| stdout.as_str()) + .expect("call .split_output() on the builder") + } + + pub fn stderr(&self) -> &str { + *self.asserted_stderr.borrow_mut() = true; + self + .std_out_err + .as_ref() + .map(|(_, stderr)| stderr.as_str()) + .expect("call .split_output() on the builder") } pub fn assert_exit_code(&self, expected_exit_code: i32) -> &Self { @@ -404,7 +489,7 @@ impl TestCommandOutput { if let Some(exit_code) = &actual_exit_code { if *exit_code != expected_exit_code { - println!("OUTPUT\n{}\nOUTPUT", self.text()); + self.print_output(); panic!( "bad exit code, expected: {:?}, actual: {:?} at {}", expected_exit_code, @@ -413,7 +498,7 @@ impl TestCommandOutput { ); } } else { - println!("OUTPUT\n{}\nOUTPUT", self.text()); + self.print_output(); if let Some(signal) = self.signal() { panic!( "process terminated by signal, expected exit code: {:?}, actual signal: {:?} at {}", @@ -433,29 +518,79 @@ impl TestCommandOutput { self } + pub fn print_output(&self) { + if let Some(combined) = &self.combined { + println!("OUTPUT\n{combined}\nOUTPUT"); + } else if let Some((stdout, stderr)) = &self.std_out_err { + println!("STDOUT OUTPUT\n{stdout}\nSTDOUT OUTPUT"); + println!("STDERR OUTPUT\n{stderr}\nSTDERR OUTPUT"); + } + } + pub fn assert_matches_text(&self, expected_text: impl AsRef<str>) -> &Self { - let expected_text = expected_text.as_ref(); - let actual = self.text(); + self.inner_assert_matches_text(self.combined_output(), expected_text) + } + + pub fn assert_matches_file(&self, file_path: impl AsRef<Path>) -> &Self { + self.inner_assert_matches_file(self.combined_output(), file_path) + } + + pub fn assert_stdout_matches_text( + &self, + expected_text: impl AsRef<str>, + ) -> &Self { + self.inner_assert_matches_text(self.stdout(), expected_text) + } - if !expected_text.contains("[WILDCARD]") { - assert_eq!(actual, expected_text, "at {}", failed_position()); - } else if !wildcard_match(expected_text, actual) { + pub fn assert_stdout_matches_file( + &self, + file_path: impl AsRef<Path>, + ) -> &Self { + self.inner_assert_matches_file(self.stdout(), file_path) + } + + pub fn assert_stderr_matches_text( + &self, + expected_text: impl AsRef<str>, + ) -> &Self { + self.inner_assert_matches_text(self.stderr(), expected_text) + } + + pub fn assert_stderrr_matches_file( + &self, + file_path: impl AsRef<Path>, + ) -> &Self { + self.inner_assert_matches_file(self.stderr(), file_path) + } + + fn inner_assert_matches_text( + &self, + actual: &str, + expected: impl AsRef<str>, + ) -> &Self { + let expected = expected.as_ref(); + if !expected.contains("[WILDCARD]") { + assert_eq!(actual, expected, "at {}", failed_position()); + } else if !wildcard_match(expected, actual) { println!("OUTPUT START\n{actual}\nOUTPUT END"); - println!("EXPECTED START\n{expected_text}\nEXPECTED END"); + println!("EXPECTED START\n{expected}\nEXPECTED END"); panic!("pattern match failed at {}", failed_position()); } - self } - pub fn assert_matches_file(&self, file_path: impl AsRef<Path>) -> &Self { + fn inner_assert_matches_file( + &self, + actual: &str, + file_path: impl AsRef<Path>, + ) -> &Self { let output_path = self.testdata_dir().join(file_path); println!("output path {}", output_path.display()); let expected_text = std::fs::read_to_string(&output_path).unwrap_or_else(|err| { panic!("failed loading {}\n\n{err:#}", output_path.display()) }); - self.assert_matches_text(expected_text) + self.inner_assert_matches_text(actual, expected_text) } } |