From c6f496aeddd67e3a8940796aab4cfbf2740effd8 Mon Sep 17 00:00:00 2001 From: Markus Pettersson Date: Fri, 22 Nov 2024 23:18:18 +0100 Subject: [PATCH 01/15] Implement FromStr on GeographicLocationConstraint --- mullvad-cli/src/cmds/relay.rs | 2 +- mullvad-cli/src/cmds/relay_constraints.rs | 61 ++++++++++++++--------- mullvad-types/src/relay_constraints.rs | 53 ++++++++++++++++++++ 3 files changed, 91 insertions(+), 25 deletions(-) diff --git a/mullvad-cli/src/cmds/relay.rs b/mullvad-cli/src/cmds/relay.rs index d0de98c27a96..0a0fa3b8e815 100644 --- a/mullvad-cli/src/cmds/relay.rs +++ b/mullvad-cli/src/cmds/relay.rs @@ -958,7 +958,7 @@ pub async fn resolve_location_constraint( } else { // The Constraint was not a relay, assuming it to be a location let location_constraint: Constraint = - Constraint::from(location_constraint_args); + Constraint::try_from(location_constraint_args)?; // If the location constraint was not "any", then validate the country/city if let Constraint::Only(constraint) = &location_constraint { diff --git a/mullvad-cli/src/cmds/relay_constraints.rs b/mullvad-cli/src/cmds/relay_constraints.rs index 97555997fcd1..9264d692bd4d 100644 --- a/mullvad-cli/src/cmds/relay_constraints.rs +++ b/mullvad-cli/src/cmds/relay_constraints.rs @@ -15,38 +15,51 @@ pub struct LocationArgs { pub hostname: Option, } -impl From for Constraint { - fn from(value: LocationArgs) -> Self { - if value.country.eq_ignore_ascii_case("any") { - return Constraint::Any; +#[derive(thiserror::Error, Debug)] +pub enum Error { + #[error("Failed to parse location constraint from input: TODO")] + Parse, +} + +impl TryFrom for GeographicLocationConstraint { + type Error = Error; + + fn try_from(value: LocationArgs) -> Result { + match (value.country, value.city, value.hostname) { + (country, None, None) => Ok(GeographicLocationConstraint::Country(country)), + (country, Some(city), None) => Ok(GeographicLocationConstraint::City(country, city)), + (country, Some(city), Some(hostname)) => Ok(GeographicLocationConstraint::Hostname( + country, city, hostname, + )), + _ => Err(Error::Parse), + //_ => unreachable!("invalid location arguments"), } + } +} - Constraint::Only(match (value.country, value.city, value.hostname) { - (country, None, None) => GeographicLocationConstraint::Country(country), - (country, Some(city), None) => GeographicLocationConstraint::City(country, city), - (country, Some(city), Some(hostname)) => { - GeographicLocationConstraint::Hostname(country, city, hostname) - } +impl TryFrom for LocationConstraint { + type Error = Error; - _ => unreachable!("invalid location arguments"), - }) + fn try_from(value: LocationArgs) -> Result { + GeographicLocationConstraint::try_from(value).map(LocationConstraint::from) } } -impl From for Constraint { - fn from(value: LocationArgs) -> Self { +impl TryFrom for Constraint { + type Error = Error; + + fn try_from(value: LocationArgs) -> Result { if value.country.eq_ignore_ascii_case("any") { - return Constraint::Any; + return Ok(Constraint::Any); } + GeographicLocationConstraint::try_from(value).map(Constraint::Only) + } +} + +impl TryFrom for Constraint { + type Error = Error; - let location = match (value.country, value.city, value.hostname) { - (country, None, None) => GeographicLocationConstraint::Country(country), - (country, Some(city), None) => GeographicLocationConstraint::City(country, city), - (country, Some(city), Some(hostname)) => { - GeographicLocationConstraint::Hostname(country, city, hostname) - } - _ => unreachable!("invalid location arguments"), - }; - Constraint::Only(LocationConstraint::Location(location)) + fn try_from(value: LocationArgs) -> Result { + LocationConstraint::try_from(value).map(Constraint::Only) } } diff --git a/mullvad-types/src/relay_constraints.rs b/mullvad-types/src/relay_constraints.rs index 4be7e25a4bac..e6eaa7ca55c0 100644 --- a/mullvad-types/src/relay_constraints.rs +++ b/mullvad-types/src/relay_constraints.rs @@ -174,6 +174,12 @@ pub enum GeographicLocationConstraint { Hostname(CountryCode, CityCode, Hostname), } +#[derive(thiserror::Error, Debug)] +#[error("Failed to parse {input} into a geographic location constraint")] +pub struct ParseGeoLocationError { + input: String, +} + impl GeographicLocationConstraint { /// Create a new [`GeographicLocationConstraint`] given a country. pub fn country(country: impl Into) -> Self { @@ -227,6 +233,27 @@ impl Match for GeographicLocationConstraint { } } +impl FromStr for GeographicLocationConstraint { + type Err = ParseGeoLocationError; + + // TODO: Implement for country and city as well? + fn from_str(input: &str) -> Result { + // A host name, such as "se-got-wg-101" maps to + // Country: se + // City: got + // hostname: se-got-wg-101 + let x = input.split("-").collect::>(); + match x[..] { + [country] => Ok(GeographicLocationConstraint::country(country)), + [country, city] => Ok(GeographicLocationConstraint::city(country, city)), + [country, city, ..] => Ok(GeographicLocationConstraint::hostname(country, city, input)), + _ => Err(ParseGeoLocationError { + input: input.to_string(), + }), + } + } +} + /// Limits the set of servers to choose based on ownership. #[derive(Debug, Copy, Clone, Eq, PartialEq, Deserialize, Serialize)] #[cfg_attr(feature = "clap", derive(clap::ValueEnum))] @@ -677,3 +704,29 @@ impl RelayOverride { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parse_hostname() { + // Parse a country + assert_eq!( + "se".parse::().unwrap(), + GeographicLocationConstraint::country("se") + ); + // Parse a city + assert_eq!( + "se-got".parse::().unwrap(), + GeographicLocationConstraint::city("se", "got") + ); + // Parse a hostname + assert_eq!( + "se-got-wg-101" + .parse::() + .unwrap(), + GeographicLocationConstraint::hostname("se", "got", "se-got-wg-101") + ); + } +} From 8e84c4280738c6b4966e5db5d3319b9698f2ab55 Mon Sep 17 00:00:00 2001 From: Markus Pettersson Date: Fri, 22 Nov 2024 18:10:12 +0100 Subject: [PATCH 02/15] Convert test-manager config into a module --- test/test-manager/src/config/error.rs | 15 ++ test/test-manager/src/config/io.rs | 80 ++++++++++ test/test-manager/src/config/manifest.rs | 47 ++++++ test/test-manager/src/config/mod.rs | 19 +++ .../src/{config.rs => config/vm.rs} | 140 +----------------- 5 files changed, 165 insertions(+), 136 deletions(-) create mode 100644 test/test-manager/src/config/error.rs create mode 100644 test/test-manager/src/config/io.rs create mode 100644 test/test-manager/src/config/manifest.rs create mode 100644 test/test-manager/src/config/mod.rs rename test/test-manager/src/{config.rs => config/vm.rs} (55%) diff --git a/test/test-manager/src/config/error.rs b/test/test-manager/src/config/error.rs new file mode 100644 index 000000000000..17ad599da9dd --- /dev/null +++ b/test/test-manager/src/config/error.rs @@ -0,0 +1,15 @@ +use std::io; + +#[derive(thiserror::Error, Debug)] +pub enum Error { + #[error("Could not find config dir")] + FindConfigDir, + #[error("Could not create config dir")] + CreateConfigDir(#[source] io::Error), + #[error("Failed to read config")] + Read(#[source] io::Error), + #[error("Failed to parse config")] + InvalidConfig(#[from] serde_json::Error), + #[error("Failed to write config")] + Write(#[source] io::Error), +} diff --git a/test/test-manager/src/config/io.rs b/test/test-manager/src/config/io.rs new file mode 100644 index 000000000000..2dcc05ca0bdf --- /dev/null +++ b/test/test-manager/src/config/io.rs @@ -0,0 +1,80 @@ +//! See [ConfigFile]. + +use std::io; +use std::ops::Deref; +use std::path::{Path, PathBuf}; + +use super::{Config, Error}; + +/// On-disk representation of [Config]. +pub struct ConfigFile { + path: PathBuf, + config: Config, +} + +impl ConfigFile { + /// Make config changes and save them to disk + pub async fn edit(&mut self, edit: impl FnOnce(&mut Config)) -> Result<(), Error> { + Self::ensure_config_dir().await?; + edit(&mut self.config); + self.config_save().await + } + + /// Make config changes and save them to disk + pub async fn load_or_default() -> Result { + let path = Self::get_config_path()?; + let config = Self::config_load_or_default(&path).await?; + let config_file = Self { path, config }; + Ok(config_file) + } + + async fn config_load_or_default>(path: P) -> Result { + Self::config_load(path).await.or_else(|error| match error { + Error::Read(ref io_err) if io_err.kind() == io::ErrorKind::NotFound => { + log::trace!("Failed to read config file"); + Ok(Config::default()) + } + error => Err(error), + }) + } + + async fn config_load>(path: P) -> Result { + let data = tokio::fs::read(path).await.map_err(Error::Read)?; + serde_json::from_slice(&data).map_err(Error::InvalidConfig) + } + + async fn config_save(&self) -> Result<(), Error> { + let data = serde_json::to_vec_pretty(&self.config).unwrap(); + tokio::fs::write(&self.path, &data) + .await + .map_err(Error::Write) + } + + /// Get configuration file path + fn get_config_path() -> Result { + Ok(Self::get_config_dir()?.join("config.json")) + } + + /// Get configuration file directory + fn get_config_dir() -> Result { + let dir = dirs::config_dir() + .ok_or(Error::FindConfigDir)? + .join("mullvad-test"); + Ok(dir) + } + + /// Create configuration file directory if it does not exist + async fn ensure_config_dir() -> Result<(), Error> { + tokio::fs::create_dir_all(Self::get_config_dir()?) + .await + .map_err(Error::CreateConfigDir) + } +} + +impl Deref for ConfigFile { + type Target = Config; + + fn deref(&self) -> &Self::Target { + &self.config + } +} diff --git a/test/test-manager/src/config/manifest.rs b/test/test-manager/src/config/manifest.rs new file mode 100644 index 000000000000..8ef96dc89d69 --- /dev/null +++ b/test/test-manager/src/config/manifest.rs @@ -0,0 +1,47 @@ +//! Config definition. +//! TODO: Document struct and link to that documentation + +use std::collections::BTreeMap; + +use serde::{Deserialize, Serialize}; + +use super::VmConfig; +use crate::tests::config::DEFAULT_MULLVAD_HOST; + +#[derive(Default, Serialize, Deserialize, Clone)] +pub struct Config { + #[serde(skip)] + pub runtime_opts: RuntimeOptions, + pub vms: BTreeMap, + pub mullvad_host: Option, +} + +#[derive(Default, Serialize, Deserialize, Clone)] +pub struct RuntimeOptions { + pub display: Display, + pub keep_changes: bool, +} + +#[derive(Default, Serialize, Deserialize, Clone)] +pub enum Display { + #[default] + None, + Local, + Vnc, +} + +impl Config { + pub fn get_vm(&self, name: &str) -> Option<&VmConfig> { + self.vms.get(name) + } + + /// Get the Mullvad host to use. + /// + /// Defaults to [`DEFAULT_MULLVAD_HOST`] if the host was not provided in the [`ConfigFile`]. + pub fn get_host(&self) -> String { + self.mullvad_host.clone().unwrap_or_else(|| { + log::debug!("No Mullvad host has been set explicitly. Falling back to default host"); + DEFAULT_MULLVAD_HOST.to_owned() + }) + } +} diff --git a/test/test-manager/src/config/mod.rs b/test/test-manager/src/config/mod.rs new file mode 100644 index 000000000000..4dc6e7248222 --- /dev/null +++ b/test/test-manager/src/config/mod.rs @@ -0,0 +1,19 @@ +//! Test manager configuration. + +mod error; +mod io; +mod manifest; +mod vm; + +use error::Error; +pub use io::ConfigFile; +pub use manifest::{Config, Display}; +pub use vm::{Architecture, OsType, PackageType, Provisioner, VmConfig, VmType}; + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parse_relay_location_per_test_override() {} +} diff --git a/test/test-manager/src/config.rs b/test/test-manager/src/config/vm.rs similarity index 55% rename from test/test-manager/src/config.rs rename to test/test-manager/src/config/vm.rs index 95a13c48a62a..911d2fcf6406 100644 --- a/test/test-manager/src/config.rs +++ b/test/test-manager/src/config/vm.rs @@ -1,141 +1,9 @@ -//! Test manager configuration. +//! Virtual machine configuration. -use serde::{Deserialize, Serialize}; -use std::{ - collections::BTreeMap, - env, io, - ops::Deref, - path::{Path, PathBuf}, -}; - -use crate::tests::config::DEFAULT_MULLVAD_HOST; - -#[derive(thiserror::Error, Debug)] -pub enum Error { - #[error("Could not find config dir")] - FindConfigDir, - #[error("Could not create config dir")] - CreateConfigDir(#[source] io::Error), - #[error("Failed to read config")] - Read(#[source] io::Error), - #[error("Failed to parse config")] - InvalidConfig(#[from] serde_json::Error), - #[error("Failed to write config")] - Write(#[source] io::Error), -} - -#[derive(Default, Serialize, Deserialize, Clone)] -pub struct Config { - #[serde(skip)] - pub runtime_opts: RuntimeOptions, - pub vms: BTreeMap, - pub mullvad_host: Option, -} - -#[derive(Default, Serialize, Deserialize, Clone)] -pub struct RuntimeOptions { - pub display: Display, - pub keep_changes: bool, -} - -#[derive(Default, Serialize, Deserialize, Clone)] -pub enum Display { - #[default] - None, - Local, - Vnc, -} - -impl Config { - async fn load_or_default>(path: P) -> Result { - Self::load(path).await.or_else(|error| match error { - Error::Read(ref io_err) if io_err.kind() == io::ErrorKind::NotFound => { - Ok(Self::default()) - } - error => Err(error), - }) - } - - async fn load>(path: P) -> Result { - let data = tokio::fs::read(path).await.map_err(Error::Read)?; - serde_json::from_slice(&data).map_err(Error::InvalidConfig) - } - - async fn save>(&self, path: P) -> Result<(), Error> { - let data = serde_json::to_vec_pretty(self).unwrap(); - tokio::fs::write(path, &data).await.map_err(Error::Write) - } - - pub fn get_vm(&self, name: &str) -> Option<&VmConfig> { - self.vms.get(name) - } - - /// Get the Mullvad host to use. - /// - /// Defaults to [`DEFAULT_MULLVAD_HOST`] if the host was not provided in the [`ConfigFile`]. - pub fn get_host(&self) -> String { - self.mullvad_host.clone().unwrap_or_else(|| { - log::debug!("No Mullvad host has been set explicitly. Falling back to default host"); - DEFAULT_MULLVAD_HOST.to_owned() - }) - } -} - -pub struct ConfigFile { - path: PathBuf, - config: Config, -} - -impl ConfigFile { - /// Make config changes and save them to disk - pub async fn load_or_default() -> Result { - Self::load_or_default_inner(Self::get_config_path()?).await - } - - /// Get configuration file path - fn get_config_path() -> Result { - Ok(Self::get_config_dir()?.join("config.json")) - } - - /// Get configuration file directory - fn get_config_dir() -> Result { - let dir = dirs::config_dir() - .ok_or(Error::FindConfigDir)? - .join("mullvad-test"); - Ok(dir) - } - - /// Create configuration file directory if it does not exist - async fn ensure_config_dir() -> Result<(), Error> { - tokio::fs::create_dir_all(Self::get_config_dir()?) - .await - .map_err(Error::CreateConfigDir) - } +use std::env; +use std::path::{Path, PathBuf}; - /// Make config changes and save them to disk - async fn load_or_default_inner>(path: P) -> Result { - Ok(Self { - path: path.as_ref().to_path_buf(), - config: Config::load_or_default(path).await?, - }) - } - - /// Make config changes and save them to disk - pub async fn edit(&mut self, edit: impl FnOnce(&mut Config)) -> Result<(), Error> { - Self::ensure_config_dir().await?; - - edit(&mut self.config); - self.config.save(&self.path).await - } -} - -impl Deref for ConfigFile { - type Target = Config; - - fn deref(&self) -> &Self::Target { - &self.config - } -} +use serde::{Deserialize, Serialize}; #[derive(clap::Args, Debug, Serialize, Deserialize, Clone)] #[serde(rename_all = "snake_case")] From 4fc6711f70f4bf8d88b9f50e7d96aee890ba4958 Mon Sep 17 00:00:00 2001 From: Markus Pettersson Date: Fri, 22 Nov 2024 19:05:31 +0100 Subject: [PATCH 03/15] WIP Add location overrides per test to test-manager config --- test/Cargo.lock | 1 + test/test-manager/Cargo.toml | 1 + test/test-manager/src/config/manifest.rs | 116 +++++++++++++++++++++++ test/test-manager/src/config/mod.rs | 8 -- 4 files changed, 118 insertions(+), 8 deletions(-) diff --git a/test/Cargo.lock b/test/Cargo.lock index a4e2ee9df70a..064ae149ad1f 100644 --- a/test/Cargo.lock +++ b/test/Cargo.lock @@ -3597,6 +3597,7 @@ dependencies = [ "dirs", "env_logger", "futures", + "glob", "hyper-util", "inventory", "ipnetwork", diff --git a/test/test-manager/Cargo.toml b/test/test-manager/Cargo.toml index 2671ea454a4d..3310ab770fdf 100644 --- a/test/test-manager/Cargo.toml +++ b/test/test-manager/Cargo.toml @@ -32,6 +32,7 @@ async-trait = { workspace = true } uuid = "1.3" dirs = "5.0.1" scopeguard = "1.2" +glob = "0.3" serde = { workspace = true } serde_json = { workspace = true } diff --git a/test/test-manager/src/config/manifest.rs b/test/test-manager/src/config/manifest.rs index 8ef96dc89d69..775d66376fd7 100644 --- a/test/test-manager/src/config/manifest.rs +++ b/test/test-manager/src/config/manifest.rs @@ -14,6 +14,14 @@ pub struct Config { pub runtime_opts: RuntimeOptions, pub vms: BTreeMap, pub mullvad_host: Option, + /// Add location override on a per-test basis. These those locations will be the + /// only available options for the given test to pick from! + /// + /// Glob patterns are used to targeet one or more tests with a set of locations. If there are multiple + /// patterns that match with a test name, only the first match will be considered. The ordering + /// is just like a regular JSON list. + // TODO: Make sure this is not serialized into null + pub location: Option, } #[derive(Default, Serialize, Deserialize, Clone)] @@ -45,3 +53,111 @@ impl Config { }) } } + +mod locations { + use std::collections::BTreeMap; + use std::ops::Deref; + + use serde::{de::Visitor, Deserialize, Serialize}; + + #[derive(Serialize, Deserialize, Clone, Default)] + pub struct Locations { + pub r#override: Override, + } + + impl Locations {} + + #[derive(Serialize, Deserialize, Clone)] + pub struct Override(BTreeMap>); + + impl Default for Override { + /// All tests default to using the "any" location. + /// Written out in a config it would look like the following: { "*": ["any"] } + fn default() -> Self { + let overrides = { + let mut overrides = BTreeMap::new(); + let glob = SerializeableGlob::from(glob::Pattern::new("*").unwrap()); + overrides.insert(glob, vec!["any".to_string()]); + overrides + }; + Override(overrides) + } + } + + /// Implement serde [Serialize] and [Deserialize] for [glob::Pattern]. + #[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Default, Debug)] + struct SerializeableGlob(glob::Pattern); + + impl<'de> Deserialize<'de> for SerializeableGlob { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + let glob = deserializer.deserialize_string(GlobVisitor)?; + Ok(SerializeableGlob(glob)) + } + } + + impl Serialize for SerializeableGlob { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + let glob = self.0.as_str(); + serializer.serialize_str(glob) + } + } + + impl From for SerializeableGlob { + fn from(pattern: glob::Pattern) -> Self { + Self(pattern) + } + } + + impl Deref for SerializeableGlob { + type Target = glob::Pattern; + + fn deref(&self) -> &Self::Target { + &self.0 + } + } + + struct GlobVisitor; + + impl Visitor<'_> for GlobVisitor { + type Value = glob::Pattern; + + fn expecting(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + formatter.write_str("Only strings can be deserialised to glob pattern") + } + + fn visit_str(self, v: &str) -> Result + where + E: serde::de::Error, + { + glob::Pattern::new(v).map_err(|err| { + E::custom(format!( + "Cannot compile glob pattern from: {v} error: {err:?}" + )) + }) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parse_relay_location_per_test_override() { + let config = " + { + \"vms\": {}, + \"mullvad_host\": \"mullvad.net\", + \"location\": { \"override\": { \"*\": [\"Low Latency\"] } } + }"; + + let config: Config = serde_json::from_str(config).unwrap(); + let _location = config.location.expect("location overrides was not parsed"); + } +} diff --git a/test/test-manager/src/config/mod.rs b/test/test-manager/src/config/mod.rs index 4dc6e7248222..4b510b87cc7f 100644 --- a/test/test-manager/src/config/mod.rs +++ b/test/test-manager/src/config/mod.rs @@ -9,11 +9,3 @@ use error::Error; pub use io::ConfigFile; pub use manifest::{Config, Display}; pub use vm::{Architecture, OsType, PackageType, Provisioner, VmConfig, VmType}; - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn parse_relay_location_per_test_override() {} -} From d9abb1c52634f03030497adfdaebb3a86c042043 Mon Sep 17 00:00:00 2001 From: Markus Pettersson Date: Fri, 22 Nov 2024 21:29:06 +0100 Subject: [PATCH 04/15] fixup! WIP Add location overrides per test to test-manager config --- test/test-manager/src/config/manifest.rs | 52 ++++++++++++++++---- test/test-manager/src/main.rs | 11 ++++- test/test-manager/src/run_tests.rs | 5 +- test/test-manager/src/tests/helpers.rs | 2 +- test/test-manager/src/tests/mod.rs | 46 ++++++++++++++++- test/test-manager/src/tests/test_metadata.rs | 4 +- test/test-manager/test_macro/src/lib.rs | 1 + test/test-rpc/src/mullvad_daemon.rs | 2 +- 8 files changed, 108 insertions(+), 15 deletions(-) diff --git a/test/test-manager/src/config/manifest.rs b/test/test-manager/src/config/manifest.rs index 775d66376fd7..afd8b7197eb5 100644 --- a/test/test-manager/src/config/manifest.rs +++ b/test/test-manager/src/config/manifest.rs @@ -55,32 +55,66 @@ impl Config { } mod locations { - use std::collections::BTreeMap; use std::ops::Deref; use serde::{de::Visitor, Deserialize, Serialize}; + // "location": { + // "override": [ + // { "test": "test_daita", locations: ["se-got-101"] }, + // { "test": "*", locations: ["Nordic"] } + // ] + // }, + #[derive(Serialize, Deserialize, Clone, Default)] pub struct Locations { - pub r#override: Override, + pub r#override: Overrides, } - impl Locations {} + impl Locations { + // Look up a test (by name) and see if there are any locations + // that we should use. + pub fn lookup(&self, test: &str) -> Option<&Vec> { + self.r#override.lookup(test) + } + } + + /// Mapping of glob pattern to a set of locations. + #[derive(Serialize, Deserialize, Clone)] + pub struct Overrides(Vec); #[derive(Serialize, Deserialize, Clone)] - pub struct Override(BTreeMap>); + pub struct Override { + test: SerializeableGlob, + locations: Vec, + } + + impl Overrides { + // Lookup the first test that matches a glob pattern. + fn lookup(&self, test: &str) -> Option<&Vec> { + self.0 + .iter() + .find( + |Override { + test: test_glob, .. + }| test_glob.matches(test), + ) + .map(|Override { locations, .. }| locations) + } + } - impl Default for Override { + impl Default for Overrides { /// All tests default to using the "any" location. /// Written out in a config it would look like the following: { "*": ["any"] } fn default() -> Self { let overrides = { - let mut overrides = BTreeMap::new(); let glob = SerializeableGlob::from(glob::Pattern::new("*").unwrap()); - overrides.insert(glob, vec!["any".to_string()]); - overrides + vec![Override { + test: glob, + locations: vec!["any".to_string()], + }] }; - Override(overrides) + Overrides(overrides) } } diff --git a/test/test-manager/src/main.rs b/test/test-manager/src/main.rs index 95274e9a7a8e..0c467d2cc2f9 100644 --- a/test/test-manager/src/main.rs +++ b/test/test-manager/src/main.rs @@ -327,7 +327,16 @@ async fn main() -> Result<()> { test_rpc::meta::Os::from(vm_config.os_type), openvpn_certificate, )); - let tests = get_filtered_tests(&test_filters)?; + let tests = { + let mut tests = get_filtered_tests(&test_filters)?; + // Fill in location overrides + if let Some(locations) = &config.location { + for test in tests.iter_mut() { + test.location = locations.lookup(test.name).cloned(); + } + } + tests + }; // For convenience, spawn a SOCKS5 server that is reachable for tests that need it let socks = socks_server::spawn(SocketAddr::new( diff --git a/test/test-manager/src/run_tests.rs b/test/test-manager/src/run_tests.rs index 88577fb3b3cd..001f7fee5f33 100644 --- a/test/test-manager/src/run_tests.rs +++ b/test/test-manager/src/run_tests.rs @@ -157,13 +157,16 @@ pub async fn run( }; for test in tests { - tests::prepare_daemon(&test_runner_client, &rpc_provider) + let mut mullvad_client = tests::prepare_daemon(&test_runner_client, &rpc_provider) .await .context("Failed to reset daemon before test")?; + tests::prepare_custom_lists(&mut mullvad_client, &test).await?; + let mullvad_client = rpc_provider .mullvad_client(test.mullvad_client_version) .await; + test_handler .run_test(&test.func, test.name, mullvad_client) .await?; diff --git a/test/test-manager/src/tests/helpers.rs b/test/test-manager/src/tests/helpers.rs index ff581c8f6249..297b8eef498c 100644 --- a/test/test-manager/src/tests/helpers.rs +++ b/test/test-manager/src/tests/helpers.rs @@ -1344,7 +1344,7 @@ pub mod custom_lists { /// Dig out a custom list from the daemon settings based on the custom list's name. /// There should be an rpc for this. - async fn find_custom_list( + pub async fn find_custom_list( rpc: &mut MullvadProxyClient, name: &str, ) -> anyhow::Result { diff --git a/test/test-manager/src/tests/mod.rs b/test/test-manager/src/tests/mod.rs index 9eb23fe13e8d..36bb21962d82 100644 --- a/test/test-manager/src/tests/mod.rs +++ b/test/test-manager/src/tests/mod.rs @@ -16,6 +16,7 @@ mod tunnel_state; mod ui; use itertools::Itertools; +use mullvad_types::relay_constraints::GeographicLocationConstraint; pub use test_metadata::TestMetadata; use anyhow::Context; @@ -137,7 +138,7 @@ pub fn get_filtered_tests(specified_tests: &[String]) -> Result anyhow::Result<()> { +) -> anyhow::Result { // Check if daemon should be restarted let mut mullvad_client = ensure_daemon_version(rpc, rpc_provider) .await @@ -155,6 +156,49 @@ pub async fn prepare_daemon( helpers::custom_lists::add_default_lists(&mut mullvad_client).await?; helpers::custom_lists::set_default_location(&mut mullvad_client).await?; + Ok(mullvad_client) +} + +/// Create an "anonynmous" custom list for this test. The custom list will +/// have the same as the test and contain the locations as specified by [TestMetadata::location]. +pub async fn prepare_custom_lists( + mullvad_client: &mut MullvadProxyClient, + test: &TestMetadata, +) -> anyhow::Result<()> { + use helpers::custom_lists::find_custom_list; + // Convert locations from the test config to actual location constraints + let locations = { + let mut locations: Vec = vec![]; + for input in test.location.clone().unwrap() { + match input.parse::() { + Ok(location) => locations.push(location), + // If some location argument can not be parsed as a GeographicLocationconstraint, + // assume it is a custom list. + Err(_parse_error) => { + let custom_list = find_custom_list(mullvad_client, &input).await.unwrap(); + // Hack: Dig out all the geographic location constraints from this custom list + for custom_list_location in custom_list.locations { + locations.push(custom_list_location) + } + } + }; + } + locations + }; + + // Add the custom list to the current app instance + let id = mullvad_client + .create_custom_list(test.name.to_string()) + .await?; + + let mut custom_list = find_custom_list(mullvad_client, test.name).await?; + + assert_eq!(id, custom_list.id); + for location in locations { + custom_list.locations.insert(location); + } + mullvad_client.update_custom_list(custom_list).await?; + Ok(()) } diff --git a/test/test-manager/src/tests/test_metadata.rs b/test/test-manager/src/tests/test_metadata.rs index 79c7f74def7e..4235a6922fb4 100644 --- a/test/test-manager/src/tests/test_metadata.rs +++ b/test/test-manager/src/tests/test_metadata.rs @@ -1,13 +1,15 @@ use super::TestWrapperFunction; use test_rpc::{meta::Os, mullvad_daemon::MullvadClientVersion}; -#[derive(Clone)] +#[derive(Clone, Debug)] pub struct TestMetadata { pub name: &'static str, pub targets: &'static [Os], pub mullvad_client_version: MullvadClientVersion, pub func: TestWrapperFunction, pub priority: Option, + // TODO: Document + pub location: Option>, } // Register our test metadata struct with inventory to allow submitting tests of this type. diff --git a/test/test-manager/test_macro/src/lib.rs b/test/test-manager/test_macro/src/lib.rs index cddb6c5a2f6d..e16968bd9db8 100644 --- a/test/test-manager/test_macro/src/lib.rs +++ b/test/test-manager/test_macro/src/lib.rs @@ -191,6 +191,7 @@ fn create_test(test_function: TestFunction) -> proc_macro2::TokenStream { mullvad_client_version: #function_mullvad_version, func: #wrapper_closure, priority: #test_function_priority, + location: None, }); } } diff --git a/test/test-rpc/src/mullvad_daemon.rs b/test/test-rpc/src/mullvad_daemon.rs index 10cc00c3fc96..e9865440b963 100644 --- a/test/test-rpc/src/mullvad_daemon.rs +++ b/test/test-rpc/src/mullvad_daemon.rs @@ -27,7 +27,7 @@ pub enum Verbosity { Trace, } -#[derive(Clone, Copy, PartialEq)] +#[derive(Clone, Copy, Debug, PartialEq)] pub enum MullvadClientVersion { None, New, From 8b8bdc38dce09283af995372a771c8ebb372a9e8 Mon Sep 17 00:00:00 2001 From: Markus Pettersson Date: Fri, 22 Nov 2024 23:30:38 +0100 Subject: [PATCH 05/15] fixup! fixup! WIP Add location overrides per test to test-manager config --- test/test-manager/src/tests/mod.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/test-manager/src/tests/mod.rs b/test/test-manager/src/tests/mod.rs index 36bb21962d82..48b400d2fdd3 100644 --- a/test/test-manager/src/tests/mod.rs +++ b/test/test-manager/src/tests/mod.rs @@ -192,12 +192,14 @@ pub async fn prepare_custom_lists( .await?; let mut custom_list = find_custom_list(mullvad_client, test.name).await?; + log::info!("Creating custom list {}", custom_list.name); assert_eq!(id, custom_list.id); for location in locations { custom_list.locations.insert(location); } mullvad_client.update_custom_list(custom_list).await?; + log::info!("Added custom list"); Ok(()) } From 949b2b7ed32374a506326cbb3f982782d7a471a1 Mon Sep 17 00:00:00 2001 From: Sebastian Holmin Date: Mon, 13 Jan 2025 14:22:41 +0100 Subject: [PATCH 06/15] Update parsing of test overrides to new spec --- test/test-manager/src/config/manifest.rs | 215 +++++++++++------------ test/test-manager/src/main.rs | 6 +- 2 files changed, 102 insertions(+), 119 deletions(-) diff --git a/test/test-manager/src/config/manifest.rs b/test/test-manager/src/config/manifest.rs index afd8b7197eb5..926cc0bedf6a 100644 --- a/test/test-manager/src/config/manifest.rs +++ b/test/test-manager/src/config/manifest.rs @@ -8,29 +8,39 @@ use serde::{Deserialize, Serialize}; use super::VmConfig; use crate::tests::config::DEFAULT_MULLVAD_HOST; -#[derive(Default, Serialize, Deserialize, Clone)] +#[derive(Debug, Default, Serialize, Deserialize, Clone)] pub struct Config { #[serde(skip)] pub runtime_opts: RuntimeOptions, pub vms: BTreeMap, pub mullvad_host: Option, - /// Add location override on a per-test basis. These those locations will be the - /// only available options for the given test to pick from! + /// Relay/location overrides for tests. /// - /// Glob patterns are used to targeet one or more tests with a set of locations. If there are multiple - /// patterns that match with a test name, only the first match will be considered. The ordering - /// is just like a regular JSON list. - // TODO: Make sure this is not serialized into null - pub location: Option, + /// Format: + /// ```json + /// { + /// // other fields + /// "test_locations": [ + /// { "daita": [ "se-got-wg-001", "se-got-wg-002" ] }, + /// { "*": [ "se" ] } + /// ] + /// } + /// ``` + /// + /// The above example will set the locations for the test `daita` to a custom list containing + /// `se-got-wg-001` and `se-got-wg-002`. The `*` is a wildcard that will match any test + /// name. The order of the list is important, as the first match will be used. + #[serde(default)] + pub test_locations: locations::TestLocationList, } -#[derive(Default, Serialize, Deserialize, Clone)] +#[derive(Debug, Default, Serialize, Deserialize, Clone)] pub struct RuntimeOptions { pub display: Display, pub keep_changes: bool, } -#[derive(Default, Serialize, Deserialize, Clone)] +#[derive(Debug, Default, Serialize, Deserialize, Clone)] pub enum Display { #[default] None, @@ -55,125 +65,85 @@ impl Config { } mod locations { - use std::ops::Deref; - - use serde::{de::Visitor, Deserialize, Serialize}; - - // "location": { - // "override": [ - // { "test": "test_daita", locations: ["se-got-101"] }, - // { "test": "*", locations: ["Nordic"] } - // ] - // }, - - #[derive(Serialize, Deserialize, Clone, Default)] - pub struct Locations { - pub r#override: Overrides, - } - - impl Locations { - // Look up a test (by name) and see if there are any locations - // that we should use. + use serde::{ + de::{Deserialize, Deserializer, Error, MapAccess, Visitor}, + ser::{Serialize, SerializeMap}, + Deserialize as DeserDerive, Serialize as SerDerive, + }; + use std::fmt; + + #[derive(Debug, Clone, Default)] + pub struct TestLocation(glob::Pattern, Vec); + + #[derive(Debug, DeserDerive, SerDerive, Clone, Default)] + pub struct TestLocationList(pub Vec); + + impl TestLocationList { + // TODO: Consider if we should handle the case of an empty list by returning vec!["any"] + /// Look up a test (by name) and see if there are any locations + /// that we should use. pub fn lookup(&self, test: &str) -> Option<&Vec> { - self.r#override.lookup(test) - } - } - - /// Mapping of glob pattern to a set of locations. - #[derive(Serialize, Deserialize, Clone)] - pub struct Overrides(Vec); - - #[derive(Serialize, Deserialize, Clone)] - pub struct Override { - test: SerializeableGlob, - locations: Vec, - } - - impl Overrides { - // Lookup the first test that matches a glob pattern. - fn lookup(&self, test: &str) -> Option<&Vec> { self.0 .iter() - .find( - |Override { - test: test_glob, .. - }| test_glob.matches(test), - ) - .map(|Override { locations, .. }| locations) + .find(|TestLocation(test_glob, _)| test_glob.matches(test)) + .map(|TestLocation(_, locations)| locations) } } - impl Default for Overrides { - /// All tests default to using the "any" location. - /// Written out in a config it would look like the following: { "*": ["any"] } - fn default() -> Self { - let overrides = { - let glob = SerializeableGlob::from(glob::Pattern::new("*").unwrap()); - vec![Override { - test: glob, - locations: vec!["any".to_string()], - }] - }; - Overrides(overrides) - } - } + struct TestLocationVisitor; - /// Implement serde [Serialize] and [Deserialize] for [glob::Pattern]. - #[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Default, Debug)] - struct SerializeableGlob(glob::Pattern); + impl<'de> Visitor<'de> for TestLocationVisitor { + // The type that our Visitor is going to produce. + type Value = TestLocation; - impl<'de> Deserialize<'de> for SerializeableGlob { - fn deserialize(deserializer: D) -> Result - where - D: serde::Deserializer<'de>, - { - let glob = deserializer.deserialize_string(GlobVisitor)?; - Ok(SerializeableGlob(glob)) + // Format a message stating what data this Visitor expects to receive. + fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str("A list of maps") } - } - impl Serialize for SerializeableGlob { - fn serialize(&self, serializer: S) -> Result + fn visit_map(self, mut access: M) -> Result where - S: serde::Serializer, + M: MapAccess<'de>, { - let glob = self.0.as_str(); - serializer.serialize_str(glob) - } - } - - impl From for SerializeableGlob { - fn from(pattern: glob::Pattern) -> Self { - Self(pattern) - } - } + let (key, value) = + access + .next_entry::>()? + .ok_or(M::Error::custom( + "Test location map should contain exactly one key-value pair, but it was empty", + ))?; + let glob = glob::Pattern::new(&key).map_err(|err| { + M::Error::custom(format!( + "Cannot compile glob pattern from: {key} error: {err:?}" + )) + })?; - impl Deref for SerializeableGlob { - type Target = glob::Pattern; + if let Some((key, value)) = access.next_entry::>()? { + return Err(M::Error::custom(format!( + "Test location map should contain exactly one key-value pair, but found another key: '{key}' and value: '{value:?}'" + ))); + } - fn deref(&self) -> &Self::Target { - &self.0 + Ok(TestLocation(glob, value)) } } - struct GlobVisitor; - - impl Visitor<'_> for GlobVisitor { - type Value = glob::Pattern; - - fn expecting(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - formatter.write_str("Only strings can be deserialised to glob pattern") + impl<'de> Deserialize<'de> for TestLocation { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + deserializer.deserialize_map(TestLocationVisitor) } + } - fn visit_str(self, v: &str) -> Result + impl Serialize for TestLocation { + fn serialize(&self, serializer: S) -> Result where - E: serde::de::Error, + S: serde::Serializer, { - glob::Pattern::new(v).map_err(|err| { - E::custom(format!( - "Cannot compile glob pattern from: {v} error: {err:?}" - )) - }) + let mut map = serializer.serialize_map(Some(1))?; + map.serialize_entry(self.0.as_str(), &self.1)?; + map.end() } } } @@ -183,15 +153,30 @@ mod tests { use super::*; #[test] - fn parse_relay_location_per_test_override() { - let config = " + fn parse_test_location_empty() { + let config = r#" + { + "vms": {}, + "mullvad_host": "mullvad.net" + }"#; + + let config: Config = serde_json::from_str(config).unwrap(); + assert!(config.test_locations.0.is_empty()); + } + + #[test] + fn parse_test_location_not_empty() { + let config = r#" { - \"vms\": {}, - \"mullvad_host\": \"mullvad.net\", - \"location\": { \"override\": { \"*\": [\"Low Latency\"] } } - }"; + "vms": {}, + "mullvad_host": "mullvad.net", + "test_locations": [ + { "daita": [ "se-got-wg-001", "se-got-wg-002" ] }, + { "*": [ "se" ] } + ] + }"#; let config: Config = serde_json::from_str(config).unwrap(); - let _location = config.location.expect("location overrides was not parsed"); + assert!(!config.test_locations.0.is_empty()); } } diff --git a/test/test-manager/src/main.rs b/test/test-manager/src/main.rs index 0c467d2cc2f9..a1d6f6988c46 100644 --- a/test/test-manager/src/main.rs +++ b/test/test-manager/src/main.rs @@ -330,10 +330,8 @@ async fn main() -> Result<()> { let tests = { let mut tests = get_filtered_tests(&test_filters)?; // Fill in location overrides - if let Some(locations) = &config.location { - for test in tests.iter_mut() { - test.location = locations.lookup(test.name).cloned(); - } + for test in tests.iter_mut() { + test.location = config.test_locations.lookup(test.name).cloned(); } tests }; From 8c79791b027ffbc4031a0ea16cf8c0680f2a89f0 Mon Sep 17 00:00:00 2001 From: Sebastian Holmin Date: Mon, 13 Jan 2025 15:41:19 +0100 Subject: [PATCH 07/15] Misc changes --- test/test-manager/src/main.rs | 13 +++++++++++-- test/test-manager/src/tests/mod.rs | 5 +++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/test/test-manager/src/main.rs b/test/test-manager/src/main.rs index a1d6f6988c46..e3bfd27e2ead 100644 --- a/test/test-manager/src/main.rs +++ b/test/test-manager/src/main.rs @@ -266,8 +266,17 @@ async fn main() -> Result<()> { }; if let Some(mullvad_host) = mullvad_host { - log::trace!("Setting Mullvad host using --mullvad-host flag"); - config.mullvad_host = Some(mullvad_host); + if let Some(old_host) = config.mullvad_host.replace(mullvad_host) { + log::info!( + "Overriding Mullvad host from {old_host} to {}", + config.mullvad_host.as_ref().unwrap() + ); + } else { + log::info!( + "Setting Mullvad host to {}", + config.mullvad_host.as_ref().unwrap() + ); + } } let mullvad_host = config.get_host(); log::debug!("Mullvad host: {mullvad_host}"); diff --git a/test/test-manager/src/tests/mod.rs b/test/test-manager/src/tests/mod.rs index 48b400d2fdd3..8372d2f8a3bf 100644 --- a/test/test-manager/src/tests/mod.rs +++ b/test/test-manager/src/tests/mod.rs @@ -159,8 +159,9 @@ pub async fn prepare_daemon( Ok(mullvad_client) } -/// Create an "anonynmous" custom list for this test. The custom list will -/// have the same as the test and contain the locations as specified by [TestMetadata::location]. +/// Create an "anonymous" custom list for this test. The custom list will +/// have the same name as the test and contain the locations as specified by +/// [TestMetadata::location]. pub async fn prepare_custom_lists( mullvad_client: &mut MullvadProxyClient, test: &TestMetadata, From a370554212f8309bff2ceefe476d8286840c8616 Mon Sep 17 00:00:00 2001 From: Sebastian Holmin Date: Tue, 14 Jan 2025 09:10:57 +0100 Subject: [PATCH 08/15] Improve logs and errors --- test/test-manager/src/tests/mod.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/test/test-manager/src/tests/mod.rs b/test/test-manager/src/tests/mod.rs index 8372d2f8a3bf..d353f71e1b03 100644 --- a/test/test-manager/src/tests/mod.rs +++ b/test/test-manager/src/tests/mod.rs @@ -176,7 +176,9 @@ pub async fn prepare_custom_lists( // If some location argument can not be parsed as a GeographicLocationconstraint, // assume it is a custom list. Err(_parse_error) => { - let custom_list = find_custom_list(mullvad_client, &input).await.unwrap(); + let custom_list = find_custom_list(mullvad_client, &input) + .await + .with_context(|| format!("Failed to parse test location '{input}'"))?; // Hack: Dig out all the geographic location constraints from this custom list for custom_list_location in custom_list.locations { locations.push(custom_list_location) @@ -187,20 +189,25 @@ pub async fn prepare_custom_lists( locations }; + log::debug!( + "Creating custom list {} with locations '{:?}'", + test.name, + locations + ); + // Add the custom list to the current app instance let id = mullvad_client .create_custom_list(test.name.to_string()) .await?; let mut custom_list = find_custom_list(mullvad_client, test.name).await?; - log::info!("Creating custom list {}", custom_list.name); assert_eq!(id, custom_list.id); for location in locations { custom_list.locations.insert(location); } mullvad_client.update_custom_list(custom_list).await?; - log::info!("Added custom list"); + log::debug!("Added custom list"); Ok(()) } From 08d2c57de06efb72c179fe72a980ba06c79bae3b Mon Sep 17 00:00:00 2001 From: Sebastian Holmin Date: Tue, 14 Jan 2025 10:05:15 +0100 Subject: [PATCH 09/15] Improve docs of `test_locations` field --- test/test-manager/src/config/manifest.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/test/test-manager/src/config/manifest.rs b/test/test-manager/src/config/manifest.rs index 926cc0bedf6a..f6ee47f85494 100644 --- a/test/test-manager/src/config/manifest.rs +++ b/test/test-manager/src/config/manifest.rs @@ -14,22 +14,24 @@ pub struct Config { pub runtime_opts: RuntimeOptions, pub vms: BTreeMap, pub mullvad_host: Option, - /// Relay/location overrides for tests. + /// Relay/location overrides for tests. The format is a list of maps, where the key is a glob + /// pattern that will be matched against the test name, and the value is a list of locations to + /// use for that test. The first match will be used. /// - /// Format: + /// Example: /// ```json /// { /// // other fields /// "test_locations": [ - /// { "daita": [ "se-got-wg-001", "se-got-wg-002" ] }, + /// { "*daita*": [ "se-got-wg-001", "se-got-wg-002" ] }, /// { "*": [ "se" ] } /// ] /// } /// ``` /// - /// The above example will set the locations for the test `daita` to a custom list containing - /// `se-got-wg-001` and `se-got-wg-002`. The `*` is a wildcard that will match any test - /// name. The order of the list is important, as the first match will be used. + /// The above example will set the locations for the test `test_daita` to a custom list + /// containing `se-got-wg-001` and `se-got-wg-002`. The `*` is a wildcard that will match + /// any test name. The order of the list is important, as the first match will be used. #[serde(default)] pub test_locations: locations::TestLocationList, } From ded2e585909d511ba13096a637bf4d0efa65d76b Mon Sep 17 00:00:00 2001 From: Sebastian Holmin Date: Tue, 14 Jan 2025 11:57:16 +0100 Subject: [PATCH 10/15] Remove predefined custom list --- test/test-manager/src/main.rs | 13 +- test/test-manager/src/run_tests.rs | 4 +- test/test-manager/src/tests/helpers.rs | 188 +++++-------------------- test/test-manager/src/tests/mod.rs | 52 +++---- test/test-manager/src/tests/tunnel.rs | 42 +----- test/test-manager/src/tests/ui.rs | 10 +- 6 files changed, 71 insertions(+), 238 deletions(-) diff --git a/test/test-manager/src/main.rs b/test/test-manager/src/main.rs index e3bfd27e2ead..42112985688c 100644 --- a/test/test-manager/src/main.rs +++ b/test/test-manager/src/main.rs @@ -336,14 +336,11 @@ async fn main() -> Result<()> { test_rpc::meta::Os::from(vm_config.os_type), openvpn_certificate, )); - let tests = { - let mut tests = get_filtered_tests(&test_filters)?; - // Fill in location overrides - for test in tests.iter_mut() { - test.location = config.test_locations.lookup(test.name).cloned(); - } - tests - }; + + let mut tests = get_filtered_tests(&test_filters)?; + for test in tests.iter_mut() { + test.location = config.test_locations.lookup(test.name).cloned(); + } // For convenience, spawn a SOCKS5 server that is reachable for tests that need it let socks = socks_server::spawn(SocketAddr::new( diff --git a/test/test-manager/src/run_tests.rs b/test/test-manager/src/run_tests.rs index 001f7fee5f33..23c7d1f8c5bb 100644 --- a/test/test-manager/src/run_tests.rs +++ b/test/test-manager/src/run_tests.rs @@ -161,7 +161,9 @@ pub async fn run( .await .context("Failed to reset daemon before test")?; - tests::prepare_custom_lists(&mut mullvad_client, &test).await?; + tests::set_test_location(&mut mullvad_client, &test) + .await + .context("Failed to create custom list from test locations")?; let mullvad_client = rpc_provider .mullvad_client(test.mullvad_client_version) diff --git a/test/test-manager/src/tests/helpers.rs b/test/test-manager/src/tests/helpers.rs index 297b8eef498c..8d1032e136cf 100644 --- a/test/test-manager/src/tests/helpers.rs +++ b/test/test-manager/src/tests/helpers.rs @@ -17,6 +17,7 @@ use mullvad_relay_selector::{ }; use mullvad_types::{ constraints::Constraint, + custom_list::CustomList, relay_constraints::{ GeographicLocationConstraint, LocationConstraint, RelayConstraints, RelaySettings, }, @@ -1199,160 +1200,45 @@ fn parse_am_i_mullvad(result: String) -> anyhow::Result { }) } -pub mod custom_lists { - use super::*; - - use mullvad_types::custom_list::{CustomList, Id}; - use std::sync::{LazyLock, Mutex}; - - // Expose all custom list variants as a shorthand. - pub use List::*; - - /// The default custom list to use as location for all tests. - pub const DEFAULT_LIST: List = List::Nordic; - - /// Mapping between [List] to daemon custom lists. Since custom list ids are assigned by the - /// daemon at the creation of the custom list settings object, we can't map a custom list - /// name to a specific list before runtime. - static IDS: LazyLock>> = LazyLock::new(|| Mutex::new(HashMap::new())); - - /// Pre-defined (well-typed) custom lists which may be useful in different test scenarios. - #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] - pub enum List { - /// A selection of Nordic servers - Nordic, - /// A selection of European servers - Europe, - /// This custom list contains relays which are close geographically to the computer running - /// the test scenarios, which hopefully means there will be little latency between the test - /// machine and these relays - LowLatency, - /// Antithesis of [List::LowLatency], these relays are located far away from the test - /// server. Use this custom list if you want to simulate scenarios where the probability - /// of experiencing high latencies is desirable. - HighLatency, - } - - impl List { - pub fn name(self) -> String { - use List::*; - match self { - Nordic => "Nordic".to_string(), - Europe => "Europe".to_string(), - LowLatency => "Low Latency".to_string(), - HighLatency => "High Latency".to_string(), - } - } - - /// Iterator over all custom lists. - pub fn all() -> impl Iterator { - use List::*; - [Nordic, Europe, LowLatency, HighLatency].into_iter() - } - - pub fn locations(self) -> impl Iterator { - use List::*; - let country = GeographicLocationConstraint::country; - let city = GeographicLocationConstraint::city; - match self { - Nordic => { - vec![country("no"), country("se"), country("fi"), country("dk")].into_iter() - } - Europe => vec![ - // North - country("se"), - // West - country("fr"), - // East - country("ro"), - // South - country("it"), - ] - .into_iter(), - LowLatency => { - // Assumption: Test server is located in Gothenburg, Sweden. - vec![city("se", "got")].into_iter() - } - HighLatency => { - // Assumption: Test server is located in Gothenburg, Sweden. - vec![country("au"), country("ca"), country("za")].into_iter() - } - } - } - - pub fn to_constraint(self) -> Option { - let ids = IDS.lock().unwrap(); - let id = ids.get(&self)?; - Some(LocationConstraint::CustomList { list_id: *id }) - } - } - - impl From for LocationConstraint { - fn from(custom_list: List) -> Self { - // TODO: Is this _too_ unsound ?? - custom_list.to_constraint().unwrap() - } - } - - /// Add a set of custom lists which can be used in different test scenarios. - /// - /// See [`List`] for available custom lists. - pub async fn add_default_lists(mullvad_client: &mut MullvadProxyClient) -> anyhow::Result<()> { - for custom_list in List::all() { - let id = mullvad_client - .create_custom_list(custom_list.name()) - .await?; - let mut daemon_dito = find_custom_list(mullvad_client, &custom_list.name()).await?; - assert_eq!(id, daemon_dito.id); - for locations in custom_list.locations() { - daemon_dito.locations.insert(locations); - } - mullvad_client.update_custom_list(daemon_dito).await?; - // Associate this custom list variant with a specific, runtime custom list id. - IDS.lock().unwrap().insert(custom_list, id); - } - Ok(()) - } +/// Set the location to the given [`LocationConstraint`]. This also includes +/// entry location for multihop. It does not, however, affect bridge location for OpenVPN. +/// This is for simplify, as bridges default to using the server closest to the exit anyway, and +/// OpenVPN is slated for removal. +pub async fn set_location_from_constraint( + mullvad_client: &mut MullvadProxyClient, + location: impl Into, +) -> anyhow::Result<()> { + let constraints = get_location_relay_constraints(location.into()); - /// Set the default location to the custom list specified by `DEFAULT_LIST`. This also includes - /// entry location for multihop. It does not, however, affect bridge location for OpenVPN. - /// This is for simplify, as bridges default to using the server closest to the exit anyway, and - /// OpenVPN is slated for removal. - pub async fn set_default_location( - mullvad_client: &mut MullvadProxyClient, - ) -> anyhow::Result<()> { - let constraints = get_custom_list_location_relay_constraints(DEFAULT_LIST); - - mullvad_client - .set_relay_settings(constraints.into()) - .await - .context("Failed to set relay settings") - } + mullvad_client + .set_relay_settings(constraints.into()) + .await + .context("Failed to set relay settings") +} - fn get_custom_list_location_relay_constraints(custom_list: List) -> RelayConstraints { - let wireguard_constraints = mullvad_types::relay_constraints::WireguardConstraints { - entry_location: Constraint::Only(custom_list.into()), - ..Default::default() - }; +fn get_location_relay_constraints(custom_list: LocationConstraint) -> RelayConstraints { + let wireguard_constraints = mullvad_types::relay_constraints::WireguardConstraints { + entry_location: Constraint::Only(custom_list.clone()), + ..Default::default() + }; - RelayConstraints { - location: Constraint::Only(custom_list.into()), - wireguard_constraints, - ..Default::default() - } + RelayConstraints { + location: Constraint::Only(custom_list), + wireguard_constraints, + ..Default::default() } +} - /// Dig out a custom list from the daemon settings based on the custom list's name. - /// There should be an rpc for this. - pub async fn find_custom_list( - rpc: &mut MullvadProxyClient, - name: &str, - ) -> anyhow::Result { - rpc.get_settings() - .await? - .custom_lists - .into_iter() - .find(|list| list.name == name) - .ok_or(anyhow!("List '{name}' not found")) - } +/// Dig out a custom list from the daemon settings based on the custom list's name. +/// There should be an rpc for this. +pub async fn find_custom_list( + rpc: &mut MullvadProxyClient, + name: &str, +) -> anyhow::Result { + rpc.get_settings() + .await? + .custom_lists + .into_iter() + .find(|list| list.name == name) + .ok_or(anyhow!("List '{name}' not found")) } diff --git a/test/test-manager/src/tests/mod.rs b/test/test-manager/src/tests/mod.rs index d353f71e1b03..704db9c0f011 100644 --- a/test/test-manager/src/tests/mod.rs +++ b/test/test-manager/src/tests/mod.rs @@ -16,7 +16,7 @@ mod tunnel_state; mod ui; use itertools::Itertools; -use mullvad_types::relay_constraints::GeographicLocationConstraint; +use mullvad_types::relay_constraints::{GeographicLocationConstraint, LocationConstraint}; pub use test_metadata::TestMetadata; use anyhow::Context; @@ -28,7 +28,7 @@ use crate::{ package::get_version_from_path, }; use config::TEST_CONFIG; -use helpers::{get_app_env, install_app}; +use helpers::{get_app_env, install_app, set_location_from_constraint}; pub use install::test_upgrade_app; use mullvad_management_interface::MullvadProxyClient; use test_rpc::{meta::Os, ServiceClient}; @@ -153,41 +153,30 @@ pub async fn prepare_daemon( .await .context("Failed to disconnect daemon after test")?; helpers::ensure_logged_in(&mut mullvad_client).await?; - helpers::custom_lists::add_default_lists(&mut mullvad_client).await?; - helpers::custom_lists::set_default_location(&mut mullvad_client).await?; Ok(mullvad_client) } -/// Create an "anonymous" custom list for this test. The custom list will +/// Create and selects an "anonymous" custom list for this test. The custom list will /// have the same name as the test and contain the locations as specified by -/// [TestMetadata::location]. -pub async fn prepare_custom_lists( +/// [`TestMetadata`] location field. +pub async fn set_test_location( mullvad_client: &mut MullvadProxyClient, test: &TestMetadata, ) -> anyhow::Result<()> { - use helpers::custom_lists::find_custom_list; + use helpers::find_custom_list; // Convert locations from the test config to actual location constraints - let locations = { - let mut locations: Vec = vec![]; - for input in test.location.clone().unwrap() { - match input.parse::() { - Ok(location) => locations.push(location), - // If some location argument can not be parsed as a GeographicLocationconstraint, - // assume it is a custom list. - Err(_parse_error) => { - let custom_list = find_custom_list(mullvad_client, &input) - .await - .with_context(|| format!("Failed to parse test location '{input}'"))?; - // Hack: Dig out all the geographic location constraints from this custom list - for custom_list_location in custom_list.locations { - locations.push(custom_list_location) - } - } - }; - } - locations - }; + let locations: Vec = test + .location + .as_ref() + .unwrap() + .iter() + .map(|input| { + input + .parse::() + .with_context(|| format!("Failed to parse {input}")) + }) + .try_collect()?; log::debug!( "Creating custom list {} with locations '{:?}'", @@ -196,19 +185,22 @@ pub async fn prepare_custom_lists( ); // Add the custom list to the current app instance - let id = mullvad_client + let list_id = mullvad_client .create_custom_list(test.name.to_string()) .await?; let mut custom_list = find_custom_list(mullvad_client, test.name).await?; - assert_eq!(id, custom_list.id); + assert_eq!(list_id, custom_list.id); for location in locations { custom_list.locations.insert(location); } mullvad_client.update_custom_list(custom_list).await?; log::debug!("Added custom list"); + set_location_from_constraint(mullvad_client, LocationConstraint::CustomList { list_id }) + .await + .context("Failed to set location to custom list")?; Ok(()) } diff --git a/test/test-manager/src/tests/tunnel.rs b/test/test-manager/src/tests/tunnel.rs index bc15ccb8a3bb..b64a16d85484 100644 --- a/test/test-manager/src/tests/tunnel.rs +++ b/test/test-manager/src/tests/tunnel.rs @@ -187,15 +187,7 @@ pub async fn test_wireguard_over_shadowsocks( rpc: ServiceClient, mut mullvad_client: MullvadProxyClient, ) -> anyhow::Result<()> { - // NOTE: We have experienced flakiness due to timeout issues if distant relays are selected. - // This is an attempt to try to reduce this type of flakiness. - use helpers::custom_lists::LowLatency; - - let query = RelayQueryBuilder::new() - .wireguard() - .shadowsocks() - .location(LowLatency) - .build(); + let query = RelayQueryBuilder::new().wireguard().shadowsocks().build(); apply_settings_from_relay_query(&mut mullvad_client, query).await?; @@ -294,16 +286,7 @@ pub async fn test_multihop( rpc: ServiceClient, mut mullvad_client: MullvadProxyClient, ) -> Result<(), Error> { - // NOTE: We have experienced flakiness due to timeout issues if distant relays are selected. - // This is an attempt to try to reduce this type of flakiness. - use helpers::custom_lists::LowLatency; - - let query = RelayQueryBuilder::new() - .wireguard() - .multihop() - .location(LowLatency) - .entry(LowLatency) - .build(); + let query = RelayQueryBuilder::new().wireguard().multihop().build(); apply_settings_from_relay_query(&mut mullvad_client, query).await?; @@ -455,10 +438,6 @@ pub async fn test_quantum_resistant_tunnel( rpc: ServiceClient, mut mullvad_client: MullvadProxyClient, ) -> anyhow::Result<()> { - // NOTE: We have experienced flakiness due to timeout issues if distant relays are selected. - // This is an attempt to try to reduce this type of flakiness. - use helpers::custom_lists::LowLatency; - mullvad_client .set_quantum_resistant_tunnel(wireguard::QuantumResistantState::Off) .await @@ -472,10 +451,7 @@ pub async fn test_quantum_resistant_tunnel( log::info!("Setting tunnel protocol to WireGuard"); - let query = RelayQueryBuilder::new() - .wireguard() - .location(LowLatency) - .build(); + let query = RelayQueryBuilder::new().wireguard().build(); apply_settings_from_relay_query(&mut mullvad_client, query).await?; @@ -536,10 +512,6 @@ pub async fn test_quantum_resistant_multihop_udp2tcp_tunnel( rpc: ServiceClient, mut mullvad_client: MullvadProxyClient, ) -> Result<(), Error> { - // NOTE: We have experienced flakiness due to timeout issues if distant relays are selected. - // This is an attempt to try to reduce this type of flakiness. - use helpers::custom_lists::LowLatency; - mullvad_client .set_quantum_resistant_tunnel(wireguard::QuantumResistantState::On) .await @@ -549,8 +521,6 @@ pub async fn test_quantum_resistant_multihop_udp2tcp_tunnel( .wireguard() .multihop() .udp2tcp() - .entry(LowLatency) - .location(LowLatency) .build(); apply_settings_from_relay_query(&mut mullvad_client, query).await?; @@ -577,10 +547,6 @@ pub async fn test_quantum_resistant_multihop_shadowsocks_tunnel( rpc: ServiceClient, mut mullvad_client: MullvadProxyClient, ) -> anyhow::Result<()> { - // NOTE: We have experienced flakiness due to timeout issues if distant relays are selected. - // This is an attempt to try to reduce this type of flakiness. - use helpers::custom_lists::LowLatency; - mullvad_client .set_quantum_resistant_tunnel(wireguard::QuantumResistantState::On) .await @@ -590,8 +556,6 @@ pub async fn test_quantum_resistant_multihop_shadowsocks_tunnel( .wireguard() .multihop() .shadowsocks() - .entry(LowLatency) - .location(LowLatency) .build(); apply_settings_from_relay_query(&mut mullvad_client, query).await?; diff --git a/test/test-manager/src/tests/ui.rs b/test/test-manager/src/tests/ui.rs index 6b9ce5b3a9b6..9491308eb878 100644 --- a/test/test-manager/src/tests/ui.rs +++ b/test/test-manager/src/tests/ui.rs @@ -91,19 +91,11 @@ pub async fn test_ui_tunnel_settings( rpc: ServiceClient, mut mullvad_client: MullvadProxyClient, ) -> anyhow::Result<()> { - // NOTE: This test connects multiple times using various settings, some of which may cause a - // significant increase in connection time, e.g. multihop and OpenVPN. For this reason, it is - // preferable to only target low latency servers. - use helpers::custom_lists::LowLatency; - // tunnel-state.spec precondition: a single WireGuard relay should be selected log::info!("Select WireGuard relay"); let entry = helpers::constrain_to_relay( &mut mullvad_client, - RelayQueryBuilder::new() - .wireguard() - .location(LowLatency) - .build(), + RelayQueryBuilder::new().wireguard().build(), ) .await?; From 6109038b9767827fc3a4b36c7d821b669231a4fb Mon Sep 17 00:00:00 2001 From: Sebastian Holmin Date: Tue, 14 Jan 2025 12:11:51 +0100 Subject: [PATCH 11/15] With no matches, use default location --- test/test-manager/src/tests/mod.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/test-manager/src/tests/mod.rs b/test/test-manager/src/tests/mod.rs index 704db9c0f011..1352a4fe3122 100644 --- a/test/test-manager/src/tests/mod.rs +++ b/test/test-manager/src/tests/mod.rs @@ -28,7 +28,7 @@ use crate::{ package::get_version_from_path, }; use config::TEST_CONFIG; -use helpers::{get_app_env, install_app, set_location_from_constraint}; +use helpers::{find_custom_list, get_app_env, install_app, set_location_from_constraint}; pub use install::test_upgrade_app; use mullvad_management_interface::MullvadProxyClient; use test_rpc::{meta::Os, ServiceClient}; @@ -164,12 +164,12 @@ pub async fn set_test_location( mullvad_client: &mut MullvadProxyClient, test: &TestMetadata, ) -> anyhow::Result<()> { - use helpers::find_custom_list; + // If no location is specified for the test, don't do anything and use the default value of the app + let Some(locations) = test.location.as_ref() else { + return Ok(()); + }; // Convert locations from the test config to actual location constraints - let locations: Vec = test - .location - .as_ref() - .unwrap() + let locations: Vec = locations .iter() .map(|input| { input From c0aea0ef182e9e20119af08427c9d9dfd50c8308 Mon Sep 17 00:00:00 2001 From: Sebastian Holmin Date: Tue, 14 Jan 2025 14:35:18 +0100 Subject: [PATCH 12/15] Improve test and docs --- test/test-manager/src/config/manifest.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/test/test-manager/src/config/manifest.rs b/test/test-manager/src/config/manifest.rs index f6ee47f85494..51386f68ed0f 100644 --- a/test/test-manager/src/config/manifest.rs +++ b/test/test-manager/src/config/manifest.rs @@ -81,9 +81,6 @@ mod locations { pub struct TestLocationList(pub Vec); impl TestLocationList { - // TODO: Consider if we should handle the case of an empty list by returning vec!["any"] - /// Look up a test (by name) and see if there are any locations - /// that we should use. pub fn lookup(&self, test: &str) -> Option<&Vec> { self.0 .iter() @@ -173,12 +170,17 @@ mod tests { "vms": {}, "mullvad_host": "mullvad.net", "test_locations": [ - { "daita": [ "se-got-wg-001", "se-got-wg-002" ] }, + { "*daita": [ "se-got-wg-001", "se-got-wg-002" ] }, { "*": [ "se" ] } ] }"#; let config: Config = serde_json::from_str(config).unwrap(); + assert!(config + .test_locations + .lookup("test_daita") + .unwrap() + .contains(&"se-got-wg-002".to_string())); assert!(!config.test_locations.0.is_empty()); } } From e650f3daa3b34beddb5f41ab60504e0bd564e5c5 Mon Sep 17 00:00:00 2001 From: Sebastian Holmin Date: Thu, 16 Jan 2025 15:40:40 +0100 Subject: [PATCH 13/15] Reduce log verbosity of API check --- mullvad-api/src/availability.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mullvad-api/src/availability.rs b/mullvad-api/src/availability.rs index 0857a7812b59..9d5e135c1553 100644 --- a/mullvad-api/src/availability.rs +++ b/mullvad-api/src/availability.rs @@ -72,7 +72,7 @@ impl ApiAvailability { /// starting it if it's not currently running. pub fn reset_inactivity_timer(&self) { let mut inner = self.acquire(); - log::debug!("Restarting API inactivity check"); + log::trace!("Restarting API inactivity check"); inner.stop_inactivity_timer(); let availability_handle = self.clone(); inner.inactivity_timer = Some(tokio::spawn(async move { @@ -252,7 +252,7 @@ impl ApiAvailabilityState { } fn stop_inactivity_timer(&mut self) { - log::debug!("Stopping API inactivity check"); + log::trace!("Stopping API inactivity check"); if let Some(timer) = self.inactivity_timer.take() { timer.abort(); } From 6c631d02738a5c84cb050b838eaa3a1f469bc521 Mon Sep 17 00:00:00 2001 From: Sebastian Holmin Date: Thu, 16 Jan 2025 15:41:09 +0100 Subject: [PATCH 14/15] Format `test-utils.sh` --- test/scripts/test-utils.sh | 136 ++++++++++++++++++------------------- 1 file changed, 68 insertions(+), 68 deletions(-) diff --git a/test/scripts/test-utils.sh b/test/scripts/test-utils.sh index 319c64602050..be826a6ef053 100755 --- a/test/scripts/test-utils.sh +++ b/test/scripts/test-utils.sh @@ -12,9 +12,9 @@ function get_test_utls_dir { local script_path="${BASH_SOURCE[0]}" local script_dir if [[ -n "$script_path" ]]; then - script_dir="$(cd "$(dirname "$script_path")" > /dev/null && pwd)" + script_dir="$(cd "$(dirname "$script_path")" >/dev/null && pwd)" else - script_dir="$(cd "$(dirname "$0")" > /dev/null && pwd)" + script_dir="$(cd "$(dirname "$0")" >/dev/null && pwd)" fi echo "$script_dir" } @@ -54,7 +54,7 @@ export CURRENT_VERSION export LATEST_STABLE_RELEASE function print_available_releases { - for release in $(jq -r '.[].tag_name'<<<"$RELEASES"); do + for release in $(jq -r '.[].tag_name' <<<"$RELEASES"); do echo "$release" done } @@ -73,7 +73,7 @@ function get_package_dir { exit 1 fi - mkdir -p "$package_dir" || exit 1 + mkdir -p "$package_dir" || exit 1 # Clean up old packages find "$package_dir" -type f -mtime +5 -delete || true @@ -89,7 +89,7 @@ function nice_time { result=$? fi s=$SECONDS - echo "\"$*\" completed in $((s/60))m:$((s%60))s" + echo "\"$*\" completed in $((s / 60))m:$((s % 60))s" return $result } # Matches $1 with a build version string and sets the following exported variables: @@ -122,22 +122,22 @@ function get_app_filename { version="${BUILD_VERSION}${COMMIT_HASH}${TAG:-}" fi case $os in - debian*|ubuntu*) - echo "MullvadVPN-${version}_amd64.deb" - ;; - fedora*) - echo "MullvadVPN-${version}_x86_64.rpm" - ;; - windows*) - echo "MullvadVPN-${version}.exe" - ;; - macos*) - echo "MullvadVPN-${version}.pkg" - ;; - *) - echo "Unsupported target: $os" 1>&2 - return 1 - ;; + debian* | ubuntu*) + echo "MullvadVPN-${version}_amd64.deb" + ;; + fedora*) + echo "MullvadVPN-${version}_x86_64.rpm" + ;; + windows*) + echo "MullvadVPN-${version}.exe" + ;; + macos*) + echo "MullvadVPN-${version}.pkg" + ;; + *) + echo "Unsupported target: $os" 1>&2 + return 1 + ;; esac } @@ -177,19 +177,19 @@ function get_e2e_filename { version="${BUILD_VERSION}${COMMIT_HASH}" fi case $os in - debian*|ubuntu*|fedora*) - echo "app-e2e-tests-${version}-x86_64-unknown-linux-gnu" - ;; - windows*) - echo "app-e2e-tests-${version}-x86_64-pc-windows-msvc.exe" - ;; - macos*) - echo "app-e2e-tests-${version}-aarch64-apple-darwin" - ;; - *) - echo "Unsupported target: $os" 1>&2 - return 1 - ;; + debian* | ubuntu* | fedora*) + echo "app-e2e-tests-${version}-x86_64-unknown-linux-gnu" + ;; + windows*) + echo "app-e2e-tests-${version}-x86_64-pc-windows-msvc.exe" + ;; + macos*) + echo "app-e2e-tests-${version}-aarch64-apple-darwin" + ;; + *) + echo "Unsupported target: $os" 1>&2 + return 1 + ;; esac } @@ -282,38 +282,38 @@ function run_tests_for_os { test_dir=$(get_test_utls_dir)/.. read -ra test_filters_arg <<<"${TEST_FILTERS:-}" # Split the string by words into an array pushd "$test_dir" - if [ -n "${TEST_DIST_DIR+x}" ]; then - if [ ! -x "${TEST_DIST_DIR%/}/test-manager" ]; then - executable_not_found_in_dist_error test-manager - fi - test_manager="${TEST_DIST_DIR%/}/test-manager" - runner_dir_flag=("--runner-dir" "$TEST_DIST_DIR") - else - test_manager="cargo run --bin test-manager" - runner_dir_flag=() + if [ -n "${TEST_DIST_DIR+x}" ]; then + if [ ! -x "${TEST_DIST_DIR%/}/test-manager" ]; then + executable_not_found_in_dist_error test-manager fi + test_manager="${TEST_DIST_DIR%/}/test-manager" + runner_dir_flag=("--runner-dir" "$TEST_DIST_DIR") + else + test_manager="cargo run --bin test-manager" + runner_dir_flag=() + fi - if [ -n "${MULLVAD_HOST+x}" ]; then - mullvad_host_arg=("--mullvad-host" "$MULLVAD_HOST") - else - mullvad_host_arg=() - fi + if [ -n "${MULLVAD_HOST+x}" ]; then + mullvad_host_arg=("--mullvad-host" "$MULLVAD_HOST") + else + mullvad_host_arg=() + fi - if ! RUST_LOG_STYLE=always $test_manager run-tests \ - --account "${ACCOUNT_TOKEN:?Error: ACCOUNT_TOKEN not set}" \ - --app-package "${APP_PACKAGE:?Error: APP_PACKAGE not set}" \ - "${upgrade_package_arg[@]}" \ - "${test_report_arg[@]}" \ - --package-dir "${package_dir}" \ - --vm "$vm" \ - --openvpn-certificate "${OPENVPN_CERTIFICATE:-"assets/openvpn.ca.crt"}" \ - "${mullvad_host_arg[@]}" \ - "${test_filters_arg[@]}" \ - "${runner_dir_flag[@]}" \ - 2>&1 | sed -r "s/${ACCOUNT_TOKEN}/\{ACCOUNT_TOKEN\}/g"; then - echo "Test run failed" - exit 1 - fi + if ! RUST_LOG_STYLE=always $test_manager run-tests \ + --account "${ACCOUNT_TOKEN:?Error: ACCOUNT_TOKEN not set}" \ + --app-package "${APP_PACKAGE:?Error: APP_PACKAGE not set}" \ + "${upgrade_package_arg[@]}" \ + "${test_report_arg[@]}" \ + --package-dir "${package_dir}" \ + --vm "$vm" \ + --openvpn-certificate "${OPENVPN_CERTIFICATE:-"assets/openvpn.ca.crt"}" \ + "${mullvad_host_arg[@]}" \ + "${test_filters_arg[@]}" \ + "${runner_dir_flag[@]}" \ + 2>&1 | sed -r "s/${ACCOUNT_TOKEN}/\{ACCOUNT_TOKEN\}/g"; then + echo "Test run failed" + exit 1 + fi popd } @@ -335,10 +335,10 @@ function build_current_version { if [ ! -f "$app_package" ]; then pushd "$app_dir" - if [[ $(git diff --quiet) ]]; then - echo "WARNING: the app repository contains uncommitted changes, this script will only rebuild the app package when the git hash changes" - fi - ./build.sh + if [[ $(git diff --quiet) ]]; then + echo "WARNING: the app repository contains uncommitted changes, this script will only rebuild the app package when the git hash changes" + fi + ./build.sh popd echo "Moving '$(realpath "$app_dir/dist/$app_filename")' to '$(realpath "$app_package")'" mv -n "$app_dir"/dist/"$app_filename" "$app_package" @@ -348,7 +348,7 @@ function build_current_version { if [ ! -f "$gui_test_bin" ]; then pushd "$app_dir"/gui - npm run build-test-executable + npm run build-test-executable popd echo "Moving '$(realpath "$app_dir/dist/$gui_test_filename")' to '$(realpath "$gui_test_bin")'" mv -n "$app_dir"/dist/"$gui_test_filename" "$gui_test_bin" From 6042a703cd72c65008305678f6b445316c2752f6 Mon Sep 17 00:00:00 2001 From: Sebastian Holmin Date: Thu, 16 Jan 2025 15:41:45 +0100 Subject: [PATCH 15/15] WIP: `config` subcommand --- test/test-manager/src/config/io.rs | 2 +- test/test-manager/src/config/manifest.rs | 8 ++++++- test/test-manager/src/main.rs | 30 +++++++++++++++++++++++- 3 files changed, 37 insertions(+), 3 deletions(-) diff --git a/test/test-manager/src/config/io.rs b/test/test-manager/src/config/io.rs index 2dcc05ca0bdf..1aaefde71b23 100644 --- a/test/test-manager/src/config/io.rs +++ b/test/test-manager/src/config/io.rs @@ -51,7 +51,7 @@ impl ConfigFile { } /// Get configuration file path - fn get_config_path() -> Result { + pub fn get_config_path() -> Result { Ok(Self::get_config_dir()?.join("config.json")) } diff --git a/test/test-manager/src/config/manifest.rs b/test/test-manager/src/config/manifest.rs index 51386f68ed0f..0c8062dce8a1 100644 --- a/test/test-manager/src/config/manifest.rs +++ b/test/test-manager/src/config/manifest.rs @@ -74,9 +74,15 @@ mod locations { }; use std::fmt; - #[derive(Debug, Clone, Default)] + #[derive(Clone, Default)] pub struct TestLocation(glob::Pattern, Vec); + impl fmt::Debug for TestLocation { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}: {:?}", self.0, &self.1) + } + } + #[derive(Debug, DeserDerive, SerDerive, Clone, Default)] pub struct TestLocationList(pub Vec); diff --git a/test/test-manager/src/main.rs b/test/test-manager/src/main.rs index 42112985688c..f1d4acb41a7f 100644 --- a/test/test-manager/src/main.rs +++ b/test/test-manager/src/main.rs @@ -13,8 +13,9 @@ mod vm; use std::net::IpAddr; use std::{net::SocketAddr, path::PathBuf}; -use anyhow::{Context, Result}; +use anyhow::{Context, Ok, Result}; use clap::{builder::PossibleValuesParser, Parser}; +use config::ConfigFile; use package::TargetInfo; use tests::{config::TEST_CONFIG, get_filtered_tests}; use vm::provision; @@ -29,8 +30,19 @@ struct Args { cmd: Commands, } +#[derive(clap::Subcommand, Debug)] +enum ConfigArg { + Get, + Set, + GetPath, +} + #[derive(clap::Subcommand, Debug)] enum Commands { + /// Manage configuration for tests and VMs + #[clap(subcommand)] + Config(ConfigArg), + /// Create or edit a VM config Set { /// Name of the VM config @@ -184,6 +196,22 @@ async fn main() -> Result<()> { .await .context("Failed to load config")?; match args.cmd { + Commands::Config(config_subcommand) => match config_subcommand { + ConfigArg::Get => { + println!("{:#?}", *config); + Ok(()) + } + ConfigArg::Set => todo!(), + ConfigArg::GetPath => { + println!( + "{}", + ConfigFile::get_config_path() + .expect("Get config path") + .display() + ); + Ok(()) + } + }, Commands::Set { vm, config: vm_config,