Resolve-Dependency.ps1: fix MinimumPSDependVersion comparison

[phbits]

Pull Request

Pull Request (PR) description

Updated MinimumPSDependVersion to use [System.Version]::New(<string>).

The only issue I've found with using this technique is using a single digit will create an error:

PS C:\> [System.Version]::new('1')
Exception calling ".ctor" with "1" argument(s): "Version string portion was too short or too long."
At line:1 char:1
+ [System.Version]::new('1')
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [], MethodInvocationException
    + FullyQualifiedErrorId : ArgumentException

Adding .0 fixes this.

PS C:\> [System.Version]::new('1.0')

Major  Minor  Build  Revision
-----  -----  -----  --------
1      0      -1     -1

Could add parameter validation though that would require a separate function since the following attribute throws an error if no value is specified.

[ValidateScript({[System.Version]::new($_)})]

I'm impartial on how you'd like to handle input validation. All I know is comparing version numbers works best using System.Version.

Fixed

• Resolve-Dependency.ps1: fix MinimumPSDependVersion comparison to use System.Version

Task list

• [ ] The PR represents a single logical change. i.e. Cosmetic updates should go in different PRs.
• [ ] Added an entry under the Unreleased section of in the CHANGELOG.md as per format.
• [ ] Local clean build passes without issue or fail tests (build.ps1 -ResolveDependency).
• [ ] Resource documentation added/updated in README.md.
• [ ] Resource parameter descriptions added/updated in README.md, schema.mof and comment-based help.
• [ ] Comment-based help added/updated.
• [ ] Localization strings added/updated in all localization files as appropriate.
• [ ] Examples appropriately added/updated.
• [ ] Unit tests added/updated. See DSC Resource Testing Guidelines.
• [ ] Integration tests added/updated (where possible). See DSC Resource Testing Guidelines.
• [ ] New/changed code adheres to DSC Resource Style Guidelines and Best Practices.

---

This change is 

[codecov[bot]]

Codecov Report

Merging #271 (1fc2c70) into master (00fd55d) will not change coverage. The diff coverage is n/a.

@@          Coverage Diff          @@
##           master   #271   +/-   ##
=====================================
  Coverage      13%    13%           
=====================================
  Files          24     24           
  Lines         431    431           
=====================================
  Hits           58     58           
  Misses        373    373           

[johlju]

@phbits we should update the Plaster template the same way I think: https://github.com/gaelcolas/Sampler/blob/master/Sampler/Templates/Build/Resolve-Dependency.ps1

@gaelcolas what do you think about the validation @phbits is mentioning?

[phbits]

@johlju Thanks for the suggestion. Just made the change.

[johlju]

The part I'm not clear on is how this change will impact scripts which call Resolve-Dependency.ps1.

This script is dot-sourced from within the build.ps1, and is normally not executed on its own. The parameter MinimumPSDependVersion is read from Resolve-Dependency.psd1), but it can also exist as a variable in the parent scope.

I suggest we skip parameter validation due to the script is being dot-sourced by the build.ps1 script.

When using the below code the variable $MinimumPSDependVersion is automatically converted to [System.Version] because that type is on the left-hand of the operator.

$psDependModule = $psDependModule | Where-Object -FilterScript { $_.Version -ge $MinimumPSDependVersion }

Using that and providing the value '1' it will actually give an even better error message:

PS > $a = Get-Module -Name 'PSDepend' -ListAvailable
PS > $a | Where-Object -FilterScript { $_.Version -ge '1' }
InvalidOperation: Could not compare "0.3.8" to "1". Error: "Cannot convert value "1" to type "System.Version". Error: "Version string portion was too short or too long. (Parameter 'input')""

PSDepend is using sematic versioning, so providing a value other than that should be rare. But if we want to point the user to where the error happened, if providing just '1', or '2', then we should wrap the comparison within a try-catch block.

So then the suggestion would be that we use:

try 
{
    $psDependModule = $psDependModule | Where-Object -FilterScript { $_.Version -ge $MinimumPSDependVersion }
}
catch 
{
    throw ('There was a problem finding the minimum version of PSDepend. Error: {0}' -f $_)
}

[phbits]

@johlju Thanks for the feedback! Updated the files with your try/catch approach and resolved the CHANGELOG conflict.

[johlju]

@phbits thank you for your contribution! 😃