diff --git a/test/test-manager/src/main.rs b/test/test-manager/src/main.rs index 8628502eff10..a56f1f2ea877 100644 --- a/test/test-manager/src/main.rs +++ b/test/test-manager/src/main.rs @@ -229,7 +229,7 @@ async fn main() -> Result<()> { } Commands::ListTests => { println!("priority\tname"); - for test in tests::get_tests() { + for test in tests::get_test_descriptions() { println!( "{priority:8}\t{name}", name = test.name, diff --git a/test/test-manager/src/run_tests.rs b/test/test-manager/src/run_tests.rs index dd07ef5f3a08..88577fb3b3cd 100644 --- a/test/test-manager/src/run_tests.rs +++ b/test/test-manager/src/run_tests.rs @@ -102,7 +102,7 @@ impl TestHandler<'_> { pub async fn run( instance: &dyn vm::VmInstance, - tests: Vec<&TestMetadata>, + tests: Vec, skip_wait: bool, print_failed_tests_only: bool, summary_logger: Option, @@ -139,6 +139,11 @@ pub async fn run( logger: Logger::get_or_init(), }; + // We need to handle the upgrade test separately since it expects the daemon to *not* be + // installed, which is done by `tests::prepare_daemon`, and only runs with + // `app_package_to_upgrade_from_filename` set. + // TODO: Extend `TestMetadata` and the `test_function` macro to specify what daemon state is + // expected, and to allow for skipping tests on arbitrary conditions. if TEST_CONFIG.app_package_to_upgrade_from_filename.is_some() { test_handler .run_test( diff --git a/test/test-manager/src/summary.rs b/test/test-manager/src/summary.rs index 1beaef91b957..f0c0723c48de 100644 --- a/test/test-manager/src/summary.rs +++ b/test/test-manager/src/summary.rs @@ -5,6 +5,8 @@ use tokio::{ io::{AsyncBufReadExt, AsyncWriteExt}, }; +use crate::tests::should_run_on_os; + #[derive(thiserror::Error, Debug)] pub enum Error { #[error("Failed to open log file {1:?}")] @@ -118,7 +120,7 @@ pub struct Summary { impl Summary { /// Read test summary from `path`. pub async fn parse_log>( - all_tests: &[&crate::tests::TestMetadata], + all_tests: &[crate::tests::TestDescription], path: P, ) -> Result { let file = fs::OpenOptions::new() @@ -155,7 +157,7 @@ impl Summary { for test in all_tests { // Add missing test results let entry = results.entry(test.name.to_owned()); - if test.should_run_on_os(os) { + if should_run_on_os(test.targets, os) { entry.or_insert(TestResult::Unknown); } else { entry.or_insert(TestResult::Skip); @@ -184,12 +186,12 @@ impl Summary { /// be parsed, we should not abort the entire summarization. pub async fn print_summary_table>(summary_files: &[P]) { // Collect test details - let tests = crate::tests::get_tests(); + let tests = crate::tests::get_test_descriptions(); let mut summaries = vec![]; let mut failed_to_parse = vec![]; for sumfile in summary_files { - match Summary::parse_log(&tests, sumfile).await { + match Summary::parse_log(&tests[..], sumfile).await { Ok(summary) => summaries.push(summary), Err(_) => failed_to_parse.push(sumfile), } diff --git a/test/test-manager/src/tests/mod.rs b/test/test-manager/src/tests/mod.rs index a7b3778907bb..9eb23fe13e8d 100644 --- a/test/test-manager/src/tests/mod.rs +++ b/test/test-manager/src/tests/mod.rs @@ -15,6 +15,7 @@ mod tunnel; mod tunnel_state; mod ui; +use itertools::Itertools; pub use test_metadata::TestMetadata; use anyhow::Context; @@ -29,7 +30,7 @@ use config::TEST_CONFIG; use helpers::{get_app_env, install_app}; pub use install::test_upgrade_app; use mullvad_management_interface::MullvadProxyClient; -use test_rpc::ServiceClient; +use test_rpc::{meta::Os, ServiceClient}; const WAIT_FOR_TUNNEL_STATE_TIMEOUT: Duration = Duration::from_secs(40); @@ -75,15 +76,43 @@ pub enum Error { Other(String), } +#[derive(Clone)] +/// An abbreviated version of [`TestMetadata`] +pub struct TestDescription { + pub name: &'static str, + pub targets: &'static [Os], + pub priority: Option, +} + +pub fn should_run_on_os(targets: &[Os], os: Os) -> bool { + targets.is_empty() || targets.contains(&os) +} + /// Get a list of all tests, sorted by priority. -pub fn get_tests() -> Vec<&'static TestMetadata> { - let mut tests: Vec<_> = inventory::iter::().collect(); - tests.sort_by_key(|test| test.priority.unwrap_or(0)); - tests +pub fn get_test_descriptions() -> Vec { + let tests: Vec<_> = inventory::iter::() + .map(|test| TestDescription { + priority: test.priority, + name: test.name, + targets: test.targets, + }) + .sorted_by_key(|test| test.priority) + .collect_vec(); + + // Since `test_upgrade_app` is not registered with inventory, we need to add it manually + let test_upgrade_app = TestDescription { + priority: None, + name: "test_upgrade_app", + targets: &[], + }; + [vec![test_upgrade_app], tests].concat() } -pub fn get_filtered_tests(specified_tests: &[String]) -> Result, anyhow::Error> { - let tests = get_tests(); +/// Return all tests with names matching the input argument. Filters out tests that are skipped for +/// the target platform and `test_upgrade_app`, which is run separately. +pub fn get_filtered_tests(specified_tests: &[String]) -> Result, anyhow::Error> { + let mut tests: Vec<_> = inventory::iter::().cloned().collect(); + tests.sort_by_key(|test| test.priority.unwrap_or(0)); let mut tests = if specified_tests.is_empty() { // Keep all tests @@ -94,13 +123,13 @@ pub fn get_filtered_tests(specified_tests: &[String]) -> Result>()? }; - tests.retain(|test| test.should_run_on_os(TEST_CONFIG.os)); + tests.retain(|test| should_run_on_os(test.targets, TEST_CONFIG.os)); Ok(tests) } diff --git a/test/test-manager/src/tests/test_metadata.rs b/test/test-manager/src/tests/test_metadata.rs index 70476ac19959..79c7f74def7e 100644 --- a/test/test-manager/src/tests/test_metadata.rs +++ b/test/test-manager/src/tests/test_metadata.rs @@ -1,20 +1,14 @@ use super::TestWrapperFunction; use test_rpc::{meta::Os, mullvad_daemon::MullvadClientVersion}; +#[derive(Clone)] pub struct TestMetadata { pub name: &'static str, - pub command: &'static str, pub targets: &'static [Os], pub mullvad_client_version: MullvadClientVersion, pub func: TestWrapperFunction, pub priority: Option, } -impl TestMetadata { - pub fn should_run_on_os(&self, os: Os) -> bool { - self.targets.is_empty() || self.targets.contains(&os) - } -} - // Register our test metadata struct with inventory to allow submitting tests of this type. inventory::collect!(TestMetadata); diff --git a/test/test-manager/test_macro/src/lib.rs b/test/test-manager/test_macro/src/lib.rs index 1b80776f29a4..cddb6c5a2f6d 100644 --- a/test/test-manager/test_macro/src/lib.rs +++ b/test/test-manager/test_macro/src/lib.rs @@ -187,7 +187,6 @@ fn create_test(test_function: TestFunction) -> proc_macro2::TokenStream { quote! { inventory::submit!(crate::tests::test_metadata::TestMetadata { name: stringify!(#func_name), - command: stringify!(#func_name), targets: &[#targets], mullvad_client_version: #function_mullvad_version, func: #wrapper_closure,