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

xVMSwitch - Fixes #107 and adds comment based help and updates unit tests to latest template version #109

Merged
merged 8 commits into from
Jul 1, 2017

Conversation

bgelens
Copy link
Contributor

@bgelens bgelens commented Jun 30, 2017

Fixes #108
Fixes: #107


This change is Reviewable

@bgelens
Copy link
Contributor Author

bgelens commented Jun 30, 2017

@PlagueHO could you review this PR please. Thanks!

@bgelens bgelens added the needs review The pull request needs a code review. label Jun 30, 2017
@bgelens bgelens changed the title xVMSwitch - Fixed #107 and added comment based help and updated unit test template version xVMSwitch - Fixes #107 and adds comment based help and updates unit tests to latest template version Jun 30, 2017
@PlagueHO
Copy link
Member

Hi @bgelens - great job! Just some minor style nitpicks in the tests - nothing that the correct settings in VS Code + applying auto formatting couldn't fix 😁


Reviewed 3 of 3 files at r1.
Review status: all files reviewed at latest revision, 17 unresolved discussions.


Tests/Unit/MSFT_xVMSwitch_BandwidthReservationMode.Tests.ps1, line 6 at r1 (raw file):

$script:moduleRoot = Split-Path -Parent (Split-Path -Parent $PSScriptRoot)
if ( (-not (Test-Path -Path (Join-Path -Path $script:moduleRoot -ChildPath 'DSCResource.Tests'))) -or `
    (-not (Test-Path -Path (Join-Path -Path $script:moduleRoot -ChildPath 'DSCResource.Tests\TestHelper.psm1'))) ) {

Nitpick: { Should be on next line - https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#one-newline-after-opening-brace


Tests/Unit/MSFT_xVMSwitch_BandwidthReservationMode.Tests.ps1, line 19 at r1 (raw file):

#endregion HEADER

function Invoke-TestSetup {

Nitpick: { Should be on next line - https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#one-newline-after-opening-brace


Tests/Unit/MSFT_xVMSwitch_BandwidthReservationMode.Tests.ps1, line 22 at r1 (raw file):

}

function Invoke-TestCleanup {

Nitpick: { Should be on next line - https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#one-newline-after-opening-brace


Tests/Unit/MSFT_xVMSwitch_BandwidthReservationMode.Tests.ps1, line 27 at r1 (raw file):

# Begin Testing
try {

Nitpick: { Should be on next line - https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#one-newline-after-opening-brace


Tests/Unit/MSFT_xVMSwitch_BandwidthReservationMode.Tests.ps1, line 89 at r1 (raw file):

            # Create an empty function to be able to mock the missing Hyper-V cmdlet
            function Get-VMSwitch { }

Nitpick: { Should be on next line - https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#one-newline-after-opening-brace


Tests/Unit/MSFT_xVMSwitch_BandwidthReservationMode.Tests.ps1, line 198 at r1 (raw file):

        Describe 'Validates Test-TargetResource Function' {

Nitpick: Should be no newline here - https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#one-newline-after-opening-brace


Tests/Unit/MSFT_xVMSwitch_BandwidthReservationMode.Tests.ps1, line 300 at r1 (raw file):

            function Set-VMSwitch { }

            # Mocks Get-VMSwitch and will return $global:mockedVMSwitch which is

Should be <# #> style for multi line comments


Tests/Unit/MSFT_xVMSwitch_BandwidthReservationMode.Tests.ps1, line 309 at r1 (raw file):

                )

                if ($ErrorAction -eq 'Stop' -and $global:mockedVMSwitch -eq $null) {

Nitpick: { Should be on next line - https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#one-newline-after-opening-brace


Tests/Unit/MSFT_xVMSwitch_BandwidthReservationMode.Tests.ps1, line 337 at r1 (raw file):

                )

                if ($AllowManagementOS) {

Nitpick: { Should be on next line - https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#one-newline-after-opening-brace


Tests/Unit/MSFT_xVMSwitch_BandwidthReservationMode.Tests.ps1, line 367 at r1 (raw file):

            # Create all the test cases for Get-TargetResource
            $getTestCases = @()
            foreach ($brmMode in $BANDWIDTH_RESERVATION_MODES) {

Nitpick: { Should be on next line - https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#one-newline-after-opening-brace


Tests/Unit/MSFT_xVMSwitch_EnableEmbeddedTeaming.Tests.ps1, line 6 at r1 (raw file):

$script:moduleRoot = Split-Path -Parent (Split-Path -Parent $PSScriptRoot)
if ( (-not (Test-Path -Path (Join-Path -Path $script:moduleRoot -ChildPath 'DSCResource.Tests'))) -or `
    (-not (Test-Path -Path (Join-Path -Path $script:moduleRoot -ChildPath 'DSCResource.Tests\TestHelper.psm1'))) ) {

Nitpick: { Should be on next line - https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#one-newline-after-opening-brace


Tests/Unit/MSFT_xVMSwitch_EnableEmbeddedTeaming.Tests.ps1, line 19 at r1 (raw file):

#endregion HEADER

function Invoke-TestSetup {

Nitpick: { Should be on next line - https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#one-newline-after-opening-brace


Tests/Unit/MSFT_xVMSwitch_EnableEmbeddedTeaming.Tests.ps1, line 22 at r1 (raw file):

}

function Invoke-TestCleanup {

Nitpick: { Should be on next line - https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#one-newline-after-opening-brace


Tests/Unit/MSFT_xVMSwitch_EnableEmbeddedTeaming.Tests.ps1, line 27 at r1 (raw file):

# Begin Testing
try {

Nitpick: { Should be on next line - https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#one-newline-after-opening-brace


Tests/Unit/MSFT_xVMSwitch_EnableEmbeddedTeaming.Tests.ps1, line 33 at r1 (raw file):

        # Function to create a exception object for testing output exceptions
        function Get-InvalidArgumentError {

Nitpick: { Should be on next line - https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#one-newline-after-opening-brace


Tests/Unit/MSFT_xVMSwitch_EnableEmbeddedTeaming.Tests.ps1, line 123 at r1 (raw file):

                )

                if ($ErrorAction -eq 'Stop' -and $global:mockedVMSwitch -eq $null) {

Nitpick: { Should be on next line - https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#one-newline-after-opening-brace


Tests/Unit/MSFT_xVMSwitch_EnableEmbeddedTeaming.Tests.ps1, line 413 at r1 (raw file):

        }
    }
} finally {

Nitpick: } and { Should be on next line - https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#one-newline-after-opening-brace


Comments from Reviewable

@bgelens
Copy link
Contributor Author

bgelens commented Jun 30, 2017

@PlagueHO All comments addressed plus some extra 😉

Reviewed 2 of 3 files at r1, 1 of 1 files at r2, 3 of 3 files at r3.
Review status: all files reviewed at latest revision, 17 unresolved discussions.


Tests/Unit/MSFT_xVMSwitch_BandwidthReservationMode.Tests.ps1, line 6 at r1 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Nitpick: { Should be on next line - https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#one-newline-after-opening-brace

Done


Tests/Unit/MSFT_xVMSwitch_BandwidthReservationMode.Tests.ps1, line 19 at r1 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Nitpick: { Should be on next line - https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#one-newline-after-opening-brace

Done,


Tests/Unit/MSFT_xVMSwitch_BandwidthReservationMode.Tests.ps1, line 22 at r1 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Nitpick: { Should be on next line - https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#one-newline-after-opening-brace

Done


Tests/Unit/MSFT_xVMSwitch_BandwidthReservationMode.Tests.ps1, line 27 at r1 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Nitpick: { Should be on next line - https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#one-newline-after-opening-brace

Done


Tests/Unit/MSFT_xVMSwitch_BandwidthReservationMode.Tests.ps1, line 89 at r1 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Nitpick: { Should be on next line - https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#one-newline-after-opening-brace

Done


Tests/Unit/MSFT_xVMSwitch_BandwidthReservationMode.Tests.ps1, line 198 at r1 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Nitpick: Should be no newline here - https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#one-newline-after-opening-brace

Can't do that with describe


Tests/Unit/MSFT_xVMSwitch_BandwidthReservationMode.Tests.ps1, line 300 at r1 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Should be <# #> style for multi line comments

Done


Tests/Unit/MSFT_xVMSwitch_BandwidthReservationMode.Tests.ps1, line 309 at r1 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Nitpick: { Should be on next line - https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#one-newline-after-opening-brace

Done


Tests/Unit/MSFT_xVMSwitch_BandwidthReservationMode.Tests.ps1, line 337 at r1 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Nitpick: { Should be on next line - https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#one-newline-after-opening-brace

Done


Tests/Unit/MSFT_xVMSwitch_BandwidthReservationMode.Tests.ps1, line 367 at r1 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Nitpick: { Should be on next line - https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#one-newline-after-opening-brace

Done


Tests/Unit/MSFT_xVMSwitch_EnableEmbeddedTeaming.Tests.ps1, line 6 at r1 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Nitpick: { Should be on next line - https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#one-newline-after-opening-brace

Done


Tests/Unit/MSFT_xVMSwitch_EnableEmbeddedTeaming.Tests.ps1, line 19 at r1 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Nitpick: { Should be on next line - https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#one-newline-after-opening-brace

Done


Tests/Unit/MSFT_xVMSwitch_EnableEmbeddedTeaming.Tests.ps1, line 22 at r1 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Nitpick: { Should be on next line - https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#one-newline-after-opening-brace

Done


Tests/Unit/MSFT_xVMSwitch_EnableEmbeddedTeaming.Tests.ps1, line 27 at r1 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Nitpick: { Should be on next line - https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#one-newline-after-opening-brace

Done


Tests/Unit/MSFT_xVMSwitch_EnableEmbeddedTeaming.Tests.ps1, line 33 at r1 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Nitpick: { Should be on next line - https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#one-newline-after-opening-brace

Done


Tests/Unit/MSFT_xVMSwitch_EnableEmbeddedTeaming.Tests.ps1, line 123 at r1 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Nitpick: { Should be on next line - https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#one-newline-after-opening-brace

Done


Tests/Unit/MSFT_xVMSwitch_EnableEmbeddedTeaming.Tests.ps1, line 413 at r1 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Nitpick: } and { Should be on next line - https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#one-newline-after-opening-brace

Done


Comments from Reviewable

@PlagueHO
Copy link
Member

PlagueHO commented Jul 1, 2017

Just a couple of missed ones. Sorry! But looking great.


Reviewed 3 of 3 files at r3.
Review status: all files reviewed at latest revision, 3 unresolved discussions.


Tests/Unit/MSFT_xVMSwitch_BandwidthReservationMode.Tests.ps1, line 198 at r1 (raw file):

Previously, bgelens (Ben Gelens) wrote…

Can't do that with describe

What I meant was that there should not be empty line here 😁 E.g.

Describe 'Validates Test-TargetResource Function' {
  # Create an empty function to be able to mock the missing Hyper-V cmdlet

Tests/Unit/MSFT_xVMSwitch_EnableEmbeddedTeaming.Tests.ps1, line 108 at r3 (raw file):

            }

            function Set-VMSwitch {

Missed this one.


Tests/Unit/MSFT_xVMSwitch_EnableEmbeddedTeaming.Tests.ps1, line 112 at r3 (raw file):

            }

            function Remove-VMSwitch {

Missed this one.


Comments from Reviewable

@bgelens
Copy link
Contributor Author

bgelens commented Jul 1, 2017

Thanks @PlagueHO! I think this is it 😄


Reviewed 1 of 3 files at r1, 2 of 2 files at r4.
Review status: all files reviewed at latest revision, 3 unresolved discussions.


Tests/Unit/MSFT_xVMSwitch_BandwidthReservationMode.Tests.ps1, line 198 at r1 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

What I meant was that there should not be empty line here 😁 E.g.

Describe 'Validates Test-TargetResource Function' {
  # Create an empty function to be able to mock the missing Hyper-V cmdlet

I see :) Done (and in other places as well)


Tests/Unit/MSFT_xVMSwitch_EnableEmbeddedTeaming.Tests.ps1, line 108 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Missed this one.

Done.


Tests/Unit/MSFT_xVMSwitch_EnableEmbeddedTeaming.Tests.ps1, line 112 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Missed this one.

Done.


Comments from Reviewable

@PlagueHO
Copy link
Member

PlagueHO commented Jul 1, 2017

Thanks @bgelens

:lgtm:


Review status: all files reviewed at latest revision, 1 unresolved discussion.


Comments from Reviewable

@bgelens bgelens merged commit fbca5c0 into dsccommunity:dev Jul 1, 2017
@joeyaiello joeyaiello removed the needs review The pull request needs a code review. label Jul 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants