From 9bbf9433e6c31a3fdb16f997fbbde2069cee6489 Mon Sep 17 00:00:00 2001 From: Karolin Varner Date: Mon, 19 Aug 2024 00:24:07 +0200 Subject: [PATCH] fix(API): Be polite and kill child processes in api integration tests --- .../tests/api-integration-tests-api-setup.rs | 61 +++++++++++-------- rosenpass/tests/api-integration-tests.rs | 51 ++++++++++------ 2 files changed, 69 insertions(+), 43 deletions(-) diff --git a/rosenpass/tests/api-integration-tests-api-setup.rs b/rosenpass/tests/api-integration-tests-api-setup.rs index 72eebbe..8c43234 100644 --- a/rosenpass/tests/api-integration-tests-api-setup.rs +++ b/rosenpass/tests/api-integration-tests-api-setup.rs @@ -19,7 +19,7 @@ use rosenpass_util::{ file::LoadValueB64, io::IoErrorKind, length_prefix_encoding::{decoder::LengthPrefixDecoder, encoder::LengthPrefixEncoder}, - mem::MoveExt, + mem::{DiscardResultExt, MoveExt}, mio::WriteWithFileDescriptors, zerocopy::ZerocopySliceExt, }; @@ -29,6 +29,15 @@ use zerocopy::AsBytes; use rosenpass::protocol::SymKey; +struct KillChild(std::process::Child); + +impl Drop for KillChild { + fn drop(&mut self) { + self.0.kill().discard_result(); + self.0.wait().discard_result() + } +} + #[test] fn api_integration_api_setup() -> anyhow::Result<()> { rosenpass_secret_memory::policy::secret_policy_use_only_malloc_secrets(); @@ -120,33 +129,37 @@ fn api_integration_api_setup() -> anyhow::Result<()> { let deliberate_fail_child_fd = 3; // Start peer a - let _proc_a = std::process::Command::new(env!("CARGO_BIN_EXE_rosenpass")) - .args(["--api-stream-fd", &deliberate_fail_child_fd.to_string()]) - .fd_mappings(vec![FdMapping { - parent_fd: deliberate_fail_api_server.move_here().as_raw_fd(), - child_fd: 3, - }])? - .args([ - "exchange-config", - peer_a.config_file_path.to_str().context("")?, - ]) - .stdin(Stdio::null()) - .stdout(Stdio::null()) - .spawn()?; + let _proc_a = KillChild( + std::process::Command::new(env!("CARGO_BIN_EXE_rosenpass")) + .args(["--api-stream-fd", &deliberate_fail_child_fd.to_string()]) + .fd_mappings(vec![FdMapping { + parent_fd: deliberate_fail_api_server.move_here().as_raw_fd(), + child_fd: 3, + }])? + .args([ + "exchange-config", + peer_a.config_file_path.to_str().context("")?, + ]) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .spawn()?, + ); // Start peer b - let proc_b = std::process::Command::new(env!("CARGO_BIN_EXE_rosenpass")) - .args([ - "exchange-config", - peer_b.config_file_path.to_str().context("")?, - ]) - .stdin(Stdio::null()) - .stderr(Stdio::null()) - .stdout(Stdio::piped()) - .spawn()?; + let mut proc_b = KillChild( + std::process::Command::new(env!("CARGO_BIN_EXE_rosenpass")) + .args([ + "exchange-config", + peer_b.config_file_path.to_str().context("")?, + ]) + .stdin(Stdio::null()) + .stderr(Stdio::null()) + .stdout(Stdio::piped()) + .spawn()?, + ); // Acquire stdout - let mut out_b = BufReader::new(proc_b.stdout.context("")?).lines(); + let mut out_b = BufReader::new(proc_b.0.stdout.take().context("")?).lines(); // Now connect to the peers let api_path = peer_a.api.listen_path[0].as_path(); diff --git a/rosenpass/tests/api-integration-tests.rs b/rosenpass/tests/api-integration-tests.rs index cc777a7..f5797da 100644 --- a/rosenpass/tests/api-integration-tests.rs +++ b/rosenpass/tests/api-integration-tests.rs @@ -8,16 +8,25 @@ use std::{ use anyhow::{bail, Context}; use rosenpass::api; use rosenpass_to::{ops::copy_slice_least_src, To}; -use rosenpass_util::zerocopy::ZerocopySliceExt; use rosenpass_util::{ file::LoadValueB64, length_prefix_encoding::{decoder::LengthPrefixDecoder, encoder::LengthPrefixEncoder}, }; +use rosenpass_util::{mem::DiscardResultExt, zerocopy::ZerocopySliceExt}; use tempfile::TempDir; use zerocopy::AsBytes; use rosenpass::protocol::SymKey; +struct KillChild(std::process::Child); + +impl Drop for KillChild { + fn drop(&mut self) { + self.0.kill().discard_result(); + self.0.wait().discard_result() + } +} + #[test] fn api_integration_test() -> anyhow::Result<()> { rosenpass_secret_memory::policy::secret_policy_use_only_malloc_secrets(); @@ -93,28 +102,32 @@ fn api_integration_test() -> anyhow::Result<()> { peer_b.commit()?; // Start peer a - let proc_a = std::process::Command::new(env!("CARGO_BIN_EXE_rosenpass")) - .args([ - "exchange-config", - peer_a.config_file_path.to_str().context("")?, - ]) - .stdin(Stdio::null()) - .stdout(Stdio::piped()) - .spawn()?; + let mut proc_a = KillChild( + std::process::Command::new(env!("CARGO_BIN_EXE_rosenpass")) + .args([ + "exchange-config", + peer_a.config_file_path.to_str().context("")?, + ]) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .spawn()?, + ); // Start peer b - let proc_b = std::process::Command::new(env!("CARGO_BIN_EXE_rosenpass")) - .args([ - "exchange-config", - peer_b.config_file_path.to_str().context("")?, - ]) - .stdin(Stdio::null()) - .stdout(Stdio::piped()) - .spawn()?; + let mut proc_b = KillChild( + std::process::Command::new(env!("CARGO_BIN_EXE_rosenpass")) + .args([ + "exchange-config", + peer_b.config_file_path.to_str().context("")?, + ]) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .spawn()?, + ); // Acquire stdout - let mut out_a = BufReader::new(proc_a.stdout.context("")?).lines(); - let mut out_b = BufReader::new(proc_b.stdout.context("")?).lines(); + let mut out_a = BufReader::new(proc_a.0.stdout.take().context("")?).lines(); + let mut out_b = BufReader::new(proc_b.0.stdout.take().context("")?).lines(); // Wait for the keys to successfully exchange a key let mut attempt = 0;