From a1b4aa2ae60d215e38c6871fae690e34964a27d7 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Tue, 26 Apr 2022 14:46:49 -0400 Subject: [PATCH] fix(test): capture inherited stdout and stderr for subprocesses in test output (#14395) --- cli/lsp/testing/execution.rs | 5 ++-- cli/ops/testing.rs | 21 ++++++----------- cli/tests/integration/test_tests.rs | 6 +++++ .../test/captured_subprocess_output.out | 17 ++++++++++++++ .../test/captured_subprocess_output.ts | 23 +++++++++++++++++++ cli/tools/test.rs | 18 +++++---------- runtime/ops/process.rs | 16 +++++++++++-- runtime/ops/spawn.rs | 11 +++++++-- 8 files changed, 84 insertions(+), 33 deletions(-) create mode 100644 cli/tests/testdata/test/captured_subprocess_output.out create mode 100644 cli/tests/testdata/test/captured_subprocess_output.ts diff --git a/cli/lsp/testing/execution.rs b/cli/lsp/testing/execution.rs index 93f3d9ba31..6f90e60a36 100644 --- a/cli/lsp/testing/execution.rs +++ b/cli/lsp/testing/execution.rs @@ -760,9 +760,8 @@ impl test::TestReporter for LspTestReporter { .and_then(|v| v.last().map(|td| td.into())) }); let value = match output { - test::TestOutput::PrintStdout(value) - | test::TestOutput::PrintStderr(value) => value.replace('\n', "\r\n"), - test::TestOutput::Stdout(bytes) | test::TestOutput::Stderr(bytes) => { + test::TestOutput::String(value) => value.replace('\n', "\r\n"), + test::TestOutput::Bytes(bytes) => { String::from_utf8_lossy(bytes).replace('\n', "\r\n") } }; diff --git a/cli/ops/testing.rs b/cli/ops/testing.rs index 3a57d307be..008e4d1139 100644 --- a/cli/ops/testing.rs +++ b/cli/ops/testing.rs @@ -79,12 +79,8 @@ pub fn create_stdout_stderr_pipes( let (stdout_reader, stdout_writer) = os_pipe::pipe().unwrap(); let (stderr_reader, stderr_writer) = os_pipe::pipe().unwrap(); - start_output_redirect_thread(stdout_reader, sender.clone(), |bytes| { - TestOutput::Stdout(bytes) - }); - start_output_redirect_thread(stderr_reader, sender, |bytes| { - TestOutput::Stderr(bytes) - }); + start_output_redirect_thread(stdout_reader, sender.clone()); + start_output_redirect_thread(stderr_reader, sender); (stdout_writer, stderr_writer) } @@ -92,7 +88,6 @@ pub fn create_stdout_stderr_pipes( fn start_output_redirect_thread( mut pipe_reader: os_pipe::PipeReader, sender: UnboundedSender, - map_test_output: impl Fn(Vec) -> TestOutput + Send + 'static, ) { tokio::task::spawn_blocking(move || loop { let mut buffer = [0; 512]; @@ -101,7 +96,9 @@ fn start_output_redirect_thread( Ok(size) => size, }; if sender - .send(TestEvent::Output(map_test_output(buffer[0..size].to_vec()))) + .send(TestEvent::Output(TestOutput::Bytes( + buffer[0..size].to_vec(), + ))) .is_err() { break; @@ -170,14 +167,10 @@ fn op_dispatch_test_event( pub fn op_print( state: &mut OpState, msg: String, - is_err: bool, + _is_err: bool, ) -> Result<(), AnyError> { let sender = state.borrow::>().clone(); - let msg = if is_err { - TestOutput::PrintStderr(msg) - } else { - TestOutput::PrintStdout(msg) - }; + let msg = TestOutput::String(msg); sender.send(TestEvent::Output(msg)).ok(); Ok(()) } diff --git a/cli/tests/integration/test_tests.rs b/cli/tests/integration/test_tests.rs index 0b811349ab..1e8db52fda 100644 --- a/cli/tests/integration/test_tests.rs +++ b/cli/tests/integration/test_tests.rs @@ -302,6 +302,12 @@ itest!(no_prompt_with_denied_perms { output: "test/no_prompt_with_denied_perms.out", }); +itest!(captured_subprocess_output { + args: "test --allow-run --allow-read --unstable test/captured_subprocess_output.ts", + exit_code: 0, + output: "test/captured_subprocess_output.out", +}); + #[test] fn recursive_permissions_pledge() { let output = util::deno_cmd() diff --git a/cli/tests/testdata/test/captured_subprocess_output.out b/cli/tests/testdata/test/captured_subprocess_output.out new file mode 100644 index 0000000000..2a40170afe --- /dev/null +++ b/cli/tests/testdata/test/captured_subprocess_output.out @@ -0,0 +1,17 @@ +[WILDCARD] +running 1 test from [WILDCARD]/captured_subprocess_output.ts +output ... +------- output ------- +1 +2 +3 +4 +5 +6 +7 +8 +----- output end ----- +ok ([WILDCARD]s) + +test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out ([WILDCARD]s) +[WILDCARD] diff --git a/cli/tests/testdata/test/captured_subprocess_output.ts b/cli/tests/testdata/test/captured_subprocess_output.ts new file mode 100644 index 0000000000..277ac340e2 --- /dev/null +++ b/cli/tests/testdata/test/captured_subprocess_output.ts @@ -0,0 +1,23 @@ +Deno.test("output", async () => { + const p = Deno.run({ + cmd: [Deno.execPath(), "eval", "console.log(1); console.error(2);"], + }); + await p.status(); + await p.close(); + Deno.spawnSync(Deno.execPath(), { + args: ["eval", "console.log(3); console.error(4);"], + stdout: "inherit", + stderr: "inherit", + }); + await Deno.spawn(Deno.execPath(), { + args: ["eval", "console.log(5); console.error(6);"], + stdout: "inherit", + stderr: "inherit", + }); + const c = await Deno.spawnChild(Deno.execPath(), { + args: ["eval", "console.log(7); console.error(8);"], + stdout: "inherit", + stderr: "inherit", + }); + await c.status; +}); diff --git a/cli/tools/test.rs b/cli/tools/test.rs index aab062a6c8..d7817eb1ad 100644 --- a/cli/tools/test.rs +++ b/cli/tools/test.rs @@ -83,10 +83,8 @@ pub struct TestDescription { #[derive(Debug, Clone, PartialEq, Deserialize)] #[serde(rename_all = "camelCase")] pub enum TestOutput { - PrintStdout(String), - PrintStderr(String), - Stdout(Vec), - Stderr(Vec), + String(String), + Bytes(Vec), } #[derive(Debug, Clone, PartialEq, Deserialize)] @@ -351,18 +349,14 @@ impl TestReporter for PrettyTestReporter { println!("{}", colors::gray("------- output -------")); } match output { - TestOutput::PrintStdout(line) => { + TestOutput::String(line) => { + // output everything to stdout in order to prevent + // stdout and stderr racing print!("{}", line) } - TestOutput::PrintStderr(line) => { - eprint!("{}", line) - } - TestOutput::Stdout(bytes) => { + TestOutput::Bytes(bytes) => { std::io::stdout().write_all(bytes).unwrap(); } - TestOutput::Stderr(bytes) => { - std::io::stderr().write_all(bytes).unwrap(); - } } } diff --git a/runtime/ops/process.rs b/runtime/ops/process.rs index 8261e9eb48..e22ed3ac31 100644 --- a/runtime/ops/process.rs +++ b/runtime/ops/process.rs @@ -186,8 +186,20 @@ fn op_run(state: &mut OpState, run_args: RunArgs) -> Result { // TODO: make this work with other resources, eg. sockets c.stdin(run_args.stdin.as_stdio(state)?); - c.stdout(run_args.stdout.as_stdio(state)?); - c.stderr(run_args.stderr.as_stdio(state)?); + c.stdout( + match run_args.stdout { + StdioOrRid::Stdio(Stdio::Inherit) => StdioOrRid::Rid(1), + value => value, + } + .as_stdio(state)?, + ); + c.stderr( + match run_args.stderr { + StdioOrRid::Stdio(Stdio::Inherit) => StdioOrRid::Rid(2), + value => value, + } + .as_stdio(state)?, + ); // We want to kill child when it's closed c.kill_on_drop(true); diff --git a/runtime/ops/spawn.rs b/runtime/ops/spawn.rs index 9ec1937af9..0f329e16ee 100644 --- a/runtime/ops/spawn.rs +++ b/runtime/ops/spawn.rs @@ -4,6 +4,7 @@ use super::io::ChildStderrResource; use super::io::ChildStdinResource; use super::io::ChildStdoutResource; use super::process::Stdio; +use super::process::StdioOrRid; use crate::permissions::Permissions; use deno_core::error::AnyError; use deno_core::op; @@ -147,8 +148,14 @@ fn create_command( } command.stdin(args.stdio.stdin.as_stdio()); - command.stdout(args.stdio.stdout.as_stdio()); - command.stderr(args.stdio.stderr.as_stdio()); + command.stdout(match args.stdio.stdout { + Stdio::Inherit => StdioOrRid::Rid(1).as_stdio(state)?, + value => value.as_stdio(), + }); + command.stderr(match args.stdio.stderr { + Stdio::Inherit => StdioOrRid::Rid(2).as_stdio(state)?, + value => value.as_stdio(), + }); Ok(command) }