Creating new PR with fixes from PR #59

[aultt]

Corrected Merge conflicts which existed and 59

---

This change is

[aultt]

@mbreakey3 Have a couple questions on this AppVeyor fails with ConvertTo-SecureString warning on pester tests. Is there a better way to handle this in these tests? Also MSFT_xSQLServerFirewall.schema.mof complains about not having a new line however I did not touch this file.

[aultt]

FYI this will close 23 and 59

[mbreakey3]

Yeah, so instead of using a string username/password, it should be declared as a [System.Management.Automation.PSCredential]

[mbreakey3]

And I'm not sure what happened with the mof file, but I would say just add the newline in

[mbreakey3]

So just add this to the top of that test file:

# Suppressing this rule because PlainText is required for one of the functions used in this test
[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSAvoidUsingConvertToSecureStringWithPlainText', '')]
param ()

Since it's a test file, the plaintext is acceptable, so we can just suppress the rule. I apologize for the confusion, I didn't catch this earlier

[mbreakey3]

I'm not quite sure what's going on with appVeyor either - I tried to re-build the commit, but it's not even showing up

[mbreakey3]

Ah, it's because of the merge conflicts, so if you can fix those, appVeyor should be able to run again

[mbreakey3]

So it looks like there's something wrong with how the module is getting imported in the tests. I would recommend starting with our unit test template and then copying your tests into that.

[aultt]

@mbreakey3 Im missing something with these tests but not sure what the issue is can you take a look?

[mbreakey3]

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 20 [r4] (raw file):

# TODO: Customize these parameters...
$script:DSCModuleName      = 'MSFT_xSqlAlias' # Example xNetworking

These should be switched: DSCModuleName is xSQLServer and DSCResourceName is MSFT_xSqlAlias

---

Comments from Reviewable

[mbreakey3]

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 76 [r5] (raw file):

    }

    $SqlAlias = Get-xSqlAliasTargetResource -Name 'localhost'

I think this should be Get-TargetResource

---

Comments from Reviewable

[aultt]

Review status: 0 of 7 files reviewed at latest revision, 2 unresolved discussions.

---

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 20 [r4] (raw file):

Previously, mbreakey3 (Mariah) wrote…

These should be switched: DSCModuleName is xSQLServer and DSCResourceName is MSFT_xSqlAlias

Done.

---

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 76 [r5] (raw file):

Previously, mbreakey3 (Mariah) wrote…

I think this should be Get-TargetResource

Done.

---

Comments from Reviewable

[aultt]

@mbreakey3 Ok I think its finally a go once you approve the files. Thanks for all your help

[mbreakey3]

Review status: 0 of 7 files reviewed at latest revision, 5 unresolved discussions.

---

Tests/Unit/MSFT_xSQLAOGroupEnsure.Tests.ps1, line 4 [r6] (raw file):

[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSAvoidUsingConvertToSecureStringWithPlainText', '')]
param ()
<#

Delete all of the template-comments

---

Tests/Unit/MSFT_xSQLAOGroupEnsure.Tests.ps1, line 99 [r6] (raw file):

        Context "When the configuration is already set" {

            $password = "dummyPassw0rd" | ConvertTo-SecureString -asPlainText -Force

since these 3 variables are used more than once you can move them up to the top of the describe block and call them $mockPassword, $mockUsername, $mockCredential

---

Tests/Unit/MSFT_xSQLAOGroupEnsure.Tests.ps1, line 155 [r6] (raw file):

        Context "When the configuration is valid" {

            $password = "dummyPassw0rd" | ConvertTo-SecureString -asPlainText -Force

move these to the top of the describe block too, as mentioned above

---

Comments from Reviewable

[mbreakey3]

Reviewed 1 of 6 files at r1, 1 of 1 files at r2, 1 of 4 files at r3.
Review status: 3 of 7 files reviewed at latest revision, 7 unresolved discussions.

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 30 [r6] (raw file):

    if ($null -ne (Get-ItemProperty -Path 'HKLM:\SOFTWARE\Microsoft\MSSQLServer\Client\ConnectTo' -Name "$Name" -ErrorAction SilentlyContinue))
    {
        $ItemValue = Get-ItemProperty -Path 'HKLM:\SOFTWARE\Microsoft\MSSQLServer\Client\ConnectTo' -Name "$Name";

None of these semi-colons should be needed

---

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 5 [r6] (raw file):

param ()

<#

remove all template comments

---

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 72 [r6] (raw file):

    #region Get-TargetResource
    Describe 'Get-TargetResource'{
    Mock -ModuleName MSFT_xSqlAlias -CommandName Get-ItemProperty -MockWith {

Make sure tab space is aligned. There should be a 4 space indentation after each curly bracket

---

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 153 [r6] (raw file):

    # TODO: Pester Tests for any non-exported Helper Cmdlets

Again, just make sure to delete all of this excess code from the template

---

Comments from Reviewable

[aultt]

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 30 [r6] (raw file):

Previously, mbreakey3 (Mariah) wrote…

None of these semi-colons should be needed

Done

---

Comments from Reviewable

[aultt]

Review status: 3 of 7 files reviewed at latest revision, 7 unresolved discussions.

---

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 5 [r6] (raw file):

Previously, mbreakey3 (Mariah) wrote…

remove all template comments

Done.

---

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 72 [r6] (raw file):

Previously, mbreakey3 (Mariah) wrote…

Make sure tab space is aligned. There should be a 4 space indentation after each curly bracket

Done.

---

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 153 [r6] (raw file):

Previously, mbreakey3 (Mariah) wrote…

Again, just make sure to delete all of this excess code from the template

Done.

---

Tests/Unit/MSFT_xSQLAOGroupEnsure.Tests.ps1, line 4 [r6] (raw file):

Previously, mbreakey3 (Mariah) wrote…

Delete all of the template-comments

Done.

---

Tests/Unit/MSFT_xSQLAOGroupEnsure.Tests.ps1, line 99 [r6] (raw file):

Previously, mbreakey3 (Mariah) wrote…

since these 3 variables are used more than once you can move them up to the top of the describe block and call them $mockPassword, $mockUsername, $mockCredential

Done.

---

Tests/Unit/MSFT_xSQLAOGroupEnsure.Tests.ps1, line 155 [r6] (raw file):

Previously, mbreakey3 (Mariah) wrote…

move these to the top of the describe block too, as mentioned above

Done.

---

Comments from Reviewable

[mbreakey3]

Review status: 3 of 7 files reviewed at latest revision, 11 unresolved discussions.

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 79 [r7] (raw file):

    Write-Verbose -Message 'Set-TargetResource'

    $ItemValue = [System.String]

If this is going to be declared up here it can just be se to $itemValue = '' (lowercase since a local variable)

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 253 [r7] (raw file):

                    }
                    else
                    {

Could you add some comments for what state each of these nested blocks represent? It's a little hard to follow

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 277 [r7] (raw file):

    Write-Debug -Message "Test-TargetResource Result: $result"

    Return $result

lowercase 'return'

---

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 5 [r6] (raw file):

Previously, aultt (Troy Ault) wrote…

Done.

Still a few more comments - like the # Example on the lines below

---

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 72 [r6] (raw file):

Previously, aultt (Troy Ault) wrote…

Done.

This line starting with Mock should also be indented 4 spaces

---

Tests/Unit/MSFT_xSQLAOGroupEnsure.Tests.ps1, line 5 [r7] (raw file):

param ()

$script:DSCModuleName      = 'xSQLServer' # Example xNetworking

Get rid of the '# Example's

---

Tests/Unit/MSFT_xSQLAOGroupEnsure.Tests.ps1, line 33 [r7] (raw file):

                    # build a custom object to return which is close to the real SMO object
                    $smoObj = [PSCustomObject]@{
                                SQLServer = 'Node01';

All of these semicolons in this file can be removed too

---

Tests/Unit/MSFT_xSQLAOGroupEnsure.Tests.ps1, line 96 [r7] (raw file):

                                        };
                    # we need to be able to call the Add method like we would for an array but it needs to also function like a hashtable so override the Add method
                    #$smoObj.AvailabilityGroups | Add-Member -MemberType ScriptMethod -Name 'Add' -Value {param($value) $_.AvailabilityGroups[$value.Name] = $value} -Force;

Is this commented out line needed?

---

Tests/Unit/MSFT_xSQLAOGroupEnsure.Tests.ps1, line 299 [r7] (raw file):

    #endregion

    # TODO: Other Optional Cleanup Code Goes Here...

Make sure all TODOs are deleted too

---

Comments from Reviewable

[aultt]

Tests/Unit/MSFT_xSQLAOGroupEnsure.Tests.ps1, line 96 [r7] (raw file):

Previously, mbreakey3 (Mariah) wrote…

Is this commented out line needed?

No removed.

---

Comments from Reviewable

[aultt]

Review status: 3 of 7 files reviewed at latest revision, 11 unresolved discussions, some commit checks failed.

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 79 [r7] (raw file):

Previously, mbreakey3 (Mariah) wrote…

If this is going to be declared up here it can just be se to $itemValue = '' (lowercase since a local variable)

Done.

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 253 [r7] (raw file):

Previously, mbreakey3 (Mariah) wrote…

Could you add some comments for what state each of these nested blocks represent? It's a little hard to follow

Done.

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 277 [r7] (raw file):

Previously, mbreakey3 (Mariah) wrote…

lowercase 'return'

Done.

---

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 5 [r6] (raw file):

Previously, mbreakey3 (Mariah) wrote…

Still a few more comments - like the # Example on the lines below

Done.

---

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 72 [r6] (raw file):

Previously, mbreakey3 (Mariah) wrote…

This line starting with Mock should also be indented 4 spaces

Done.

---

Tests/Unit/MSFT_xSQLAOGroupEnsure.Tests.ps1, line 5 [r7] (raw file):

Previously, mbreakey3 (Mariah) wrote…

Get rid of the '# Example's

Done.

---

Tests/Unit/MSFT_xSQLAOGroupEnsure.Tests.ps1, line 33 [r7] (raw file):

Previously, mbreakey3 (Mariah) wrote…

All of these semicolons in this file can be removed too

Done.

---

Tests/Unit/MSFT_xSQLAOGroupEnsure.Tests.ps1, line 299 [r7] (raw file):

Previously, mbreakey3 (Mariah) wrote…

Make sure all TODOs are deleted too

Done.

---

Comments from Reviewable

[kwirkykat]

@aultt Just tested dev locally and all tests are passing:

Going to test your branch locally now to see what's up.

[kwirkykat]

@aultt Your branch is passing as well:

Gonna start messing with appveyor...

[aultt]

Ok let me know if there is anything I need to do.

On Tue, Aug 9, 2016 at 4:37 PM -0400, "Katie Keim" <[email protected]mailto:[email protected]> wrote:

@aultthttps://github.com/aultt Your branch is passing as well:
[image]https://cloud.githubusercontent.com/assets/6372220/17532615/58ec72a8-5e36-11e6-99ad-853ccfc0cdec.png

Gonna start messing with appveyor...

You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com//pull/89#issuecomment-238683054, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AMl96DG5A5wlKoz_s_dVjYAQ8YTWtAgjks5qeOT3gaJpZM4JYNuo.

[kwirkykat]

@aultt Ok. I think I know what's going on here:

When I run these tests locally, I do not originally have the 'Microsoft.SqlServer.Management.Smo.Server' type defined. Because I don't have this type defined, the tests load a 'fake' Microsoft.SqlServer.Management.Smo.Server from the SMO.cs stubs file in Tests\Unit. This definition of the type has a constructor that takes one boolean argument. The mocks in the Pester tests use this constructor with the one boolean.

When these tests run on AppVeyor, however, the actual type Microsoft.SqlServer.Management.Smo.Server is already defined. Because the actual type is already defined, the Pester tests cannot redefine the type with the mocked type. The actual Microsoft.SqlServer.Management.Smo.Server does not have a constructor that takes one boolean as an argument. So, when AppVeyor tries to run the tests with the constrcutor with one boolean, the tests fail because the type defined on that machine does not have a constructor that takes those arguments.

So how to solve this?
I would say the fastest option right now would be to replace the calls to the constructor with one boolean in the tests with the constructor with 0 arguments since both the mocked type and the actual type have these constructors. @johlju already did this in #98, so if I were you I would just wait until that PR is merged, then pick up those changes.

[johlju]

Some code clean up. If you may :)

---

Reviewed 1 of 6 files at r1, 2 of 3 files at r8, 4 of 9 files at r12.
Review status: all files reviewed at latest revision, 27 unresolved discussions.

---

README.md, line 374 [r12] (raw file):

  - modified functions
      - New-TerminatingError - *added optional parameter `InnerException` to be able to give the user more information in the returned message*
* Pester Tests for xSQLAOGroupEnsure and xSQLAlias

Remove the test that is not in this PR.

---

README.md, line 376 [r12] (raw file):

* Pester Tests for xSQLAOGroupEnsure and xSQLAlias
* Correction on xSQLAOGroupEnsure to pass Setup Credential Correctly

Remove this blank line

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 11 [r12] (raw file):

    param
    (
        [parameter(Mandatory = $true)]

Parameter with capital P. Can you change all of the "parameter" is changed to "Parameter" in the rest of this file?

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 20 [r12] (raw file):

    $returnValue = @{
            Name = [System.String]

This hash table is indented one step to much I think.

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 26 [r12] (raw file):

            PipeName = [System.String]
            Ensure = [System.String]
        }

This { is indented to much too

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 164 [r12] (raw file):

        }
    }

Two blank lines here. Can your remove them?

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 167 [r12] (raw file):

}

One blank line to much here

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.schema.mof, line 2 [r12] (raw file):

[ClassVersion("1.0.0.0"), FriendlyName("xSQLAlias")]
class MSFT_xSQLAlias : OMI_BaseResource

Can you please add descriptions to all of these parameters in the schema file?

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.schema.mof, line 6 [r12] (raw file):

    [Key] String Name;
    [Write, ValueMap{"TCP","NP"}, Values{"TCP","NP"}] String Protocol;
    [Write] String ServerName;

Shouldn't server name be Key as well? Not sure if this is a scenario, but otherwise we cannot use the same name on more than one server? Just want to make sure.

---

DSCResources/MSFT_xSQLAOGroupEnsure/MSFT_xSQLAOGroupEnsure.psm1, line 135 [r12] (raw file):

            $ConnectionModeInSecondaryRole = switch ($ReadableSecondary)
                               {
                                   'None' {"AllowNoConnections"}

Could you add space to all of these so its { "AllowNoConnections" }. And for the rest below as well

---

DSCResources/MSFT_xSQLAOGroupEnsure/MSFT_xSQLAOGroupEnsure.psm1, line 153 [r12] (raw file):

                $availabilityGroup.HealthCheckTimeout = $HealthCheckTimeout
            }
           Catch

Could you clean up the brackets {} and code in in these try-catch blocks? They are not indented correctly. Please go thru all try-catch blocks. Many seems to be out of place, indenting wise.

---

DSCResources/MSFT_xSQLAOGroupEnsure/MSFT_xSQLAOGroupEnsure.psm1, line 261 [r12] (raw file):

               {
                Throw "Unable to Create $AvailabilityGroup on $SQLServer\$SQLInstanceName"
                Exit

Do you need Exit here when you are throwing?

---

DSCResources/MSFT_xSQLAOGroupEnsure/MSFT_xSQLAOGroupEnsure.psm1, line 277 [r12] (raw file):

        }
    }

Please remove the blank spaces here

---

DSCResources/MSFT_xSQLAOGroupEnsure/MSFT_xSQLAOGroupEnsure.psm1, line 344 [r12] (raw file):

        "Present"
            {

Blank line here, and indenting seems wrong too.

---

Comments from Reviewable

[aultt]

Yea merging an old one in removed the AOGroupEnsure test will have to add to stub later.

---

Comments from Reviewable

[aultt]

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.schema.mof, line 6 [r12] (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Shouldn't server name be Key as well? Not sure if this is a scenario, but otherwise we cannot use the same name on more than one server? Just want to make sure.

Yes I would agree.

---

Comments from Reviewable

[aultt]

DSCResources/MSFT_xSQLAOGroupEnsure/MSFT_xSQLAOGroupEnsure.psm1, line 277 [r12] (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Please remove the blank spaces here

Done

---

Comments from Reviewable

[aultt]

Review status: 2 of 6 files reviewed at latest revision, 27 unresolved discussions, some commit checks failed.

---

README.md, line 374 [r12] (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Remove the test that is not in this PR.

Done.

---

README.md, line 376 [r12] (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Remove this blank line

Done.

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 11 [r12] (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Parameter with capital P. Can you change all of the "parameter" is changed to "Parameter" in the rest of this file?

Done.

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 20 [r12] (raw file):

Previously, johlju (Johan Ljunggren) wrote…

This hash table is indented one step to much I think.

Done.

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 26 [r12] (raw file):

Previously, johlju (Johan Ljunggren) wrote…

This { is indented to much too

Done.

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 164 [r12] (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Two blank lines here. Can your remove them?

Done.

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 167 [r12] (raw file):

Previously, johlju (Johan Ljunggren) wrote…

One blank line to much here

Done.

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.schema.mof, line 2 [r12] (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Can you please add descriptions to all of these parameters in the schema file?

Done.

---

DSCResources/MSFT_xSQLAOGroupEnsure/MSFT_xSQLAOGroupEnsure.psm1, line 135 [r12] (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Could you add space to all of these so its { "AllowNoConnections" }. And for the rest below as well

Done.

---

DSCResources/MSFT_xSQLAOGroupEnsure/MSFT_xSQLAOGroupEnsure.psm1, line 153 [r12] (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Could you clean up the brackets {} and code in in these try-catch blocks? They are not indented correctly. Please go thru all try-catch blocks. Many seems to be out of place, indenting wise.

Done.

---

DSCResources/MSFT_xSQLAOGroupEnsure/MSFT_xSQLAOGroupEnsure.psm1, line 261 [r12] (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Do you need Exit here when you are throwing?

Done.

---

DSCResources/MSFT_xSQLAOGroupEnsure/MSFT_xSQLAOGroupEnsure.psm1, line 344 [r12] (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Blank line here, and indenting seems wrong too.

Done.

---

Comments from Reviewable

[aultt]

@johlju I believe I have addressed all the items you requested @mbreakey3 can you review

[mbreakey3]

Reviewed 1 of 4 files at r3, 2 of 9 files at r12, 1 of 5 files at r13.
Review status: 2 of 6 files reviewed at latest revision, 50 unresolved discussions.

---

README.md, line 329 [r13] (raw file):

 * **Protocol**: The protocol of either tcp or np (named pipes).
 * **RetryCount**: Maximum number of retries to check HA group existency.
 * **TCPPort**: The tcp port of the instance.

'The TCP port'

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 33 [r13] (raw file):

    }

    if ($null -ne (Get-ItemProperty -Path 'HKLM:\SOFTWARE\Microsoft\MSSQLServer\Client\ConnectTo' -Name "$Name" -ErrorAction SilentlyContinue))

Does $Name need to be in quotes here: -Name "$Name" ?

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 101 [r13] (raw file):

    if ($Ensure -eq 'Present')
    {
        If($PSCmdlet.ShouldProcess("'$Name'","Replace the Client Alias"))

if (...) lowercase if and add space before parenthesis

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 108 [r13] (raw file):

            {
                Write-Debug -Message 'Check if value requires changing'
                $CurrentValue = Get-ItemProperty -Path 'HKLM:\SOFTWARE\Microsoft\MSSQLServer\Client\ConnectTo' -Name "$Name"

$currentValue - lowercase

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 124 [r13] (raw file):

            Write-Debug -Message 'Check OSArchitecture'
            #If this is a 64 bit machine also update Wow6432Node

If

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 125 [r13] (raw file):

            Write-Debug -Message 'Check OSArchitecture'
            #If this is a 64 bit machine also update Wow6432Node
            if ((Get-Wmiobject -class win32_OperatingSystem).OSArchitecture -eq '64-bit')

Get-WmiObject .
Alos, capitalize '-Class'

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 148 [r13] (raw file):

        }
    }

delete one of these newlines

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 153 [r13] (raw file):

    if ($Ensure -eq 'Absent')
    {
        If($PSCmdlet.ShouldProcess("'$Name'","Remove the Client Alias (if exists)"))

'if (....)

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 155 [r13] (raw file):

        If($PSCmdlet.ShouldProcess("'$Name'","Remove the Client Alias (if exists)"))
        {
            #If the base path doesn't exist then we don't need to do anything

space after #

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 162 [r13] (raw file):

                Write-Debug -Message 'Check OSArchitecture'
                #If this is a 64 bit machine also update Wow6432Node

space after #

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 163 [r13] (raw file):

                Write-Debug -Message 'Check OSArchitecture'
                #If this is a 64 bit machine also update Wow6432Node
                if ((Get-Wmiobject -class win32_OperatingSystem).OSArchitecture -eq '64-bit' -and (Test-Path -Path 'HKLM:\SOFTWARE\Wow6432Node\Microsoft\MSSQLServer\Client\ConnectTo'))

Get-WmiObject -Class

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 203 [r13] (raw file):

    Write-Debug -Message 'Test-TargetResource'

    $result = [System.Boolean]$true

space after [System.Boolean]

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 213 [r13] (raw file):

            if ($Ensure -eq 'Present')
            {
                $ItemValue = Get-ItemProperty -Path 'HKLM:\SOFTWARE\Microsoft\MSSQLServer\Client\ConnectTo' -Name "$Name"

lowercase - $iremValue

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 215 [r13] (raw file):

                $ItemValue = Get-ItemProperty -Path 'HKLM:\SOFTWARE\Microsoft\MSSQLServer\Client\ConnectTo' -Name "$Name"

                $ItemConfig = $ItemValue."$Name" -split ','

lowercase $itemConfig

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 232 [r13] (raw file):

                }

                #If this is a 64 bit machine also check Wow6432Node

space after # (and all following comments as well

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 233 [r13] (raw file):

                #If this is a 64 bit machine also check Wow6432Node
                if ((Get-Wmiobject -class win32_OperatingSystem).OSArchitecture -eq '64-bit')

Get-WmiObject -Class

---

DSCResources/MSFT_xSQLAOGroupEnsure/MSFT_xSQLAOGroupEnsure.psm1, line 261 [r12] (raw file):

Previously, aultt (Troy Ault) wrote…

Done.

Does that mean you do need the Exit?

---

DSCResources/MSFT_xSQLAOGroupEnsure/MSFT_xSQLAOGroupEnsure.psm1, line 249 [r13] (raw file):

            }

            #Create Availability Group

spaces after all '#'

---

DSCResources/MSFT_xSQLAOGroupEnsure/MSFT_xSQLAOGroupEnsure.psm1, line 250 [r13] (raw file):

            #Create Availability Group
            Try{

All brackets should be on their own line (except for some cases in pester tests)

---

DSCResources/MSFT_xSQLAOGroupEnsure/MSFT_xSQLAOGroupEnsure.psm1, line 260 [r13] (raw file):

        }
        "Absent"

this should be in single quotes since there's no variables within it: 'Absent'

---

DSCResources/MSFT_xSQLAOGroupEnsure/MSFT_xSQLAOGroupEnsure.psm1, line 335 [r13] (raw file):

    Switch ($Ensure)
    {
        "Present"

'Present'

---

DSCResources/MSFT_xSQLAOGroupEnsure/MSFT_xSQLAOGroupEnsure.psm1, line 339 [r13] (raw file):

            $AGPresent=$sql.AvailabilityGroups.Contains($AvailabilityGroupName)
            if ($AGPresent)
                {$Return = $true}

Instead of using $Return. Declare a local variable called something like $isInDesiredState or something and declare it at the top of the function and set it to $true. Then change it's value to $false if needed.

---

DSCResources/MSFT_xSQLAOGroupEnsure/MSFT_xSQLAOGroupEnsure.psm1, line 343 [r13] (raw file):

                {$Return = $false}
        }
        "Absent"

'Absent'

---

DSCResources/MSFT_xSQLAOGroupEnsure/MSFT_xSQLAOGroupEnsure.psm1, line 345 [r13] (raw file):

        "Absent"
        {
            if(!$sql.AvailabilityGroups[$AvailabilityGroupName])

space between 'if' and '('

---

DSCResources/MSFT_xSQLAOGroupEnsure/MSFT_xSQLAOGroupEnsure.psm1, line 346 [r13] (raw file):

        {
            if(!$sql.AvailabilityGroups[$AvailabilityGroupName])
            {$Return = $true}

all of these brackets should be on their own line

---

DSCResources/MSFT_xSQLAOGroupEnsure/MSFT_xSQLAOGroupEnsure.psm1, line 351 [r13] (raw file):

    }
    $Return

delete the extra newlines here

---

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 31 [r13] (raw file):

    $SqlAlias = Get-TargetResource -Name 'localhost' -Servername 'localhost'

    It 'Should return hashtable with Key Protocol'{

All of the It statements should be within a Describe block

---

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 42 [r13] (raw file):

    #region Set-TargetResource
    Describe 'Set-TargetResource'{

These Describe blocks should encompass all of the It statements for this test group

---

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 45 [r13] (raw file):

        Mock -ModuleName MSFT_xSqlAlias -CommandName Test-Path -MockWith {
        Write-Output $true

Is there a reason you used Write-Output instead of return?

---

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 56 [r13] (raw file):

    }    

        Mock -ModuleName MSFT_xSqlAlias -CommandName Get-Wmiobject -MockWith {

Get-WmiObject

---

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 68 [r13] (raw file):

    }

    It 'Call Set-ItemProperty exactly 2 times (1 for 32bit and 1 for 64 bit reg keys)' {

It 'Should call...'

---

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 83 [r13] (raw file):

        Mock -ModuleName MSFT_xSqlAlias -CommandName Get-ItemProperty -MockWith {
        Write-Output @{

Indent these 3 lines below 'Mock'

---

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 95 [r13] (raw file):

    }

    It 'Should return true when Test is passed as Alias thats already set'{

'Should return '$true...'

---

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 99 [r13] (raw file):

    }

    It 'Should return false when Test is passed as Alias that is not set'{

'Should return $false'

---

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 105 [r13] (raw file):

    #end region Test-TargetResource

    InModuleScope $script:DSCResourceName {

delete this block

---

Comments from Reviewable

[johlju]

Reviewed 4 of 5 files at r13.
Review status: 5 of 6 files reviewed at latest revision, 54 unresolved discussions.

---

README.md, line 374 [r12] (raw file):

Previously, aultt (Troy Ault) wrote…

Done.

LGTM

---

README.md, line 376 [r12] (raw file):

Previously, aultt (Troy Ault) wrote…

Done.

LGTM

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 11 [r12] (raw file):

Previously, aultt (Troy Ault) wrote…

Done.

LGTM

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 20 [r12] (raw file):

Previously, aultt (Troy Ault) wrote…

Done.

LGTM

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 26 [r12] (raw file):

Previously, aultt (Troy Ault) wrote…

Done.

LGTM

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 164 [r12] (raw file):

Previously, aultt (Troy Ault) wrote…

Done.

LGTM

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 167 [r12] (raw file):

Previously, aultt (Troy Ault) wrote…

Done.

LGTM

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.schema.mof, line 2 [r12] (raw file):

Previously, aultt (Troy Ault) wrote…

Done.

LGTM

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.schema.mof, line 6 [r12] (raw file):

Previously, aultt (Troy Ault) wrote…

Yes I would agree.

LGTM

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.schema.mof, line 5 [r13] (raw file):

{
    [Key, Description("The SQL Server Alias name.")] String Name;
    [Write, Description("Protocal to use when connecting."), ValueMap{"TCP","NP"}, Values{"TCP","NP"}] String Protocol;

Typo in Protocol here

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.schema.mof, line 7 [r13] (raw file):

    [Write, Description("Protocal to use when connecting."), ValueMap{"TCP","NP"}, Values{"TCP","NP"}] String Protocol;
    [Key, Description("The SQL Server you are aliasing.")] String ServerName;
    [Write, Description("The TCP port SQL is listening on")] Sint32 TCPPort;

End sentence with . (full stop)

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.schema.mof, line 8 [r13] (raw file):

    [Key, Description("The SQL Server you are aliasing.")] String ServerName;
    [Write, Description("The TCP port SQL is listening on")] Sint32 TCPPort;
    [Read, Description("Named Pipes Name ")] String PipeName;

Whitespace in the end of the string. And end sentence with . (full stop)

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.schema.mof, line 9 [r13] (raw file):

    [Write, Description("The TCP port SQL is listening on")] Sint32 TCPPort;
    [Read, Description("Named Pipes Name ")] String PipeName;
    [Write, Description("Determines if Alias be Present or Absent."), ValueMap{"Present","Absent"}, Values{"Present","Absent"}] String Ensure;

"if alias is to be" might be better ? (English is my second language so don't trust me completely ;)

---

DSCResources/MSFT_xSQLAOGroupEnsure/MSFT_xSQLAOGroupEnsure.psm1, line 135 [r12] (raw file):

Previously, aultt (Troy Ault) wrote…

Done.

Could you also do it so all of them has space in between the brackets? Like `'None' { "AllowNoConnections" }`

---

DSCResources/MSFT_xSQLAOGroupEnsure/MSFT_xSQLAOGroupEnsure.psm1, line 153 [r12] (raw file):

Previously, aultt (Troy Ault) wrote…

Done.

LGTM

---

DSCResources/MSFT_xSQLAOGroupEnsure/MSFT_xSQLAOGroupEnsure.psm1, line 261 [r12] (raw file):

Previously, mbreakey3 (Mariah) wrote…

Does that mean you do need the Exit?

If you don't need the `Exit` after throw, then please remove `Exit` from a few other places in the code :)

---

DSCResources/MSFT_xSQLAOGroupEnsure/MSFT_xSQLAOGroupEnsure.psm1, line 277 [r12] (raw file):

Previously, aultt (Troy Ault) wrote…

Done

There are still blank spaces here :/ You missed some of them :)

---

DSCResources/MSFT_xSQLAOGroupEnsure/MSFT_xSQLAOGroupEnsure.psm1, line 344 [r12] (raw file):

Previously, aultt (Troy Ault) wrote…

Done.

LGTM

---

Tests/Unit/MSFT_xSQLAOGroupEnsure.Tests.ps1, line 137 [r11] (raw file):

Previously, aultt (Troy Ault) wrote…

Removed for now will add in follow up PR

LGTM

---

Tests/Unit/MSFT_xSQLAOGroupEnsure.Tests.ps1, line 150 [r11] (raw file):

Previously, aultt (Troy Ault) wrote…

Removed for now will add in follow up PR> Removed for now will add in follow up PR

LGTM

---

Comments from Reviewable

[aultt]

Closing PR as I don't have additional time to allocate

[aultt]

Closing as I don't have additional time to allocate

[johlju]

@aultt If you don't have the time to continue. I'm would gladly continue with this PR. Is it possible to transfer the fork/PR over to me some way? @mbreakey3 Do you know if it's possible?

[kwirkykat]

@johlju The easiest way to pull his changes over to his fork is to just create a PR with the branch in your fork you want to merge these changes into as the base and aultt's dev branch as the head.

Like this:

You could also do it with git rebasing if you want to set up his dev branch as a remote to your fork and then rebase one of your branches onto his dev branch.

git fetch aultt(or whatever you name your remote, 'upstream' in the GitHub sync fork instructions)
git rebase aultt/dev

[johlju]

@mbreakey3 Then we have a new PR for this one in PR #114
@aultt Thanks for all your work! Hope to see you around soon again! 😄

[aultt]

Thanks @johlju I'll still be here just have to focus on some other things for now.

[johlju]

Do not review on this PR further. All these comments are fixed in PR #114, continue the review process there.

---

Review status: 5 of 6 files reviewed at latest revision, 41 unresolved discussions, some commit checks failed.

---

README.md, line 329 [r13] (raw file):

Previously, mbreakey3 (Mariah) wrote…

'The TCP port'

Done

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 33 [r13] (raw file):

Previously, mbreakey3 (Mariah) wrote…

Does $Name need to be in quotes here: -Name "$Name" ?

Fixed. Remove the quotes.

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 101 [r13] (raw file):

Previously, mbreakey3 (Mariah) wrote…

if (...) lowercase if and add space before parenthesis

Done

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 108 [r13] (raw file):

Previously, mbreakey3 (Mariah) wrote…

$currentValue - lowercase

Done. Fixed all these throughout

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 124 [r13] (raw file):

Previously, mbreakey3 (Mariah) wrote…

If

Done

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 125 [r13] (raw file):

Previously, mbreakey3 (Mariah) wrote…

Get-WmiObject .
Alos, capitalize '-Class'

Done, throughout

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 148 [r13] (raw file):

Previously, mbreakey3 (Mariah) wrote…

delete one of these newlines

Done

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 153 [r13] (raw file):

Previously, mbreakey3 (Mariah) wrote…

'if (....)

Done

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 155 [r13] (raw file):

Previously, mbreakey3 (Mariah) wrote…

space after #

Done. All comments fixed.

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 162 [r13] (raw file):

Previously, mbreakey3 (Mariah) wrote…

space after #

Done

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 163 [r13] (raw file):

Previously, mbreakey3 (Mariah) wrote…

Get-WmiObject -Class

Done

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 203 [r13] (raw file):

Previously, mbreakey3 (Mariah) wrote…

space after [System.Boolean]

Done

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 213 [r13] (raw file):

Previously, mbreakey3 (Mariah) wrote…

lowercase - $iremValue

Done

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 215 [r13] (raw file):

Previously, mbreakey3 (Mariah) wrote…

lowercase $itemConfig

Done

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 232 [r13] (raw file):

Previously, mbreakey3 (Mariah) wrote…

space after # (and all following comments as well

Done

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 233 [r13] (raw file):

Previously, mbreakey3 (Mariah) wrote…

Get-WmiObject -Class

Done

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.schema.mof, line 5 [r13] (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Typo in Protocol here

Done

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.schema.mof, line 7 [r13] (raw file):

Previously, johlju (Johan Ljunggren) wrote…

End sentence with . (full stop)

Done

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.schema.mof, line 8 [r13] (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Whitespace in the end of the string. And end sentence with . (full stop)

Done

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.schema.mof, line 9 [r13] (raw file):

Previously, johlju (Johan Ljunggren) wrote…

"if alias is to be" might be better ? (English is my second language so don't trust me completely ;)

Done

---

DSCResources/MSFT_xSQLAOGroupEnsure/MSFT_xSQLAOGroupEnsure.psm1, line 135 [r12] (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Could you also do it so all of them has space in between the brackets? Like 'None' { "AllowNoConnections" }

Done

---

DSCResources/MSFT_xSQLAOGroupEnsure/MSFT_xSQLAOGroupEnsure.psm1, line 261 [r12] (raw file):

Previously, johlju (Johan Ljunggren) wrote…

If you don't need the Exit after throw, then please remove Exit from a few other places in the code :)

Removed Exit because we are throwing which we stop the call to the method.

---

DSCResources/MSFT_xSQLAOGroupEnsure/MSFT_xSQLAOGroupEnsure.psm1, line 277 [r12] (raw file):

Previously, johlju (Johan Ljunggren) wrote…

There are still blank spaces here :/ You missed some of them :)

Done

---

DSCResources/MSFT_xSQLAOGroupEnsure/MSFT_xSQLAOGroupEnsure.psm1, line 249 [r13] (raw file):

Previously, mbreakey3 (Mariah) wrote…

spaces after all '#'

Done

---

DSCResources/MSFT_xSQLAOGroupEnsure/MSFT_xSQLAOGroupEnsure.psm1, line 250 [r13] (raw file):

Previously, mbreakey3 (Mariah) wrote…

All brackets should be on their own line (except for some cases in pester tests)

Done

---

DSCResources/MSFT_xSQLAOGroupEnsure/MSFT_xSQLAOGroupEnsure.psm1, line 260 [r13] (raw file):

Previously, mbreakey3 (Mariah) wrote…

this should be in single quotes since there's no variables within it: 'Absent'

Done throughout the code

---

DSCResources/MSFT_xSQLAOGroupEnsure/MSFT_xSQLAOGroupEnsure.psm1, line 335 [r13] (raw file):

Previously, mbreakey3 (Mariah) wrote…

'Present'

Done

---

DSCResources/MSFT_xSQLAOGroupEnsure/MSFT_xSQLAOGroupEnsure.psm1, line 339 [r13] (raw file):

Previously, mbreakey3 (Mariah) wrote…

Instead of using $Return. Declare a local variable called something like $isInDesiredState or something and declare it at the top of the function and set it to $true. Then change it's value to $false if needed.

Done

---

DSCResources/MSFT_xSQLAOGroupEnsure/MSFT_xSQLAOGroupEnsure.psm1, line 343 [r13] (raw file):

Previously, mbreakey3 (Mariah) wrote…

'Absent'

Done

---

DSCResources/MSFT_xSQLAOGroupEnsure/MSFT_xSQLAOGroupEnsure.psm1, line 345 [r13] (raw file):

Previously, mbreakey3 (Mariah) wrote…

space between 'if' and '('

Done

---

DSCResources/MSFT_xSQLAOGroupEnsure/MSFT_xSQLAOGroupEnsure.psm1, line 346 [r13] (raw file):

Previously, mbreakey3 (Mariah) wrote…

all of these brackets should be on their own line

Done

---

DSCResources/MSFT_xSQLAOGroupEnsure/MSFT_xSQLAOGroupEnsure.psm1, line 351 [r13] (raw file):

Previously, mbreakey3 (Mariah) wrote…

delete the extra newlines here

Done

---

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 31 [r13] (raw file):

Previously, mbreakey3 (Mariah) wrote…

All of the It statements should be within a Describe block

They were, just indented wrongly so I also read they were outside of the describe block. Fixed now.

---

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 42 [r13] (raw file):

Previously, mbreakey3 (Mariah) wrote…

These Describe blocks should encompass all of the It statements for this test group

Same here. They were

---

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 45 [r13] (raw file):

Previously, mbreakey3 (Mariah) wrote…

Is there a reason you used Write-Output instead of return?

Fixed. Using return instead.

---

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 56 [r13] (raw file):

Previously, mbreakey3 (Mariah) wrote…

Get-WmiObject

Done

---

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 68 [r13] (raw file):

Previously, mbreakey3 (Mariah) wrote…

It 'Should call...'

Done

---

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 83 [r13] (raw file):

Previously, mbreakey3 (Mariah) wrote…

Indent these 3 lines below 'Mock'

Done

---

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 95 [r13] (raw file):

Previously, mbreakey3 (Mariah) wrote…

'Should return '$true...'

Done

---

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 99 [r13] (raw file):

Previously, mbreakey3 (Mariah) wrote…

'Should return $false'

Done

---

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 105 [r13] (raw file):

Previously, mbreakey3 (Mariah) wrote…

delete this block

Done

---

Comments from Reviewable

[johlju]

@aultt That test you removed temporarily from this PR (because of the strange errors that was happening with it). Do you still have it? I would be awesome if you still have it, then I can incorporate that into the new PR and one more test is done. :)

[johlju]

@aultt I found the test in an earlier commit. I fixed the test and added it to the new PR #114. All good now. :)