Merge pull request #599 from SteveL-MSFT/assertion-fix

Assertion fix

.github/workflows/rust.yml

@@ -2,9 +2,9 @@ name: Rust
 
 on:
   push:
-    branches: [ "main" ]
+    branches: [ "main", "3.0.0-rc1" ]
   pull_request:
-    branches: [ "main" ]
+    branches: [ "main", "3.0.0-rc1" ]
     paths-ignore:
       - "docs/**"
       - "*.md"

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

@@ -99,8 +99,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_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},
 };
@@ -90,12 +101,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());
@@ -282,8 +329,8 @@ pub fn config(subcommand: &ConfigSubCommand, parameters: &Option<String>, stdin:
         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_lib/src/discovery/command_discovery.rs

@@ -75,8 +75,7 @@ impl CommandDiscovery {
                     paths.push(exe_home_pb);
 
                     if let Ok(new_path) = env::join_paths(paths.clone()) {
-                        debug!("Using PATH: {:?}", new_path.to_string_lossy());
-                        env::set_var("PATH", &new_path);
+                        env::set_var("PATH", new_path);
                     }
                 }
             }
@@ -297,7 +296,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);
 
@@ -580,7 +579,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() {
@@ -367,24 +377,171 @@ pub fn get_diff(expected: &Value, actual: &Value) -> Vec<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);
+}

resources/apt/test/apt.tests.ps1

@@ -27,8 +27,8 @@ Describe 'Apt resource tests' {
             if (-not $aptExists) {
                 Set-ItResult -Skip -Because "Apt not found"
             }
-            $out = dsc config get -p $yamlPath | ConvertFrom-Json -Depth 10
-            $LASTEXITCODE | Should -Be 0
+            $out = dsc -l trace config get -p $yamlPath 2> "$TestDrive/stderr.txt" | ConvertFrom-Json -Depth 10
+            $LASTEXITCODE | Should -Be 0 -Because (Get-Content "$TestDrive/stderr.txt" | Out-String)
             $exists = $null -ne (Get-Command $pkgName -CommandType Application -ErrorAction Ignore)
             $observed = $out.results[1].result.actualState._exist
             $observed | Should -Be $exists