Add config tests for analysis task

src/agent/onefuzz-task/src/config_test_utils.rs

@@ -1,3 +1,13 @@
+use onefuzz::expand::{GetExpand, PlaceHolder};
+
+// Moving this trait method into the GetExpand trait--and returning `Vec<(PlaceHolder, Box<dyn Any>)>` instead
+// would let us use define a default implementation for `get_expand()` while also coupling the expand values we
+// test with those we give to the expander.
+// It seems to me like a non-trivial (and perhaps bad) design change though.
+pub trait GetExpandFields: GetExpand {
+    fn get_expand_fields(&self) -> Vec<(PlaceHolder, String)>;
+}
+
 pub mod arbitraries {
     use std::path::PathBuf;
 
@@ -7,7 +17,7 @@ pub mod arbitraries {
     use reqwest::Url;
     use uuid::Uuid;
     
-    use crate::tasks::config::CommonConfig;
+    use crate::tasks::{config::CommonConfig, analysis};
     
     prop_compose! {
         fn arb_uuid()(
@@ -85,9 +95,20 @@ pub mod arbitraries {
             }
         }
     }
-    
+
     prop_compose! {
-        fn arb_common_config(tag_limit: usize)(
+        fn arb_string_vec_no_vars()(
+            // I don't know how to figure out the expected value of the target options if they could contain variables (e.g. {machine_id})
+            // This should be fine since this isn't used to test nested expansion
+            options in prop::collection::vec("[^{}]*", 10),
+        ) -> Vec<String> {
+            options
+        }
+    }
+    
+
+    prop_compose! {
+        fn arb_common_config()(
             job_id in arb_uuid(),
             task_id in arb_uuid(),
             instance_id in arb_uuid(),
@@ -101,7 +122,7 @@ pub mod arbitraries {
             extra_output in option::of(arb_synced_dir()),
             min_available_memory_mb in any::<u64>(),
             machine_identity in arb_machine_identity(),
-            tags in prop::collection::hash_map(".*", ".*", tag_limit),
+            tags in prop::collection::hash_map(".*", ".*", 10),
             from_agent_to_task_endpoint in ".*",
             from_task_to_agent_endpoint in ".*",
         ) -> CommonConfig {
@@ -131,12 +152,50 @@ pub mod arbitraries {
         type Strategy = BoxedStrategy<Self>;
     
         fn arbitrary_with(_args: Self::Parameters) -> Self::Strategy {
-            arb_common_config(10).boxed()
+            arb_common_config().boxed()
         }
     }
-    
+
-    // Make a trait out of this and add it to a common test module
+    prop_compose! {
-    impl CommonConfig {
+        fn arb_analysis_config()(
-        // Get all the fields from the type that are passed to the expander
+            analyzer_exe in Just("src/lib.rs".to_string()),
+            analyzer_options in arb_string_vec_no_vars(),
+            analyzer_env in prop::collection::hash_map(".*", ".*", 10),
+            target_exe in arb_pathbuf(),
+            target_options in arb_string_vec_no_vars(),
+            input_queue in Just(None),
+            crashes in option::of(arb_synced_dir()),
+            analysis in arb_synced_dir(),
+            tools in option::of(arb_synced_dir()),
+            reports in option::of(arb_synced_dir()),
+            unique_reports in option::of(arb_synced_dir()),
+            no_repro in option::of(arb_synced_dir()),
+            common in arb_common_config(),
+        ) -> analysis::generic::Config {
+            analysis::generic::Config {
+                analyzer_exe,
+                analyzer_options,
+                analyzer_env,
+                target_exe,
+                target_options,
+                input_queue,
+                crashes,
+                analysis,
+                tools,
+                reports,
+                unique_reports,
+                no_repro,
+                common,
+            }
+        }
+    }
+
+    impl Arbitrary for analysis::generic::Config {
+        type Parameters = ();
+        type Strategy = BoxedStrategy<Self>;
+
+        fn arbitrary_with(_args: Self::Parameters) -> Self::Strategy {
+            arb_analysis_config().boxed()
+        }
     }
 }

src/agent/onefuzz-task/src/lib.rs

@@ -8,4 +8,4 @@ extern crate onefuzz_telemetry;
 pub mod local;
 pub mod tasks;
 #[cfg(test)]
-pub mod test_utils;
+pub mod config_test_utils;

src/agent/onefuzz-task/src/tasks/analysis/generic.rs

@@ -268,3 +268,55 @@ pub async fn run_tool(
         .with_context(|| format!("analyzer failed to run: {analyzer_path}"))?;
     Ok(())
 }
+
+#[cfg(test)]
+mod tests {
+    use proptest::prelude::*;
+    use onefuzz::expand::{GetExpand, PlaceHolder};
+
+    use crate::config_test_utils::GetExpandFields;
+
+    use super::Config;
+
+    impl GetExpandFields for Config {
+        fn get_expand_fields(&self) -> Vec<(PlaceHolder, String)> {
+            let mut params = self.common.get_expand_fields();
+            params.push((PlaceHolder::AnalyzerExe, dunce::canonicalize(&self.analyzer_exe).unwrap().to_string_lossy().to_string()));
+            params.push((PlaceHolder::AnalyzerOptions, self.analyzer_options.join(" ")));
+            params.push((PlaceHolder::TargetExe, dunce::canonicalize(&self.target_exe).unwrap().to_string_lossy().to_string()));
+            params.push((PlaceHolder::TargetOptions, self.target_options.join(" ")));
+            params.push((PlaceHolder::OutputDir, dunce::canonicalize(&self.analysis.local_path).unwrap().to_string_lossy().to_string()));
+            if let Some(tools) = &self.tools {
+                params.push((PlaceHolder::ToolsDir, dunce::canonicalize(&tools.local_path).unwrap().to_string_lossy().to_string()));
+            }
+            if let Some(reports) = &self.reports {
+                params.push((PlaceHolder::ReportsDir, dunce::canonicalize(&reports.local_path).unwrap().to_string_lossy().to_string()));
+            }
+            if let Some(crashes) = &self.crashes {
+                if let Some(account) = crashes.remote_path.clone().and_then(|u| u.account()) {
+                    params.push((PlaceHolder::CrashesAccount, account));
+                }
+                if let Some(container) = crashes.remote_path.clone().and_then(|u| u.container()) {
+                    params.push((PlaceHolder::CrashesContainer, container));
+                }
+            }
+
+            params
+        }
+    }
+
+    proptest! {
+        #[test]
+        fn test_get_expand_values_match_config(
+            config in any::<Config>(),
+        ) {
+            let expand = config.get_expand();
+            let params = config.get_expand_fields();
+
+            for (param, expected) in params.iter() {
+                let evaluated = expand.evaluate_value(format!("{}", param.get_string())).unwrap();
+                assert_eq!(evaluated, *expected, "placeholder {} did not match expected value", param.get_string());
+            }
+        }
+    }
+}

src/agent/onefuzz-task/src/tasks/config.rs

@@ -131,6 +131,7 @@ impl GetExpand for CommonConfig {
             .machine_id()
             .job_id(&self.job_id)
             .task_id(&self.task_id)
+            .setup_dir(&self.setup_dir)
             .set_optional_ref(
                 &self.instance_telemetry_key,
                 Expand::instance_telemetry_key
@@ -139,7 +140,6 @@ impl GetExpand for CommonConfig {
                 &self.microsoft_telemetry_key,
                 Expand::microsoft_telemetry_key
             )
-            .setup_dir(&self.setup_dir)
             .set_optional_ref(
                 &self.extra_setup_dir,
                 Expand::extra_setup_dir
@@ -393,43 +393,47 @@ impl Config {
 #[cfg(test)]
 mod tests {
     use proptest::prelude::*;
-    use onefuzz::expand::GetExpand;
+    use onefuzz::expand::{GetExpand, PlaceHolder};
-    use std::collections::HashMap;
+
+    use crate::config_test_utils::GetExpandFields;
 
     use super::CommonConfig;
 
+    impl GetExpandFields for CommonConfig {
+        fn get_expand_fields(&self) -> Vec<(PlaceHolder, String)> {
+            let mut params = vec![
+                (PlaceHolder::MachineId, self.machine_identity.machine_id.to_string()),
+                (PlaceHolder::JobId, self.job_id.to_string()),
+                (PlaceHolder::TaskId, self.task_id.to_string()),
+                (PlaceHolder::SetupDir, dunce::canonicalize(&self.setup_dir).unwrap().to_string_lossy().to_string()),
+            ];
+            if let Some(key) = &self.instance_telemetry_key {
+                params.push((PlaceHolder::InstanceTelemetryKey, key.to_string()));
+            }
+            if let Some(key) = &self.microsoft_telemetry_key {
+                params.push((PlaceHolder::MicrosoftTelemetryKey, key.clone().to_string()));
+            }
+            if let Some(dir) = &self.extra_setup_dir {
+                params.push((PlaceHolder::ExtraSetupDir, dunce::canonicalize(dir).unwrap().to_string_lossy().to_string()));
+            }
+            if let Some(dir) = &self.extra_output {
+                params.push((PlaceHolder::ExtraOutputDir, dunce::canonicalize(&dir.local_path).unwrap().to_string_lossy().to_string()));
+            }
+
+            params
+        }
+    }
+
     proptest! {
-        // generate an arbitrary config
-        // map the expanded values from the config to their expander names
-        // verify that the get_expand() result has all the same values as are in the config
         #[test]
         fn test_get_expand_values_match_config(
             config in any::<CommonConfig>(),
         ) {
             let expand = config.get_expand();
-
+            let params = config.get_expand_fields();
-            // for now, use a hardcoded list of parameter names that the config supplies
-            let mut params = HashMap::from([
-                ("machine_id", config.machine_identity.machine_id.to_string()),
-                ("job_id", config.job_id.to_string()),
-                ("task_id", config.task_id.to_string()),
-                ("setup_dir", dunce::canonicalize(config.setup_dir.clone()).unwrap().to_string_lossy().to_string()),
-            ]);
-            if let Some(key) = &config.instance_telemetry_key {
-                params.insert("instance_telemetry_key", key.to_string());
-            }
-            if let Some(key) = &config.microsoft_telemetry_key {
-                params.insert("microsoft_telemetry_key", key.clone().to_string());
-            }
-            if let Some(dir) = &config.extra_setup_dir {
-                params.insert("extra_setup_dir", dunce::canonicalize(dir).unwrap().to_string_lossy().to_string());
-            }
-            if let Some(dir) = &config.extra_output {
-                params.insert("extra_output_dir", dunce::canonicalize(&dir.local_path).unwrap().to_string_lossy().to_string());
-            }
 
             for (param, expected) in params.iter() {
-                let evaluated = expand.evaluate_value(format!("{{{param}}}")).unwrap();
+                let evaluated = expand.evaluate_value(format!("{}", param.get_string())).unwrap();
                 assert_eq!(evaluated, *expected);
             }
         }