Remove must_succede test macro parameter

This simplifes handling of test results.

Author: Serock3

test/test-manager/src/run_tests.rs

@@ -1,7 +1,7 @@
 use crate::{
     logging::{Logger, Panic, TestOutput, TestResult},
     mullvad_daemon::{self, MullvadClientArgument},
-    summary::{self, maybe_log_test_result},
+    summary::SummaryLogger,
     tests::{self, config::TEST_CONFIG, get_tests, TestContext},
     vm,
 };
@@ -23,8 +23,8 @@ pub async fn run(
     test_filters: &[String],
     skip_wait: bool,
     print_failed_tests_only: bool,
-    mut summary_logger: Option<summary::SummaryLogger>,
-) -> Result<()> {
+    mut summary_logger: Option<SummaryLogger>,
+) -> Result<TestResult> {
     log::trace!("Setting test constants");
     TEST_CONFIG.init(config);
 
@@ -127,38 +127,14 @@ pub async fn run(
 
         test_output.print();
 
-        maybe_log_test_result(
-            summary_logger.as_mut(),
+        register_test_result(
+            test_output.result,
+            &mut failed_tests,
             test.name,
-            if test_succeeded {
-                summary::TestResult::Pass
-            } else {
-                summary::TestResult::Fail
-            },
+            &mut successful_tests,
+            summary_logger.as_mut(),
         )
-        .await
-        .context("Failed to log test result")?;
-
-        match test_result.result {
-            Err(panic) => {
-                failed_tests.push(test.name);
-                final_result = Err(panic).context("test panicked");
-                if test.must_succeed {
-                    break;
-                }
-            }
-            Ok(Err(failure)) => {
-                failed_tests.push(test.name);
-                final_result = Err(failure).context("test failed");
-                if test.must_succeed {
-                    break;
-                }
-            }
-            Ok(Ok(result)) => {
-                successful_tests.push(test.name);
-                final_result = final_result.and(Ok(result));
-            }
-        }
+        .await?;
     }
 
     log::info!("TESTS THAT SUCCEEDED:");
@@ -167,7 +143,7 @@ pub async fn run(
     }
 
     log::info!("TESTS THAT FAILED:");
-    for test in failed_tests {
+    for test in &failed_tests {
         log::info!("{test}");
     }
 

test/test-manager/src/summary.rs

@@ -107,19 +107,6 @@ impl SummaryLogger {
     }
 }
 
-/// Convenience function that logs when there's a value, and is a no-op otherwise.
-// y u no trait async fn
-pub async fn maybe_log_test_result(
-    summary_logger: Option<&mut SummaryLogger>,
-    test_name: &str,
-    test_result: TestResult,
-) -> Result<(), Error> {
-    match summary_logger {
-        Some(logger) => logger.log_test_result(test_name, test_result).await,
-        None => Ok(()),
-    }
-}
-
 /// Parsed summary results
 pub struct Summary {
     /// Name of the configuration
@@ -263,11 +250,7 @@ pub async fn print_summary_table<P: AsRef<Path>>(summary_files: &[P]) {
     for test in &tests {
         println!("<tr>");
 
-        println!(
-            "<td>{}{}</td>",
-            test.name,
-            if test.must_succeed { " *" } else { "" }
-        );
+        println!("<td>{}</td>", test.name,);
 
         let mut failed_platforms = vec![];
         for summary in &summaries {

test/test-manager/src/tests/test_metadata.rs

@@ -9,7 +9,6 @@ pub struct TestMetadata {
     pub func: TestWrapperFunction,
     pub priority: Option<i32>,
     pub always_run: bool,
-    pub must_succeed: bool,
 }
 
 impl TestMetadata {

test/test-manager/test_macro/src/lib.rs

@@ -23,9 +23,6 @@ use test_rpc::meta::Os;
 /// * `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.
 ///
-/// * `must_succeed` - If the testing suite stops running if this test fails. `must_succeed`
-///   defaults to false.
-///
 /// * `always_run` - If the test should always run regardless of what test filters are provided by
 ///   the user. `always_run` defaults to false.
 ///
@@ -51,10 +48,10 @@ use test_rpc::meta::Os;
 ///
 /// ## Create a test with custom parameters
 ///
-/// This test will run early in the test loop and must succeed and will always run.
+/// This test will run early in the test loop and will always run.
 ///
 /// ```ignore
-/// #[test_function(priority = -1337, must_succeed = true, always_run = true)]
+/// #[test_function(priority = -1337, always_run = true)]
 /// pub async fn test_function(
 ///     rpc: ServiceClient,
 ///     mut mullvad_client: mullvad_management_interface::MullvadProxyClient,
@@ -109,7 +106,6 @@ fn parse_marked_test_function(
 fn get_test_macro_parameters(attributes: &syn::AttributeArgs) -> Result<MacroParameters> {
     let mut priority = None;
     let mut always_run = false;
-    let mut must_succeed = false;
     let mut targets = vec![];
 
     for attribute in attributes {
@@ -129,11 +125,6 @@ fn get_test_macro_parameters(attributes: &syn::AttributeArgs) -> Result<MacroPar
                 Lit::Bool(lit_bool) => always_run = lit_bool.value(),
                 _ => bail!(nv, "'always_run' should have a bool value"),
             }
-        } else if nv.path.is_ident("must_succeed") {
-            match lit {
-                Lit::Bool(lit_bool) => must_succeed = lit_bool.value(),
-                _ => bail!(nv, "'must_succeed' should have a bool 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");
@@ -157,7 +148,6 @@ fn get_test_macro_parameters(attributes: &syn::AttributeArgs) -> Result<MacroPar
     Ok(MacroParameters {
         priority,
         always_run,
-        must_succeed,
         targets,
     })
 }
@@ -176,7 +166,6 @@ fn create_test(test_function: TestFunction) -> proc_macro2::TokenStream {
         .collect();
 
     let always_run = test_function.macro_parameters.always_run;
-    let must_succeed = test_function.macro_parameters.must_succeed;
 
     let func_name = test_function.name;
     let function_mullvad_version = test_function.function_parameters.mullvad_client.version();
@@ -219,7 +208,6 @@ fn create_test(test_function: TestFunction) -> proc_macro2::TokenStream {
             func: #wrapper_closure,
             priority: #test_function_priority,
             always_run: #always_run,
-            must_succeed: #must_succeed,
         });
     }
 }
@@ -233,7 +221,6 @@ struct TestFunction {
 struct MacroParameters {
     priority: Option<i32>,
     always_run: bool,
-    must_succeed: bool,
     targets: Vec<Os>,
 }