Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Assertion fix #599

Merged
merged 11 commits into from
Nov 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion dsc/assertion.dsc.resource.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
"config",
"--as-group",
"test",
"--as-get"
"--as-config"
],
"input": "stdin",
"return": "state"
Expand Down
4 changes: 4 additions & 0 deletions dsc/src/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
63 changes: 55 additions & 8 deletions dsc/src/subcommand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
};
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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 {
Expand Down
7 changes: 3 additions & 4 deletions dsc_lib/src/discovery/command_discovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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() }
Expand Down
177 changes: 167 additions & 10 deletions dsc_lib/src/dscresources/dscresource.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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);
}
4 changes: 2 additions & 2 deletions resources/apt/test/apt.tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading