From 5fd3fe5954666e0e6c27642688db26d58d04bd0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20L=C3=B6nnhager?= Date: Wed, 27 Dec 2023 14:38:36 +0100 Subject: [PATCH] Add 'target_os' attribute to test macro --- test/test-manager/src/config.rs | 10 +++++++ test/test-manager/src/main.rs | 1 + test/test-manager/src/run_tests.rs | 8 +++-- test/test-manager/src/tests/config.rs | 3 ++ test/test-manager/src/tests/install.rs | 2 +- test/test-manager/src/tests/test_metadata.rs | 10 +++++++ test/test-manager/src/tests/tunnel.rs | 4 +-- test/test-manager/src/tests/ui.rs | 2 +- test/test-manager/test_macro/src/lib.rs | 31 +++++++++++++++++--- test/test-rpc/src/client.rs | 8 ----- test/test-rpc/src/lib.rs | 3 -- test/test-rpc/src/meta.rs | 25 +++++++++------- test/test-runner/src/main.rs | 5 ---- 13 files changed, 75 insertions(+), 37 deletions(-) diff --git a/test/test-manager/src/config.rs b/test/test-manager/src/config.rs index 7145dca8a5e3..0acbeb322ee9 100644 --- a/test/test-manager/src/config.rs +++ b/test/test-manager/src/config.rs @@ -195,6 +195,16 @@ pub enum OsType { Macos, } +impl From for test_rpc::meta::Os { + fn from(ostype: OsType) -> Self { + match ostype { + OsType::Windows => Self::Windows, + OsType::Linux => Self::Linux, + OsType::Macos => Self::Macos, + } + } +} + #[derive(clap::ValueEnum, Debug, Serialize, Deserialize, Clone, Copy, PartialEq, Eq)] #[serde(rename_all = "snake_case")] pub enum PackageType { diff --git a/test/test-manager/src/main.rs b/test/test-manager/src/main.rs index 37a46c2580cb..78b951e90c86 100644 --- a/test/test-manager/src/main.rs +++ b/test/test-manager/src/main.rs @@ -273,6 +273,7 @@ async fn main() -> Result<()> { host_bridge_name: crate::vm::network::macos::find_vm_bridge()?, #[cfg(not(target_os = "macos"))] host_bridge_name: crate::vm::network::linux::BRIDGE_NAME.to_owned(), + os: test_rpc::meta::Os::from(vm_config.os_type), }, &*instance, &test_filters, diff --git a/test/test-manager/src/run_tests.rs b/test/test-manager/src/run_tests.rs index 4a296ce2887c..b316511b06f0 100644 --- a/test/test-manager/src/run_tests.rs +++ b/test/test-manager/src/run_tests.rs @@ -1,5 +1,5 @@ use crate::summary::{self, maybe_log_test_result}; -use crate::tests::TestContext; +use crate::tests::{config::TEST_CONFIG, TestContext}; use crate::{ logging::{panic_as_string, TestOutput}, mullvad_daemon, tests, @@ -26,7 +26,7 @@ pub async fn run( mut summary_logger: Option, ) -> Result<()> { log::trace!("Setting test constants"); - tests::config::TEST_CONFIG.init(config); + TEST_CONFIG.init(config); let pty_path = instance.get_pty(); @@ -47,7 +47,9 @@ pub async fn run( let mullvad_client = mullvad_daemon::new_rpc_client(connection_handle, mullvad_daemon_transport); - let mut tests: Vec<_> = inventory::iter::().collect(); + let mut tests: Vec<_> = inventory::iter::() + .filter(|test| test.should_run_on_os(TEST_CONFIG.os)) + .collect(); tests.sort_by_key(|test| test.priority.unwrap_or(0)); if !test_filters.is_empty() { diff --git a/test/test-manager/src/tests/config.rs b/test/test-manager/src/tests/config.rs index a0a22368ddc1..7ffe737aa779 100644 --- a/test/test-manager/src/tests/config.rs +++ b/test/test-manager/src/tests/config.rs @@ -1,5 +1,6 @@ use once_cell::sync::OnceCell; use std::ops::Deref; +use test_rpc::meta::Os; // Default `mullvad_host`. This should match the production env. pub const DEFAULT_MULLVAD_HOST: &str = "mullvad.net"; @@ -20,6 +21,8 @@ pub struct TestConfig { pub mullvad_host: String, pub host_bridge_name: String, + + pub os: Os, } #[derive(Debug, Clone)] diff --git a/test/test-manager/src/tests/install.rs b/test/test-manager/src/tests/install.rs index 48ce2334936f..ec1344ede0f5 100644 --- a/test/test-manager/src/tests/install.rs +++ b/test/test-manager/src/tests/install.rs @@ -360,7 +360,7 @@ async fn replace_openvpn_cert(rpc: &ServiceClient) -> Result<(), Error> { const SOURCE_CERT_FILENAME: &str = "openvpn.ca.crt"; const DEST_CERT_FILENAME: &str = "ca.crt"; - let dest_dir = match rpc.get_os().await.expect("failed to get OS") { + let dest_dir = match TEST_CONFIG.os { Os::Windows => "C:\\Program Files\\Mullvad VPN\\resources", Os::Linux => "/opt/Mullvad VPN/resources", Os::Macos => "/Applications/Mullvad VPN.app/Contents/Resources", diff --git a/test/test-manager/src/tests/test_metadata.rs b/test/test-manager/src/tests/test_metadata.rs index 39d802e5e09e..3e28a4380b6a 100644 --- a/test/test-manager/src/tests/test_metadata.rs +++ b/test/test-manager/src/tests/test_metadata.rs @@ -1,9 +1,11 @@ use super::TestWrapperFunction; +use test_rpc::meta::Os; use test_rpc::mullvad_daemon::MullvadClientVersion; pub struct TestMetadata { pub name: &'static str, pub command: &'static str, + pub target_os: Option, pub mullvad_client_version: MullvadClientVersion, pub func: TestWrapperFunction, pub priority: Option, @@ -12,5 +14,13 @@ pub struct TestMetadata { pub cleanup: bool, } +impl TestMetadata { + pub fn should_run_on_os(&self, os: Os) -> bool { + self.target_os + .map(|target_os| target_os == os) + .unwrap_or(true) + } +} + // Register our test metadata struct with inventory to allow submitting tests of this type. inventory::collect!(TestMetadata); diff --git a/test/test-manager/src/tests/tunnel.rs b/test/test-manager/src/tests/tunnel.rs index d36ba4febe1a..9544f098b21c 100644 --- a/test/test-manager/src/tests/tunnel.rs +++ b/test/test-manager/src/tests/tunnel.rs @@ -1,7 +1,7 @@ use super::helpers::{ self, connect_and_wait, disconnect_and_wait, set_bridge_settings, set_relay_settings, }; -use super::{Error, TestContext}; +use super::{config::TEST_CONFIG, Error, TestContext}; use crate::network_monitor::{start_packet_monitor, MonitorOptions}; use mullvad_management_interface::{types, ManagementServiceClient}; @@ -502,7 +502,7 @@ async fn check_tunnel_psk( mullvad_client: &ManagementServiceClient, should_have_psk: bool, ) { - match rpc.get_os().await.expect("failed to get OS") { + match TEST_CONFIG.os { Os::Linux => { let name = helpers::get_tunnel_interface(mullvad_client.clone()) .await diff --git a/test/test-manager/src/tests/ui.rs b/test/test-manager/src/tests/ui.rs index 3600eb1d27ac..461d20ae108b 100644 --- a/test/test-manager/src/tests/ui.rs +++ b/test/test-manager/src/tests/ui.rs @@ -32,7 +32,7 @@ pub async fn run_test_env< let new_params: Vec; let bin_path; - match rpc.get_os().await? { + match TEST_CONFIG.os { Os::Linux => { bin_path = PathBuf::from("/usr/bin/xvfb-run"); diff --git a/test/test-manager/test_macro/src/lib.rs b/test/test-manager/test_macro/src/lib.rs index b82b796eba55..07f353ad7a92 100644 --- a/test/test-manager/test_macro/src/lib.rs +++ b/test/test-manager/test_macro/src/lib.rs @@ -37,6 +37,9 @@ use syn::{AttributeArgs, Lit, Meta, NestedMeta}; /// filters are provided by the user. /// `always_run` defaults to false. /// +/// * `target_os` - The test should only run on the specified OS. This can currently be +/// set to `linux`, `windows`, or `macos`. +/// /// # Examples /// /// ## Create a standard test. @@ -102,12 +105,14 @@ fn get_test_macro_parameters(attributes: &syn::AttributeArgs) -> MacroParameters let mut cleanup = true; let mut always_run = false; let mut must_succeed = false; + let mut target_os = None; + for attribute in attributes { if let NestedMeta::Meta(Meta::NameValue(nv)) = attribute { if nv.path.is_ident("priority") { match &nv.lit { Lit::Int(lit_int) => { - priority = Some(lit_int.clone()); + priority = Some(lit_int.base10_parse().unwrap()); } _ => panic!("'priority' should have an integer value"), } @@ -132,6 +137,13 @@ fn get_test_macro_parameters(attributes: &syn::AttributeArgs) -> MacroParameters } _ => panic!("'cleanup' should have a bool value"), } + } else if nv.path.is_ident("target_os") { + match &nv.lit { + Lit::Str(lit_str) => { + target_os = Some(lit_str.value()); + } + _ => panic!("'target_os' should have a string value"), + } } } } @@ -141,13 +153,22 @@ fn get_test_macro_parameters(attributes: &syn::AttributeArgs) -> MacroParameters cleanup, always_run, must_succeed, + target_os, } } fn create_test(test_function: TestFunction) -> proc_macro2::TokenStream { let test_function_priority = match test_function.macro_parameters.priority { - Some(priority) => quote! {Some(#priority)}, - None => quote! {None}, + Some(priority) => quote! { Some(#priority) }, + None => quote! { None }, + }; + let target_os = match test_function.macro_parameters.target_os { + Some(target_os) => { + quote! { + Some(::std::str::FromStr::from_str(#target_os).expect("invalid target os")) + } + } + None => quote! { None }, }; let should_cleanup = test_function.macro_parameters.cleanup; let always_run = test_function.macro_parameters.always_run; @@ -191,6 +212,7 @@ fn create_test(test_function: TestFunction) -> proc_macro2::TokenStream { inventory::submit!(crate::tests::test_metadata::TestMetadata { name: stringify!(#func_name), command: stringify!(#func_name), + target_os: #target_os, mullvad_client_version: #function_mullvad_version, func: Box::new(#wrapper_closure), priority: #test_function_priority, @@ -208,10 +230,11 @@ struct TestFunction { } struct MacroParameters { - priority: Option, + priority: Option, cleanup: bool, always_run: bool, must_succeed: bool, + target_os: Option, } enum MullvadClient { diff --git a/test/test-rpc/src/client.rs b/test/test-rpc/src/client.rs index 29f86b944370..2c47328e00f9 100644 --- a/test/test-rpc/src/client.rs +++ b/test/test-rpc/src/client.rs @@ -109,14 +109,6 @@ impl ServiceClient { .map_err(Error::Tarpc) } - /// Return the OS of the guest. - pub async fn get_os(&self) -> Result { - self.client - .get_os(tarpc::context::current()) - .await - .map_err(Error::Tarpc) - } - /// Wait for the Mullvad service to enter a specified state. The state is inferred from the presence /// of a named pipe or UDS, not the actual system service state. pub async fn mullvad_daemon_wait_for_state( diff --git a/test/test-rpc/src/lib.rs b/test/test-rpc/src/lib.rs index 73ab467ab8d4..5919a894d12c 100644 --- a/test/test-rpc/src/lib.rs +++ b/test/test-rpc/src/lib.rs @@ -111,9 +111,6 @@ mod service { async fn get_mullvad_app_logs() -> logging::LogOutput; - /// Return the OS of the guest. - async fn get_os() -> meta::Os; - /// Return status of the system service. async fn mullvad_daemon_get_status() -> mullvad_daemon::ServiceStatus; diff --git a/test/test-rpc/src/meta.rs b/test/test-rpc/src/meta.rs index 67c87738e03c..958733b9bedc 100644 --- a/test/test-rpc/src/meta.rs +++ b/test/test-rpc/src/meta.rs @@ -1,12 +1,26 @@ use serde::{Deserialize, Serialize}; +use std::str::FromStr; -#[derive(Debug, Serialize, Deserialize, PartialEq, Eq)] +#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Clone, Copy)] pub enum Os { Linux, Macos, Windows, } +impl FromStr for Os { + type Err = Box; + + fn from_str(s: &str) -> Result { + match s { + "linux" => Ok(Os::Linux), + "macos" => Ok(Os::Macos), + "windows" => Ok(Os::Windows), + other => Err(format!("unknown os {other}").into()), + } + } +} + impl std::fmt::Display for Os { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { @@ -16,12 +30,3 @@ impl std::fmt::Display for Os { } } } - -#[cfg(target_os = "linux")] -pub const CURRENT_OS: Os = Os::Linux; - -#[cfg(target_os = "windows")] -pub const CURRENT_OS: Os = Os::Windows; - -#[cfg(target_os = "macos")] -pub const CURRENT_OS: Os = Os::Macos; diff --git a/test/test-runner/src/main.rs b/test/test-runner/src/main.rs index c67876526bb7..d7ace9d15a25 100644 --- a/test/test-runner/src/main.rs +++ b/test/test-runner/src/main.rs @@ -9,7 +9,6 @@ use std::{ use tarpc::context; use tarpc::server::Channel; use test_rpc::{ - meta, mullvad_daemon::{ServiceStatus, SOCKET_PATH}, package::Package, transport::GrpcForwarder, @@ -96,10 +95,6 @@ impl Service for TestServer { Ok(result) } - async fn get_os(self, _: context::Context) -> meta::Os { - meta::CURRENT_OS - } - async fn mullvad_daemon_get_status( self, _: context::Context,