BREAKING CHANGE: xWebVirtualDirectory renamed to WebVirtualDirectory

[t3mi]

Pull Request (PR) description

BREAKING CHANGE: Resource xWebVirtualDirectory renamed to WebVirtualDirectory.
Besides that:

• Parameter Website renamed to Site to comply with other resources and PowerShell command keys
• Parameter WebApplication renamed to Application to comply with other resources and PowerShell command keys
• New parameters added: PhysicalPathAccessAccount and PhysicalPathAccessPass which define access credential to physical path.
• Parameter PhysicalPath no longer required for Ensure = 'Absent' case.
• Examples were added to include different scenarios.

This Pull Request (PR) fixes the following issues

• UNC path can be used as a PhysicalPath value #94
• Removal command changed to Remove-Item to hide the confirmation error #366

Task list

• Added an entry under the Unreleased section of the change log in the README.md.
  Entry should say what was changed, and how that affects users (if applicable).
• 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-io]

Codecov Report

Merging #428 into dev will increase coverage by 0.23%.
The diff coverage is 98.38%.

@@           Coverage Diff            @@
##              dev   #428      +/-   ##
========================================
+ Coverage   90.77%    91%   +0.23%     
========================================
  Files          17     17              
  Lines        2438   2456      +18     
========================================
+ Hits         2213   2235      +22     
+ Misses        225    221       -4

Impacted Files	Coverage Δ
..._WebVirtualDirectory/MSFT_WebVirtualDirectory.psm1	98.38% <98.38%> (ø)

---

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 3ed63bc...388c5fa. Read the comment docs.

[regedit32]

@t3mi Thanks for submitting another PR and addressing issues. I will try to make time this weekend to review this and your other PR. Cheers!

[regedit32]

@t3mi sorry it's been a while since I was able to come back to this one. I had some time to review, but not completely. I think there's enough comments for now and I'll come back and finish the review. Thanks for working on this.

Reviewable status: 0 of 17 files reviewed, 18 unresolved discussions (waiting on @t3mi)

---

DSCResources/MSFT_WebVirtualDirectory/MSFT_WebVirtualDirectory.psm1, line 50 at r2 (raw file):

$Ensure

variable should be camelCase

---

DSCResources/MSFT_WebVirtualDirectory/MSFT_WebVirtualDirectory.psm1, line 55 at r2 (raw file):

$Ensure

variable should be camelCase

---

DSCResources/MSFT_WebVirtualDirectory/MSFT_WebVirtualDirectory.psm1, line 60 at r2 (raw file):

$Ensure

camelCase

---

DSCResources/MSFT_WebVirtualDirectory/MSFT_WebVirtualDirectory.psm1, line 285 at r2 (raw file):

Quoted 11 lines of code…

    $virtualDirectory = Get-WebVirtualDirectory -Site $Site `
                                                -Application $Application `
                                                -Name $Name
    # Check Ensure
    if (($Ensure -eq 'Present' -and $virtualDirectory.Count -eq 0) -or `
        ($Ensure -eq 'Absent' -and $virtualDirectory.Count -eq 1))
    {
        $inDesiredState = $false
        Write-Verbose -Message ($LocalizedData.VerboseTestTargetFalseEnsure `
                                -f $Name)
    }

We can simplify some of this logic by calling $targetResource = Get-TargetResource @PSBoundParameters and comparing `$targetResource.Ensure to $Ensure. That will probably simplify some of the later logic as well

---

DSCResources/MSFT_WebVirtualDirectory/MSFT_WebVirtualDirectory.schema.mof, line 8 at r2 (raw file):

    [Write, Description("Specifies the username for physical path access in the virtual directory.")] String PhysicalPathAccessAccount;
    [Write, Description("Specifies the password for physical path access in the virtual directory.")] String PhysicalPathAccessPass;

Let's make this a credential instead so we don't pass in plaintext passwords.

---

Examples/Sample_WebVirtualDirectory_NewVirtualDirectory.ps1, line 13 at r2 (raw file):

        # Target nodes to apply the configuration
        [System.String[]]
        $NodeName = 'localhost',

missing [Parameter()]

---

Examples/Sample_WebVirtualDirectory_NewVirtualDirectoryWithUncShare.ps1, line 13 at r2 (raw file):

        # Target nodes to apply the configuration
        [System.String[]]
        $NodeName = 'localhost',

missing [Parameter()]

---

Tests/Integration/MSFT_WebVirtualDirectory.Integration.Tests.ps1, line 15 at r2 (raw file):

$TestEnvironment

variables should be camelCase

---

Tests/Integration/MSFT_WebVirtualDirectory.Integration.Tests.ps1, line 27 at r2 (raw file):

$ConfigFile

variables should be camelCase

---

Tests/Integration/MSFT_WebVirtualDirectory.Integration.Tests.ps1, line 30 at r2 (raw file):

$DSCConfig

variables should be camelCase. applies throughout

---

Tests/Integration/MSFT_WebVirtualDirectory.Integration.Tests.ps1, line 37 at r2 (raw file):

    Describe "$($script:DSCResourceName)_Present" {

We also need tests to assert Get-DscConfiguration returns the expected values. That is a common area for bugs to occur.

---

Tests/Integration/MSFT_WebVirtualDirectory.Integration.Tests.ps1, line 56 at r2 (raw file):

$result | Should -Be 'True'

Test-DscConfiguration should return a boolean, so let's assert Should -Be $true

---

Tests/Integration/MSFT_WebVirtualDirectory.Integration.Tests.ps1, line 111 at r2 (raw file):

    Describe "$($script:DSCResourceName)_Absent" {

We also need tests to assert Get-DscConfiguration returns the expected values. That is a common area for bugs to occur.

---

Tests/Integration/MSFT_WebVirtualDirectory.Integration.Tests.ps1, line 118 at r2 (raw file):

Invoke-Expression -Command

Per the style guidelines, we should Avoid Invoke-Expression. The call operator & would be more appropriate. Same applies to the other instances of Invoke-Experession in this test.

---

Tests/Unit/MSFT_WebVirtualDirectory.Tests.ps1, line 16 at r2 (raw file):

$TestEnvironment

variables should be camelCase

---

Tests/Unit/MSFT_WebVirtualDirectory.Tests.ps1, line 29 at r2 (raw file):

$MockParameters

variables should be camelCase. Also, let's be more descriptive on the variable name and call it something like $getTargetResourceParameters. We can remove the "mock" in the name, since these are actual parameters we're passing to the function. Let's apply the same idea to the rest of the varibables named $MockParameters

---

Tests/Unit/MSFT_WebVirtualDirectory.Tests.ps1, line 66 at r2 (raw file):

$MockOutput

variables should be camelCase. Also let's be more descriptive on what the variable contains and call it something like $mockGetWebVirtualDirectory. It helps make the script more readable and provides context to the variable when reading it anywhere else in the script. Let's apply the same to the rest of the variables named $MockOutput

---

Tests/Unit/MSFT_WebVirtualDirectory.Tests.ps1, line 183 at r2 (raw file):

Assert-MockCalled -CommandName Get-WebVirtualDirectory -Exactly 1

Missing the named parameter -Times. -Exactly is actually a swtich parameter used in conjunction with -Times. Applies throughout.

[johlju]

The resources has been renamed in another PR. I'm closing this PR, if there are other changes in this PR that need to be merged, please reopen or send in a new pull request.