Merge pull request #595 from SteveL-MSFT/assert-array

fix output of Assertion resource for test to resemble config and add support for array comparison

dsc/assertion.dsc.resource.json

@@ -37,7 +37,7 @@
       "config",
       "--as-group",
       "test",
-      "--as-get"
+      "--as-config"
     ],
     "input": "stdin",
     "return": "state"

dsc/src/args.rs

@@ -102,8 +102,12 @@ pub enum ConfigSubCommand {
         path: Option<String>,
         #[clap(short = 'f', long, help = "The output format to use")]
         format: Option<OutputFormat>,
+        // Used by Assertion resource to return `test` result as a `get` result
         #[clap(long, hide = true)]
         as_get: bool,
+        // Used by Assertion resource to return `test` result as a configuration `test` result
+        #[clap(long, hide = true)]
+        as_config: bool,
     },
     #[clap(name = "validate", about = "Validate the current configuration", hide = true)]
     Validate {

dsc/src/subcommand.rs

@@ -6,12 +6,23 @@ use crate::resolve::{get_contents, Include};
 use crate::resource_command::{get_resource, self};
 use crate::tablewriter::Table;
 use crate::util::{DSC_CONFIG_ROOT, EXIT_DSC_ERROR, EXIT_INVALID_ARGS, EXIT_INVALID_INPUT, EXIT_JSON_ERROR, get_schema, write_output, get_input, set_dscconfigroot, validate_json};
-use dsc_lib::configure::{Configurator, config_doc::{Configuration, ExecutionKind}, config_result::ResourceGetResult};
-use dsc_lib::dscerror::DscError;
-use dsc_lib::dscresources::invoke_result::ResolveResult;
 use dsc_lib::{
+    configure::{
+        config_doc::{
+            Configuration,
+            ExecutionKind,
+            Resource,
+        },
+        config_result::ResourceGetResult,
+        Configurator,
+    },
+    dscerror::DscError,
     DscManager,
-    dscresources::invoke_result::ValidateResult,
+    dscresources::invoke_result::{
+        ResolveResult,
+        TestResult,
+        ValidateResult,
+    },
     dscresources::dscresource::{Capability, ImplementedAs, Invoke},
     dscresources::resource_manifest::{import_manifest, ResourceManifest},
 };
@@ -93,12 +104,48 @@ pub fn config_set(configurator: &mut Configurator, format: &Option<OutputFormat>
     }
 }
 
-pub fn config_test(configurator: &mut Configurator, format: &Option<OutputFormat>, as_group: &bool, as_get: &bool)
+pub fn config_test(configurator: &mut Configurator, format: &Option<OutputFormat>, as_group: &bool, as_get: &bool, as_config: &bool)
 {
     match configurator.invoke_test() {
         Ok(result) => {
             if *as_group {
-                let json = if *as_get {
+                let json = if *as_config {
+                    let mut result_configuration = Configuration::new();
+                    result_configuration.resources = Vec::new();
+                    for test_result in result.results {
+                        let properties = match test_result.result {
+                            TestResult::Resource(test_response) => {
+                                if test_response.actual_state.is_object() {
+                                    test_response.actual_state.as_object().cloned()
+                                } else {
+                                    debug!("actual_state is not an object");
+                                    None
+                                }
+                            },
+                            TestResult::Group(_) => {
+                                // not expected
+                                debug!("Unexpected Group TestResult");
+                                None
+                            }
+                        };
+                        let resource = Resource {
+                            name: test_result.name,
+                            resource_type: test_result.resource_type,
+                            properties,
+                            depends_on: None,
+                            metadata: None,
+                        };
+                        result_configuration.resources.push(resource);
+                    }
+                    match serde_json::to_string(&result_configuration) {
+                        Ok(json) => json,
+                        Err(err) => {
+                            error!("JSON Error: {err}");
+                            exit(EXIT_JSON_ERROR);
+                        }
+                    }
+                }
+                else if *as_get {
                     let mut group_result = Vec::<ResourceGetResult>::new();
                     for test_result in result.results {
                         group_result.push(test_result.into());
@@ -294,8 +341,8 @@ pub fn config(subcommand: &ConfigSubCommand, parameters: &Option<String>, mounte
         ConfigSubCommand::Set { format, .. } => {
             config_set(&mut configurator, format, as_group);
         },
-        ConfigSubCommand::Test { format, as_get, .. } => {
-            config_test(&mut configurator, format, as_group, as_get);
+        ConfigSubCommand::Test { format, as_get, as_config, .. } => {
+            config_test(&mut configurator, format, as_group, as_get, as_config);
         },
         ConfigSubCommand::Validate { document, path, format} => {
             let mut result = ValidateResult {

dsc/tests/dsc_config_test.tests.ps1

@@ -0,0 +1,35 @@
+# Copyright (c) Microsoft Corporation.
+# Licensed under the MIT License.
+
+Describe 'dsc config test tests' {
+    It 'Assertion works correctly' {
+        $configYaml = @'
+ $schema: https://raw.githubusercontent.com/PowerShell/DSC/main/schemas/2023/08/config/document.json
+ resources:
+   - name: Operating System Assertion
+     type: Microsoft.DSC/Assertion
+     properties:
+       $schema: https://raw.githubusercontent.com/PowerShell/DSC/main/schemas/2023/08/config/document.json
+       resources:
+         - name: Is64BitOS
+           type: Microsoft/OSInfo
+           properties:
+             bitness: '64'
+         - name: 64bit test 2
+           type: Microsoft/OSInfo
+           properties:
+             family: Windows
+'@
+
+        $out = dsc config test -d $configYaml | ConvertFrom-Json
+        $LASTEXITCODE | Should -Be 0
+
+        if ($IsWindows) {
+            $out.results[0].result.inDesiredState | Should -BeTrue
+        }
+        else {
+            $out.results[0].result.inDesiredState | Should -BeFalse
+            $out.results[0].result.differingProperties | Should -Contain 'resources'
+        }
+    }
+}

dsc_lib/src/discovery/command_discovery.rs

@@ -95,7 +95,7 @@ impl CommandDiscovery {
             Ok(v) => {
                 resource_path_setting = v;
             },
-            Err(e) => { 
+            Err(e) => {
                 debug!("{e}");
             }
         }
@@ -144,7 +144,7 @@ impl CommandDiscovery {
                     paths.push(exe_home_pb);
 
                     if let Ok(new_path) = env::join_paths(paths.clone()) {
-                        env::set_var("PATH", &new_path);
+                        env::set_var("PATH", new_path);
                     }
                 }
             }
@@ -369,7 +369,7 @@ impl ResourceDiscovery for CommandDiscovery {
         } else {
             self.discover_resources("*")?;
             self.discover_adapted_resources(type_name_filter, adapter_name_filter)?;
-            
+
             // add/update found adapted resources to the lookup_table
             add_resources_to_lookup_table(&self.adapted_resources);
 
@@ -652,7 +652,7 @@ fn save_adapted_resources_lookup_table(lookup_table: &HashMap<String, String>)
 fn load_adapted_resources_lookup_table() -> HashMap<String, String>
 {
     let file_path = get_lookup_table_file_path();
-    
+
     let lookup_table: HashMap<String, String> = match fs::read(file_path.clone()){
         Ok(data) => { serde_json::from_slice(&data).unwrap_or_default() },
         Err(_) => { HashMap::new() }

dsc_lib/src/dscresources/dscresource.rs

@@ -7,7 +7,7 @@ use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
 use serde_json::Value;
 use std::collections::HashMap;
-use tracing::debug;
+use tracing::{debug, info};
 
 use super::{command_resource, dscerror, invoke_result::{ExportResult, GetResult, ResolveResult, ResourceTestResponse, SetResult, TestResult, ValidateResult}, resource_manifest::import_manifest};
 
@@ -338,6 +338,16 @@ pub fn get_well_known_properties() -> HashMap<String, Value> {
 }
 
 #[must_use]
+/// Performs a comparison of two JSON Values if the expected is a strict subset of the actual
+///
+/// # Arguments
+///
+/// * `expected` - The expected value
+/// * `actual` - The actual value
+///
+/// # Returns
+///
+/// An array of top level properties that differ, if any
 pub fn get_diff(expected: &Value, actual: &Value) -> Vec<String> {
     let mut diff_properties: Vec<String> = Vec::new();
     if expected.is_null() {
@@ -363,28 +373,176 @@ pub fn get_diff(expected: &Value, actual: &Value) -> Vec<String> {
             if value.is_object() {
                 let sub_diff = get_diff(value, &actual[key]);
                 if !sub_diff.is_empty() {
+                    debug!("diff: sub diff for {key}");
                     diff_properties.push(key.to_string());
                 }
             }
             else {
-                match actual.as_object() {
-                    Some(actual_object) => {
-                        if actual_object.contains_key(key) {
-                            if value != &actual[key] {
+                // skip `$schema` key as that is provided as input, but not output typically
+                if key == "$schema" {
+                    continue;
+                }
+
+                if let Some(actual_object) = actual.as_object() {
+                    if actual_object.contains_key(key) {
+                        if let Some(value_array) = value.as_array() {
+                            if let Some(actual_array) = actual[key].as_array() {
+                                if !is_same_array(value_array, actual_array) {
+                                    info!("diff: arrays differ for {key}");
+                                    diff_properties.push(key.to_string());
+                                }
+                            } else {
+                                info!("diff: {} is not an array", actual[key]);
                                 diff_properties.push(key.to_string());
                             }
-                        }
-                        else {
+                        } else if value != &actual[key] {
                             diff_properties.push(key.to_string());
                         }
-                    },
-                    None => {
+                    } else {
+                        info!("diff: {key} missing");
                         diff_properties.push(key.to_string());
-                    },
+                    }
+                } else {
+                    info!("diff: {key} not object");
+                    diff_properties.push(key.to_string());
                 }
             }
         }
     }
 
     diff_properties
 }
+
+/// Compares two arrays independent of order
+fn is_same_array(expected: &Vec<Value>, actual: &Vec<Value>) -> bool {
+    if expected.len() != actual.len() {
+        info!("diff: arrays are different lengths");
+        return false;
+    }
+
+    for item in expected {
+        if !array_contains(actual, item) {
+            info!("diff: actual array missing expected element");
+            return false;
+        }
+    }
+
+    true
+}
+
+fn array_contains(array: &Vec<Value>, find: &Value) -> bool {
+    for item in array {
+        if find.is_boolean() && item.is_boolean() && find.as_bool().unwrap() == item.as_bool().unwrap() {
+            return true;
+        }
+
+        if find.is_f64() && item.is_f64() && (find.as_f64().unwrap() - item.as_f64().unwrap()).abs() < 0.1 {
+            return true;
+        }
+
+        if find.is_i64() && item.is_i64() && find.as_i64().unwrap() == item.as_i64().unwrap() {
+            return true;
+        }
+
+        if find.is_null() && item.is_null() {
+            return true;
+        }
+
+        if find.is_number() && item.is_number() && find.as_number().unwrap() == item.as_number().unwrap() {
+            return true;
+        }
+
+        if find.is_string() && item.is_string() && find.as_str().unwrap() == item.as_str().unwrap() {
+            return true;
+        }
+
+        if find.is_u64() && item.is_u64() && find.as_u64().unwrap() == item.as_u64().unwrap() {
+            return true;
+        }
+
+        if find.is_object() && item.is_object() {
+            let obj_diff = get_diff(find, item);
+            if obj_diff.is_empty() {
+                return true;
+            }
+        }
+
+        if find.is_array() && item.is_array() && is_same_array(item.as_array().unwrap(), find.as_array().unwrap()) {
+            return true;
+        }
+    }
+
+    false
+}
+
+#[test]
+fn same_array() {
+    use serde_json::json;
+    let array_one = vec![json!("a"), json!(1), json!({"a":"b"}), json!(null)];
+    let array_two = vec![json!("a"), json!(1), json!({"a":"b"}), json!(null)];
+    assert_eq!(is_same_array(&array_one, &array_two), true);
+}
+
+#[test]
+fn same_array_out_of_order() {
+    use serde_json::json;
+    let array_one = vec![json!("a"), json!(true), json!({"a":"b"})];
+    let array_two = vec![json!({"a":"b"}), json!("a"), json!(true)];
+    assert_eq!(is_same_array(&array_one, &array_two), true);
+}
+
+#[test]
+fn different_array() {
+    use serde_json::json;
+    let array_one = vec![json!("a"), json!(1), json!({"a":"b"})];
+    let array_two = vec![json!({"a":"b"}), json!("a"), json!(2)];
+    assert_eq!(is_same_array(&array_one, &array_two), false);
+}
+
+#[test]
+fn different_array_sizes() {
+    use serde_json::json;
+    let array_one = vec![json!("a"), json!(1), json!({"a":"b"})];
+    let array_two = vec![json!({"a":"b"}), json!("a")];
+    assert_eq!(is_same_array(&array_one, &array_two), false);
+}
+
+#[test]
+fn array_with_multiple_objects_with_actual_superset() {
+    use serde_json::json;
+    let array_one = vec![json!("a"), json!(1), json!({"a":"b"}), json!({"c":"d"})];
+    let array_two = vec![json!("a"), json!(1), json!({"c":"d", "a":"b"}), json!({"c":"d"})];
+    assert_eq!(is_same_array(&array_one, &array_two), true);
+}
+
+#[test]
+fn array_with_multiple_objects_with_expected_superset() {
+    use serde_json::json;
+    let array_one = vec![json!("a"), json!(1), json!({"a":"b", "c":"d"}), json!({"c":"d"})];
+    let array_two = vec![json!("a"), json!(1), json!({"a":"b"}), json!({"c":"d"})];
+    assert_eq!(is_same_array(&array_one, &array_two), false);
+}
+
+#[test]
+fn array_with_duplicates_out_of_order() {
+    use serde_json::json;
+    let array_one = vec![json!("a"), json!(1), json!({"a":"b"}), json!({"a":"b"})];
+    let array_two = vec![json!({"a":"b"}), json!("a"), json!(1), json!({"a":"b"})];
+    assert_eq!(is_same_array(&array_one, &array_two), true);
+}
+
+#[test]
+fn same_array_with_nested_array() {
+    use serde_json::json;
+    let array_one = vec![json!("a"), json!(1), json!({"a":"b"}), json!(vec![json!("a"), json!(1)])];
+    let array_two = vec![json!("a"), json!(1), json!({"a":"b"}), json!(vec![json!("a"), json!(1)])];
+    assert_eq!(is_same_array(&array_one, &array_two), true);
+}
+
+#[test]
+fn different_array_with_nested_array() {
+    use serde_json::json;
+    let array_one = vec![json!("a"), json!(1), json!({"a":"b"}), json!(vec![json!("a"), json!(1)])];
+    let array_two = vec![json!("a"), json!(1), json!({"a":"b"}), json!(vec![json!("a"), json!(2)])];
+    assert_eq!(is_same_array(&array_one, &array_two), false);
+}