Merge branch 'test-function-skip'

3 files changed, 38 insertions, 25 deletions

diff --git a/test/test-manager/src/tests/mod.rs b/test/test-manager/src/tests/mod.rs
index f39e4371d1..975c545fba 100644
--- a/test/test-manager/src/tests/mod.rs
+++ b/test/test-manager/src/tests/mod.rs
@@ -23,7 +23,7 @@ pub use test_metadata::TestMetadata;
 
 use anyhow::Context;
 use futures::future::BoxFuture;
-use std::{ops::Not, time::Duration};
+use std::time::Duration;
 
 use crate::{
     logging::print_mullvad_logs, mullvad_daemon::RpcClientProvider, package::get_version_from_path,
@@ -122,7 +122,8 @@ pub fn get_filtered_tests(
     tests.sort_by_key(|test| test.priority.unwrap_or(0));
 
     let mut tests = if specified_tests.is_empty() {
-        // Keep all tests
+        // Include all but tests labelled with 'skip'
+        tests.retain(|test| !test.skip);
         tests
     } else {
         specified_tests
@@ -137,14 +138,13 @@ pub fn get_filtered_tests(
             .collect::<Result<_, anyhow::Error>>()?
     };
 
-    tests.retain(|test| {
+    let on_skip_list = |test: &TestMetadata| {
         skipped_tests
             .iter()
             .any(|skip| skip.eq_ignore_ascii_case(test.name))
-            .not()
-    });
+    };
 
-    tests.retain(|test| should_run_on_os(test.targets, TEST_CONFIG.os));
+    tests.retain(|test| should_run_on_os(test.targets, TEST_CONFIG.os) && !on_skip_list(test));
 
     Ok(tests)
 }
diff --git a/test/test-manager/src/tests/test_metadata.rs b/test/test-manager/src/tests/test_metadata.rs
index 31a55498f9..c058ade633 100644
--- a/test/test-manager/src/tests/test_metadata.rs
+++ b/test/test-manager/src/tests/test_metadata.rs
@@ -10,6 +10,8 @@ pub struct TestMetadata {
     pub priority: Option<i32>,
     /// A list of location that will be used for by the test
     pub location: Option<Vec<String>>,
+    /// If the current test should be skipped.
+    pub skip: bool,
 }
 
 // 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 ee560384e2..88b1f4f1df 100644
--- a/test/test-manager/test_macro/src/lib.rs
+++ b/test/test-manager/test_macro/src/lib.rs
@@ -18,7 +18,7 @@ use test_rpc::meta::Os;
 ///
 /// # Arguments
 ///
-/// The `test_function` macro takes 4 optional arguments
+/// The `test_function` macro takes 3 optional arguments
 ///
 /// * `priority` - The order in which tests will be run where low numbers run before high numbers
 ///   and tests with the same number run in undefined order. `priority` defaults to 0.
@@ -26,6 +26,8 @@ use test_rpc::meta::Os;
 /// * `target_os` - The test should only run on the specified OS. This can currently be set to
 ///   `linux`, `windows`, or `macos`.
 ///
+/// * `skip` - The test should not be run.
+///
 /// # Examples
 ///
 /// ## Create a standard test.
@@ -101,6 +103,7 @@ fn parse_marked_test_function(
 fn get_test_macro_parameters(attributes: &syn::AttributeArgs) -> Result<MacroParameters> {
     let mut priority = None;
     let mut targets = vec![];
+    let mut skip = false;
 
     for attribute in attributes {
         // we only use name-value attributes
@@ -109,32 +112,37 @@ fn get_test_macro_parameters(attributes: &syn::AttributeArgs) -> Result<MacroPar
         };
         let lit = &nv.lit;
 
-        if nv.path.is_ident("priority") {
-            match lit {
+        match &nv.path {
+            path if path.is_ident("priority") => match lit {
                 Lit::Int(lit_int) => priority = Some(lit_int.base10_parse().unwrap()),
                 _ => bail!(nv, "'priority' should have an integer value"),
-            }
-        } else if nv.path.is_ident("target_os") {
-            let Lit::Str(lit_str) = lit else {
-                bail!(nv, "'target_os' should have a string value");
-            };
+            },
+            path if path.is_ident("target_os") => {
+                let Lit::Str(lit_str) = lit else {
+                    bail!(nv, "'target_os' should have a string value");
+                };
 
-            let target = match lit_str.value().parse() {
-                Ok(os) => os,
-                Err(e) => bail!(lit_str, "{e}"),
-            };
+                let target = match lit_str.value().parse() {
+                    Ok(os) => os,
+                    Err(e) => bail!(lit_str, "{e}"),
+                };
 
-            if targets.contains(&target) {
-                bail!(nv, "Duplicate target");
-            }
+                if targets.contains(&target) {
+                    bail!(nv, "Duplicate target");
+                }
 
-            targets.push(target);
-        } else {
-            bail!(nv, "unknown attribute");
+                targets.push(target);
+            }
+            path if path.is_ident("skip") => skip = true,
+            _ => bail!(nv, "unknown attribute"),
         }
     }
 
-    Ok(MacroParameters { priority, targets })
+    Ok(MacroParameters {
+        priority,
+        targets,
+        skip,
+    })
 }
 
 fn create_test(test_function: TestFunction) -> proc_macro2::TokenStream {
@@ -149,6 +157,7 @@ fn create_test(test_function: TestFunction) -> proc_macro2::TokenStream {
             Os::Windows => quote! { ::test_rpc::meta::Os::Windows, },
         })
         .collect();
+    let skip = test_function.macro_parameters.skip;
 
     let func_name = test_function.name;
     let wrapper_closure = quote! {
@@ -170,6 +179,7 @@ fn create_test(test_function: TestFunction) -> proc_macro2::TokenStream {
             func: #wrapper_closure,
             priority: #test_function_priority,
             location: None,
+            skip: #skip,
         });
     }
 }
@@ -182,4 +192,5 @@ struct TestFunction {
 struct MacroParameters {
     priority: Option<i32>,
     targets: Vec<Os>,
+    skip: bool,
 }