Fix registry resource what-if for non-existing key - #1692
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes dsc config set --what-if for the Windows Registry resource when the desired state is removal (_exist: false) but the target key does not exist, preventing empty/invalid JSON output that previously caused JSON: EOF while parsing a value failures.
Changes:
- Add what-if handling for delete operations when the registry key is missing (emit a “would do nothing” what-if message instead of returning no output).
- Relax
DeleteResultdeserialization to allow/ignore extra properties returned by resources for delete what-if output. - Add a regression test and a localized message string for the non-existing-key delete what-if path.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| resources/registry/tests/registry.config.whatif.tests.ps1 | Adds a config-level what-if regression test for removing a non-existing registry key. |
| lib/dsc-lib/src/dscresources/invoke_result.rs | Removes strict unknown-field rejection for DeleteResult so delete outputs can include extra fields without failing parsing. |
| lib/dsc-lib-registry/src/lib.rs | Implements delete what-if behavior for missing keys by returning metadata instead of None. |
| lib/dsc-lib-registry/locales/en-us.toml | Adds the localized what-if message for “key not found, would do nothing”. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
Code coverage collection failure is due to rust-lang/rust#77553 |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
| whatIfCreateKey = "Key '%{subkey}' not found, would create it" | ||
| whatIfDeleteValue = "Would delete value '%{value_name}'" | ||
| whatIfDeleteSubkey = "Would delete subkey '%{subkey_name}'" | ||
| whatIfDeleteNonexistingKey = "Key '%{subkey}' not found, would do nothing" |
There was a problem hiding this comment.
Minor suggestion on wording, even though it deviates from the format of the existing messages:
| whatIfDeleteNonexistingKey = "Key '%{subkey}' not found, would do nothing" | |
| whatIfDeleteNonexistingKey = "Key '%{subkey}' does not exist, no action would be taken" |
There was a problem hiding this comment.
I like your suggestion, feel free to create a new PR if you want to change it (maybe also review the other messages)
…#1698) * Fix registry resource what-if for non-existing key (#1692) * Fix registry resource what-if for non-existing key * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Update how exit code is returned to work with code cov tools * Allow coverage threshold override label Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Steve Lee (POWERSHELL HE/HIM) (from Dev Box) <slee@ntdev.microsoft.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Fix clippy rule violation (#1688) * Fix clippy rule violation * fix build on Windows * Add registry decoder test coverage Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * remove incorrect msrustup env var --------- Co-authored-by: Steve Lee (POWERSHELL HE/HIM) (from Dev Box) <slee@ntdev.microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Steve Lee <slee@microsoft.com> Co-authored-by: Steve Lee (POWERSHELL HE/HIM) (from Dev Box) <slee@ntdev.microsoft.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
PR Summary
The registry library didn't handle the case where it's
what-ifand the key didn't exist and_exist: falsewhich simply returned nothing as nothing needed to be done, however, this empty result causes thesetoperation to fail with an error.The fix here is if this is
what-ifand the key doesn't exist, then add metadata recognizing that the key already doesn't exist so nothing would be done.This required one other change to allow additional properties to
DeleteResultwhich gets thrown away. The reason for this is to simplify resource development so they can return the same struct output fordeleteas in other cases instead of special casing within the resource.Due to a known issue with the rust code cov tools, needed to update how the main process exits to return an exit code as the current use of
process::exit()somehow causes the coverage data to not be collected.For this config:
now results in this output (instead of an error):
PR Context
Fix #1691