From 8f478167ab1c9330a9e9831efde267784d0a076a Mon Sep 17 00:00:00 2001 From: Markus Pettersson Date: Thu, 12 Oct 2023 16:31:48 +0200 Subject: [PATCH] Move `run_test` function to `run_tests.rs` Move `run_test` function to `run_tests.rs` from `logging.rs`, which makes sense. --- test/test-manager/src/logging.rs | 65 +++--------------------------- test/test-manager/src/run_tests.rs | 58 +++++++++++++++++++++++++- 2 files changed, 63 insertions(+), 60 deletions(-) diff --git a/test/test-manager/src/logging.rs b/test/test-manager/src/logging.rs index 00f59962681a..e7bd8a9ed8a1 100644 --- a/test/test-manager/src/logging.rs +++ b/test/test-manager/src/logging.rs @@ -1,13 +1,7 @@ use crate::tests::Error; use colored::Colorize; -use futures::FutureExt; -use std::panic; -use std::sync::Mutex; -use std::{future::Future, sync::Arc}; -use test_rpc::{ - logging::{LogOutput, Output}, - ServiceClient, -}; +use std::sync::{Arc, Mutex}; +use test_rpc::logging::{LogOutput, Output}; /// Logger that optionally supports logging records to a buffer #[derive(Clone)] @@ -118,10 +112,10 @@ impl log::Log for Logger { pub struct PanicMessage(String); pub struct TestOutput { - error_messages: Vec, - test_name: &'static str, + pub error_messages: Vec, + pub test_name: &'static str, pub result: Result, PanicMessage>, - log_output: LogOutput, + pub log_output: LogOutput, } impl TestOutput { @@ -189,54 +183,7 @@ impl TestOutput { } } -pub async fn run_test( - runner_rpc: ServiceClient, - mullvad_rpc: MullvadClient, - test: &F, - test_name: &'static str, - test_context: super::tests::TestContext, -) -> Result -where - F: Fn(super::tests::TestContext, ServiceClient, MullvadClient) -> R, - R: Future>, -{ - let _flushed = runner_rpc.try_poll_output().await; - - // Assert that the test is unwind safe, this is the same assertion that cargo tests do. This - // assertion being incorrect can not lead to memory unsafety however it could theoretically - // lead to logic bugs. The problem of forcing the test to be unwind safe is that it causes a - // large amount of unergonomic design. - let result = panic::AssertUnwindSafe(test(test_context, runner_rpc.clone(), mullvad_rpc)) - .catch_unwind() - .await - .map_err(panic_as_string); - - let mut output = vec![]; - if matches!(result, Ok(Err(_)) | Err(_)) { - let output_after_test = runner_rpc.try_poll_output().await; - match output_after_test { - Ok(mut output_after_test) => { - output.append(&mut output_after_test); - } - Err(e) => { - output.push(Output::Other(format!("could not get logs: {:?}", e))); - } - } - } - let log_output = runner_rpc - .get_mullvad_app_logs() - .await - .map_err(Error::Rpc)?; - - Ok(TestOutput { - log_output, - test_name, - error_messages: output, - result, - }) -} - -fn panic_as_string(error: Box) -> PanicMessage { +pub fn panic_as_string(error: Box) -> PanicMessage { if let Some(result) = error.downcast_ref::() { return PanicMessage(result.clone()); } diff --git a/test/test-manager/src/run_tests.rs b/test/test-manager/src/run_tests.rs index 9fbefb4219d9..c1f29dbea812 100644 --- a/test/test-manager/src/run_tests.rs +++ b/test/test-manager/src/run_tests.rs @@ -1,9 +1,18 @@ use crate::summary::{self, maybe_log_test_result}; use crate::tests::TestContext; -use crate::{logging::run_test, mullvad_daemon, tests, vm}; +use crate::{ + logging::{panic_as_string, TestOutput}, + mullvad_daemon, tests, + tests::Error, + vm, +}; use anyhow::{Context, Result}; +use futures::FutureExt; use mullvad_management_interface::ManagementServiceClient; +use std::future::Future; +use std::panic; use std::time::Duration; +use test_rpc::logging::Output; use test_rpc::{mullvad_daemon::MullvadClientVersion, ServiceClient}; const BAUD: u32 = 115200; @@ -167,3 +176,50 @@ pub async fn run( final_result } + +pub async fn run_test( + runner_rpc: ServiceClient, + mullvad_rpc: MullvadClient, + test: &F, + test_name: &'static str, + test_context: super::tests::TestContext, +) -> Result +where + F: Fn(super::tests::TestContext, ServiceClient, MullvadClient) -> R, + R: Future>, +{ + let _flushed = runner_rpc.try_poll_output().await; + + // Assert that the test is unwind safe, this is the same assertion that cargo tests do. This + // assertion being incorrect can not lead to memory unsafety however it could theoretically + // lead to logic bugs. The problem of forcing the test to be unwind safe is that it causes a + // large amount of unergonomic design. + let result = panic::AssertUnwindSafe(test(test_context, runner_rpc.clone(), mullvad_rpc)) + .catch_unwind() + .await + .map_err(panic_as_string); + + let mut output = vec![]; + if matches!(result, Ok(Err(_)) | Err(_)) { + let output_after_test = runner_rpc.try_poll_output().await; + match output_after_test { + Ok(mut output_after_test) => { + output.append(&mut output_after_test); + } + Err(e) => { + output.push(Output::Other(format!("could not get logs: {:?}", e))); + } + } + } + let log_output = runner_rpc + .get_mullvad_app_logs() + .await + .map_err(Error::Rpc)?; + + Ok(TestOutput { + log_output, + test_name, + error_messages: output, + result, + }) +}