Cleaned up test creation logic

Author: LarryOsterman

sdk/core/azure_core/CHANGELOG.md

@@ -16,7 +16,6 @@
 
 ### Breaking Changes
 
-
 - Changed `ClientOptions::retry` from `Option<RetryOptions>` to `RetryOptions`.
 - Changed `DeserializeWith::deserialize_with()` to be sync.
 - Changed `Pipeline::send()` to return a `Result<RawResponse>`.

sdk/core/azure_core_test/src/perf/framework_tests.rs

@@ -51,27 +51,25 @@ fn create_fibonacci1_test(runner: &PerfRunner) -> CreatePerfTestReturn {
             Ok(())
         }
     }
-    // Manually handle the Result instead of using ? because this function does not return a Result.
-    let count: Option<&String> = match runner.try_get_test_arg("count") {
-        Ok(v) => v,
-        Err(e) => {
-            // Return a future that immediately yields the error.
-            return Box::pin(async move { Err(e) });
-        }
-    };
-    println!("Fibonacci1Test with count: {:?}", count);
-    let count = count.expect("count argument is mandatory");
-    let count = match count.parse::<u32>() {
-        Ok(v) => v,
-        Err(e) => {
-            let err = azure_core::Error::with_message(
+
+    // Helper function to handle the async creation of the test.
+    async fn create_test(runner: PerfRunner) -> Result<Box<dyn PerfTest>> {
+        let count: Option<&String> = runner.try_get_test_arg("count")?;
+
+        println!("Fibonacci1Test with count: {:?}", count);
+        let count = count.expect("count argument is mandatory");
+        let count = count.parse::<u32>().map_err(|e| {
+            azure_core::Error::with_error(
                 azure_core::error::ErrorKind::Other,
-                format!("invalid count argument: {}", e),
-            );
-            return Box::pin(async move { Err(err) });
-        }
-    };
-    Box::pin(async move { Ok(Box::new(Fibonacci1Test { count }) as Box<dyn PerfTest>) })
+                e,
+                "Invalid count argument",
+            )
+        })?;
+        Ok(Box::new(Fibonacci1Test { count }) as Box<dyn PerfTest>)
+    }
+
+    // Return a pinned future that creates the test.
+    Box::pin(create_test(runner.clone()))
 }
 
 #[tokio::test]

sdk/core/azure_core_test/src/perf/mod.rs

@@ -2,6 +2,7 @@
 // Licensed under the MIT License.
 
 #![doc = include_str!("README.md")]
+#![cfg(not(target_arch = "wasm32"))]
 
 use crate::TestContext;
 use azure_core::{time::Duration, Error, Result};
@@ -17,8 +18,7 @@ use std::{
 };
 use tokio::{select, task::JoinSet};
 
-#[cfg_attr(target_arch = "wasm32", async_trait::async_trait(?Send))]
-#[cfg_attr(not(target_arch = "wasm32"), async_trait::async_trait)]
+#[async_trait::async_trait]
 pub trait PerfTest: Send + Sync {
     async fn setup(&self, context: &TestContext) -> azure_core::Result<()>;
     async fn run(&self /*, context: &TestContext*/) -> azure_core::Result<()>;
@@ -67,15 +67,15 @@ pub struct TestOption {
     pub sensitive: bool,
 }
 
-#[derive(Debug)]
-#[allow(dead_code)]
+#[derive(Debug, Clone)]
 struct PerfRunnerOptions {
     no_cleanup: bool,
     iterations: u32,
     parallel: usize,
     duration: Duration,
     warmup: Duration,
     disable_progress: bool,
+    #[allow(dead_code)]
     test_results_filename: String,
 }
 
@@ -109,10 +109,9 @@ impl From<&ArgMatches> for PerfRunnerOptions {
 }
 
 /// Context information required by performance tests.
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 pub struct PerfRunner {
     options: PerfRunnerOptions,
-    #[allow(dead_code)]
     tests: Vec<TestMetadata>,
     arguments: ArgMatches,
     package_dir: &'static str,
@@ -264,7 +263,7 @@ impl PerfRunner {
             let operations_per_second =
                 self.options.duration.as_seconds_f64() / iteration_count as f64;
             let duration_per_operation = Duration::seconds_f64(operations_per_second);
-            println!("{} seconds/operation", duration_per_operation);
+            println!("{:4} seconds/operation", duration_per_operation);
         }
         Ok(())
     }
@@ -298,7 +297,7 @@ impl PerfRunner {
                 _ = async {
                         loop {
                             tokio::time::sleep(tokio::time::Duration::from_secs(1)).await;
-                            println!("{:?} elapsed: {} op/sec,  {} sec/ operation.",
+                            println!("{:<10?} elapsed: {:.5} op/sec, {:4} sec/operation.",
                                 start.elapsed(),
                                 self.progress.load(Ordering::SeqCst) as f64 / start.elapsed().as_secs_f64(),
                                 Duration::seconds_f64( start.elapsed().as_secs_f64() / self.progress.load(Ordering::SeqCst) as f64 ));

sdk/keyvault/azure_security_keyvault_secrets/perf/get_secret.rs

@@ -35,24 +35,19 @@ impl GetSecrets {
     }
 
     fn create_new_test(runner: &PerfRunner) -> CreatePerfTestReturn {
-        let vault_url_ref: Option<&String> = match runner.try_get_test_arg("vault_url") {
-            Ok(v) => v,
-            Err(e) => {
-                // Return a future that immediately yields the error.
-                return Box::pin(async move { Err(e) });
-            }
-        };
-        // Own the String so the future can be 'static.
-        let vault_url = vault_url_ref
-            .expect("vault_url argument is mandatory")
-            .clone();
-        Box::pin(async move {
+        async fn create_secret_client(runner: PerfRunner) -> Result<Box<dyn PerfTest>> {
+            let vault_url_ref: Option<&String> = runner.try_get_test_arg("vault_url")?;
+            let vault_url = vault_url_ref
+                .expect("vault_url argument is mandatory")
+                .clone();
             Ok(Box::new(GetSecrets {
                 vault_url,
                 random_key_name: OnceLock::new(),
                 client: OnceLock::new(),
             }) as Box<dyn PerfTest>)
-        })
+        }
+
+        Box::pin(create_secret_client(runner.clone()))
     }
 
     fn create_random_key_name() -> String {
@@ -103,8 +98,7 @@ impl PerfTest for GetSecrets {
             .unwrap()
             .get_secret(self.get_random_key_name(), None)
             .await?
-            .into_body()
-            .await?;
+            .into_body()?;
         Ok(())
     }
 }