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

Fixed Compare-DscParameterState returning false positive when parameter with an empty hashtable or CimInstance property is passed in DesriedValues - fixes issue #65 #76

Merged
merged 3 commits into from Sep 10, 2021

Conversation

NicolasBn
Copy link
Member

@NicolasBn NicolasBn commented Jul 16, 2021

Pull Request (PR) description

  • Correction to Compare-DscParameterState returning false positive when parameter
    with an empty hashtable or CimInstance property is passed in DesriedValues - fixes
    issue #65.
  • Correction somes problems in Compare-DscParameterState - see issue #70 :
    • When you use -ReverseCheck, this value is used in recursive call of
      Test-DscParameterState and Compare-DscParameterState, and that called
      another time the function.
    • When you use -Properties and -ReverseCheck, and you have an array in member,
      that return a wrong value, because the properties are set in recursive calls of
      -ReverseCheck to test the value of array.
    • When you use -ReverseCheck and, in the function Test-DscSompareState/Compare-DscParameterState
      are recursively called (like to test or compare value of array), -ReverseCheck
      value is removed from $PSBoundParameter. And the ReverseCheck isn't done.

This Pull Request (PR) fixes the following issues

- Fixes #70 
- Fixes #65 

Task list

  • Added an entry to the change log under the Unreleased section of the
    file CHANGELOG.md. Entry should say what was changed and how that
    affects users (if applicable), and reference the issue being resolved
    (if applicable).
  • Documentation added/updated in README.md.
  • Comment-based help added/updated for all new/changed functions.
  • Localization strings added/updated in all localization files as appropriate.
  • Examples appropriately added/updated.
  • Unit tests added/updated. See DSC Community Testing Guidelines.
  • Integration tests added/updated (where possible). See
    DSC Community Testing Guidelines.
  • New/changed code adheres to DSC Community Style Guidelines.

This change is Reviewable

@codecov
Copy link

codecov bot commented Jul 16, 2021

Codecov Report

❗ No coverage uploaded for pull request base (main@8d7f8d6). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main      #76   +/-   ##
=======================================
  Coverage        ?   91.25%           
=======================================
  Files           ?       26           
  Lines           ?      469           
  Branches        ?        0           
=======================================
  Hits            ?      428           
  Misses          ?       41           
  Partials        ?        0           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8d7f8d6...c81b00e. Read the comment docs.

source/Public/Compare-DscParameterState.ps1 Show resolved Hide resolved
source/en-US/DscResource.Common.strings.psd1 Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@johlju johlju added the needs review The pull request needs a code review. label Aug 6, 2021
Copy link
Member

@gaelcolas gaelcolas left a comment

Choose a reason for hiding this comment

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

Thanks @NicolasBn, that looks good to me but I'd like @raandree to have a quick look too before we click merge.
:lgtm:

Reviewed 1 of 4 files at r1, 3 of 3 files at r2.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @NicolasBn)

@gaelcolas gaelcolas merged commit c816a09 into dsccommunity:main Sep 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs review The pull request needs a code review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants