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

Fix when resource test returns '_inDesiredState', that takes precedence #676

Merged
merged 3 commits into from
Mar 10, 2025

Conversation

SteveL-MSFT
Copy link
Member

@SteveL-MSFT SteveL-MSFT commented Mar 6, 2025

PR Summary

Even when a resource implement test returns state and contains _inDesiredState property, that value isn't being used and a synthetic comparison still occurs.

With this change, whether a resource returns state or stateAndDiff and contains _inDesiredState, then that value will be used regardless if there are any differing properties. Diff of properties will still occur for information purposes even if _inDesiredState = true

Also fixed a resource string issue where the placeholder wasn't being replaced correctly.

PR Context

Fix #674

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Overview

This PR fixes the issue where a resource’s _inDesiredState property is ignored in the test process and addresses a resource string placeholder replacement.

  • Introduces a new InDesiredState struct in dsctest to clearly encapsulate the in‐desired‐state information.
  • Adjusts command_resource.rs to prioritize the _inDesiredState value over synthetic diff comparison.
  • Adds a new subcommand and CLI support for the in-desired-state test along with a corresponding JSON schema and localized message updates.

Reviewed Changes

File Description
tools/dsctest/src/in_desired_state.rs Added InDesiredState struct to hold resource state and a supporting field.
dsc_lib/src/dscresources/command_resource.rs Updated invoke_test to consider _inDesiredState for determining desired state.
tools/dsctest/src/main.rs Introduced InDesiredState subcommand and handling of its JSON input.
tools/dsctest/src/args.rs Extended CLI argument parsing for the new in-desired-state subcommand.
dsc_lib/locales/en-us.toml Fixed string placeholder for the executable in error messages.

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

tools/dsctest/src/in_desired_state.rs:12

  • [nitpick] The field name 'value' is ambiguous; consider renaming it to better reflect its purpose, such as 'state_description' or another descriptive term.
pub value: String,

@SteveL-MSFT
Copy link
Member Author

Was thinking about this and originally I figured that if a resource returned stateAndDiff that the diff would determine if in desired state, but I think we should just trust the resource regardless if there is a diff or not. So I'll make that change.

@SteveL-MSFT
Copy link
Member Author

@tgauth please continue the review

@SteveL-MSFT SteveL-MSFT added this to the 3.1-Approved milestone Mar 7, 2025
@SteveL-MSFT SteveL-MSFT added this pull request to the merge queue Mar 10, 2025
Merged via the queue into PowerShell:main with commit 3703da2 Mar 10, 2025
4 checks passed
@SteveL-MSFT SteveL-MSFT deleted the indesiredstate branch March 10, 2025 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

_inDesiredState not being respected in test state output
2 participants