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

ADReplicationSubnet: Add Integration Testing, Add Description #507

Merged
merged 33 commits into from
Sep 25, 2019
Merged

ADReplicationSubnet: Add Integration Testing, Add Description #507

merged 33 commits into from
Sep 25, 2019

Conversation

1800Zeta
Copy link
Contributor

@1800Zeta 1800Zeta commented Sep 19, 2019

Pull Request (PR) description

Adds integration testing ability. Adds the Description field to Subnets

This Pull Request (PR) fixes the following issues

Task list

  • Added an entry under the Unreleased section in the CHANGELOG.md.
    Entry should say what was changed, and how that affects users (if applicable).
  • Resource documentation added/updated in resource directory README.md.
  • Resource parameter descriptions added/updated in schema.mof
    and comment-based help.
  • Comment-based help added/updated.
  • Conceptual help topic added/updated (cultureFolder\about_ResourceName.help.txt).
  • 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 Reviewable

@1800Zeta 1800Zeta closed this Sep 19, 2019
@1800Zeta 1800Zeta reopened this Sep 20, 2019
@johlju johlju added needs review The pull request needs a code review. updated by author The pull request was last updated by the author. waiting for code fix A review left open comments, and the pull request is waiting for changes to be pushed by the author. and removed needs review The pull request needs a code review. labels Sep 21, 2019
@johlju johlju removed the updated by author The pull request was last updated by the author. label Sep 21, 2019
@codecov-io
Copy link

codecov-io commented Sep 21, 2019

Codecov Report

Merging #507 into dev will increase coverage by <1%.
The diff coverage is 93%.

Impacted file tree graph

@@         Coverage Diff          @@
##            dev   #507    +/-   ##
====================================
+ Coverage    98%    98%   +<1%     
====================================
  Files        23     23            
  Lines      3108   3114     +6     
  Branches     10     10            
====================================
+ Hits       3063   3070     +7     
+ Misses       35     34     -1     
  Partials     10     10

@1800Zeta 1800Zeta marked this pull request as ready for review September 23, 2019 06:25
@johlju johlju added needs review The pull request needs a code review. and removed waiting for code fix A review left open comments, and the pull request is waiting for changes to be pushed by the author. labels Sep 24, 2019
Copy link
Member

@johlju johlju left a comment

Choose a reason for hiding this comment

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

Awesome work! I will test the integration tests manually tomorrow too.

Reviewed 4 of 8 files at r6, 3 of 3 files at r9.
Reviewable status: 7 of 9 files reviewed, 4 unresolved discussions (waiting on @1800Zeta)


DSCResources/MSFT_ADReplicationSubnet/en-US/MSFT_ADReplicationSubnet.strings.psd1, line 8 at r9 (raw file):

SetReplicationSubnetDescription    = Set the replication subnet '{0}' description to '{1}'.

Add this string key to the bottom of the list and add the id (ADRS0010) to the string.


Tests/Unit/MSFT_ADReplicationSubnet.Tests.ps1, line 95 at r9 (raw file):

should

Should (upper-case 'S')


Tests/Unit/MSFT_ADReplicationSubnet.Tests.ps1, line 188 at r9 (raw file):

It 'Should return false for wrong Description' {

Can we also test that it returns true when the descriptions are equal?


Tests/Unit/MSFT_ADReplicationSubnet.Tests.ps1, line 315 at r9 (raw file):

fail

Maybe we shouldn't use the word fail here unless we testing it to fail?

@johlju johlju added waiting for code fix A review left open comments, and the pull request is waiting for changes to be pushed by the author. and removed needs review The pull request needs a code review. labels Sep 24, 2019
Copy link
Contributor Author

@1800Zeta 1800Zeta left a comment

Choose a reason for hiding this comment

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

Reviewable status: 7 of 9 files reviewed, 4 unresolved discussions (waiting on @johlju)


DSCResources/MSFT_ADReplicationSubnet/en-US/MSFT_ADReplicationSubnet.strings.psd1, line 8 at r9 (raw file):

Previously, johlju (Johan Ljunggren) wrote…
SetReplicationSubnetDescription    = Set the replication subnet '{0}' description to '{1}'.

Add this string key to the bottom of the list and add the id (ADRS0010) to the string.

Done.


Tests/Unit/MSFT_ADReplicationSubnet.Tests.ps1, line 95 at r9 (raw file):

Previously, johlju (Johan Ljunggren) wrote…
should

Should (upper-case 'S')

Done.


Tests/Unit/MSFT_ADReplicationSubnet.Tests.ps1, line 188 at r9 (raw file):

Previously, johlju (Johan Ljunggren) wrote…
It 'Should return false for wrong Description' {

Can we also test that it returns true when the descriptions are equal?

Done.


Tests/Unit/MSFT_ADReplicationSubnet.Tests.ps1, line 315 at r9 (raw file):

Previously, johlju (Johan Ljunggren) wrote…
fail

Maybe we shouldn't use the word fail here unless we testing it to fail?

Done.

Copy link
Member

@johlju johlju left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 8 files at r6, 2 of 2 files at r10.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @1800Zeta)


Tests/Integration/MSFT_ADReplicationSubnet.Integration.Tests.ps1, line 37 at r9 (raw file):

        }

        $configurationName = "$($script:dscResourceName)_CreateSubnet_Config"

Getting this error. I think we need to add the Site that is used for testing as a prerequisites test and then make a cleanup test at the bottom that removes the site? 🤔 What do you think?

Executing script .\Tests\Integration\MSFT_ADReplicationSubnet.Integration.Tests.ps1

  Describing MSFT_ADReplicationSubnet_Integration

    Context When using configuration MSFT_ADReplicationSubnet_CreateSubnet_Config
VERBOSE: Perform operation 'Invoke CimMethod' with following parameters, ''methodName' = SendConfigurationApply,'className' = MSFT_DSCLocalConfigurationManager,'namespaceName' = root/Microsoft/Windows/DesiredStateConfiguration'.
VERBOSE: An LCM method call arrived from computer DC01 with user sid S-1-5-21-530133819-3181352061-503517500-500.
VERBOSE: [DC01]: LCM:  [ Start  Set      ]
VERBOSE: [DC01]:                            [DSCEngine] Importing the module C:\source\ActiveDirectoryDsc\DscResources\MSFT_ADReplicationSubnet\MSFT_ADReplicationSubnet.psm1 in force mode.
VERBOSE: [DC01]: LCM:  [ Start  Resource ]  [[ADReplicationSubnet]LondonSubnet]
VERBOSE: [DC01]: LCM:  [ Start  Test     ]  [[ADReplicationSubnet]LondonSubnet]
VERBOSE: [DC01]:                            [[ADReplicationSubnet]LondonSubnet] Importing the module MSFT_ADReplicationSubnet in force mode.
VERBOSE: [DC01]:                            [[ADReplicationSubnet]LondonSubnet] Getting replication subnet '10.0.0.0/24'. (ADRS0003)
VERBOSE: [DC01]:                            [[ADReplicationSubnet]LondonSubnet] Replication subnet '10.0.0.0/24' is absent. (ADRS0006)
VERBOSE: [DC01]:                            [[ADReplicationSubnet]LondonSubnet] The replication subnet '10.0.0.0/24' is not in the desired state. (ADRS0009)
VERBOSE: [DC01]: LCM:  [ End    Test     ]  [[ADReplicationSubnet]LondonSubnet]  in 0.2190 seconds.
VERBOSE: [DC01]: LCM:  [ Start  Set      ]  [[ADReplicationSubnet]LondonSubnet]
VERBOSE: [DC01]:                            [[ADReplicationSubnet]LondonSubnet] Importing the module MSFT_ADReplicationSubnet in force mode.
VERBOSE: [DC01]:                            [[ADReplicationSubnet]LondonSubnet] Create the replication subnet '10.0.0.0/24'. (ADRS0001)
      [-] Should compile and apply the MOF without throwing 1.68s
        Expected no exception to be thrown, but an exception "Identity info provided in the extended attribute: 'Site' could not be resolved. Reason: 'Cannot find an object with identity: 'London' under: 'CN=Sites,CN=Configuration,DC=contoso,DC=com'.'." was thrown from C:\sou
rce\ActiveDirectoryDsc\Tests\Integration\MSFT_ADReplicationSubnet.Integration.Tests.ps1:59 char:21
            + ...               Start-DscConfiguration @startDscConfigurationParameters
            +                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~.
        60:                 } | Should -Not -Throw
        at <ScriptBlock>, C:\source\ActiveDirectoryDsc\Tests\Integration\MSFT_ADReplicationSubnet.Integration.Tests.ps1: line 41
VERBOSE: An LCM method call arrived from computer DC01 with user sid S-1-5-21-530133819-3181352061-503517500-500.
WARNING: [DC01]:                            [] The GET operation will be carried against a pending configuration since the latest configuration has not converged yet.
VERBOSE: [DC01]:                            [DSCEngine] Importing the module C:\source\ActiveDirectoryDsc\DscResources\MSFT_ADReplicationSubnet\MSFT_ADReplicationSubnet.psm1 in force mode.
VERBOSE: [DC01]: LCM:  [ Start  Get      ]
VERBOSE: [DC01]:                            [[ADReplicationSubnet]LondonSubnet] Importing the module MSFT_ADReplicationSubnet in force mode.
VERBOSE: [DC01]:                            [[ADReplicationSubnet]LondonSubnet] Getting replication subnet '10.0.0.0/24'. (ADRS0003)
VERBOSE: [DC01]:                            [[ADReplicationSubnet]LondonSubnet] Replication subnet '10.0.0.0/24' is absent. (ADRS0006)
VERBOSE: [DC01]: LCM:  [ End    Get      ]  [[ADReplicationSubnet]LondonSubnet]  in 0.2820 seconds.
VERBOSE: [DC01]: LCM:  [ End    Get      ]    in  0.7200 seconds.
      [+] Should be able to call Get-DscConfiguration without throwing 1.24s
      [+] Should have set the resource and all the parameters should match 7ms
VERBOSE: Perform operation 'Invoke CimMethod' with following parameters, ''methodName' = TestConfiguration,'className' = MSFT_DSCLocalConfigurationManager,'namespaceName' = root/Microsoft/Windows/DesiredStateConfiguration'.
VERBOSE: An LCM method call arrived from computer DC01 with user sid S-1-5-21-530133819-3181352061-503517500-500.
VERBOSE: [DC01]: LCM:  [ Start  Test     ]
WARNING: [DC01]:                            [] The TEST operation will be carried against a pending configuration since the latest configuration has not converged yet.
VERBOSE: [DC01]:                            [DSCEngine] Importing the module C:\source\ActiveDirectoryDsc\DscResources\MSFT_ADReplicationSubnet\MSFT_ADReplicationSubnet.psm1 in force mode.
VERBOSE: [DC01]: LCM:  [ Start  Resource ]  [[ADReplicationSubnet]LondonSubnet]
VERBOSE: [DC01]: LCM:  [ Start  Test     ]  [[ADReplicationSubnet]LondonSubnet]
VERBOSE: [DC01]:                            [[ADReplicationSubnet]LondonSubnet] Importing the module MSFT_ADReplicationSubnet in force mode.
VERBOSE: [DC01]:                            [[ADReplicationSubnet]LondonSubnet] Getting replication subnet '10.0.0.0/24'. (ADRS0003)
VERBOSE: [DC01]:                            [[ADReplicationSubnet]LondonSubnet] Replication subnet '10.0.0.0/24' is absent. (ADRS0006)
VERBOSE: [DC01]:                            [[ADReplicationSubnet]LondonSubnet] The replication subnet '10.0.0.0/24' is not in the desired state. (ADRS0009)
VERBOSE: [DC01]: LCM:  [ End    Test     ]  [[ADReplicationSubnet]LondonSubnet] False in 0.2970 seconds.
VERBOSE: [DC01]: LCM:  [ End    Resource ]  [[ADReplicationSubnet]LondonSubnet]
VERBOSE: [DC01]: LCM:  [ End    Test     ]     Completed processing test operation. The operation returned False.
VERBOSE: [DC01]: LCM:  [ End    Test     ]    in  0.7030 seconds.
VERBOSE: Operation 'Invoke CimMethod' complete.
VERBOSE: Time taken for configuration job to complete is 1.681 seconds
      [-] Should return $true when Test-DscConfiguration is run 1.76s
        Expected strings to be the same, but they were different.
        Expected length: 4
        Actual length:   5
        Strings differ at index 0.
        Expected: 'True'
        But was:  'False'
        82:                 Test-DscConfiguration -Verbose | Should -Be 'True'
        at <ScriptBlock>, C:\source\ActiveDirectoryDsc\Tests\Integration\MSFT_ADReplicationSubnet.Integration.Tests.ps1: line 82

Copy link
Contributor Author

@1800Zeta 1800Zeta left a comment

Choose a reason for hiding this comment

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

Reviewable status: 7 of 9 files reviewed, 1 unresolved discussion (waiting on @johlju)


Tests/Integration/MSFT_ADReplicationSubnet.Integration.Tests.ps1, line 37 at r9 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Getting this error. I think we need to add the Site that is used for testing as a prerequisites test and then make a cleanup test at the bottom that removes the site? 🤔 What do you think?

Executing script .\Tests\Integration\MSFT_ADReplicationSubnet.Integration.Tests.ps1

  Describing MSFT_ADReplicationSubnet_Integration

    Context When using configuration MSFT_ADReplicationSubnet_CreateSubnet_Config
VERBOSE: Perform operation 'Invoke CimMethod' with following parameters, ''methodName' = SendConfigurationApply,'className' = MSFT_DSCLocalConfigurationManager,'namespaceName' = root/Microsoft/Windows/DesiredStateConfiguration'.
VERBOSE: An LCM method call arrived from computer DC01 with user sid S-1-5-21-530133819-3181352061-503517500-500.
VERBOSE: [DC01]: LCM:  [ Start  Set      ]
VERBOSE: [DC01]:                            [DSCEngine] Importing the module C:\source\ActiveDirectoryDsc\DscResources\MSFT_ADReplicationSubnet\MSFT_ADReplicationSubnet.psm1 in force mode.
VERBOSE: [DC01]: LCM:  [ Start  Resource ]  [[ADReplicationSubnet]LondonSubnet]
VERBOSE: [DC01]: LCM:  [ Start  Test     ]  [[ADReplicationSubnet]LondonSubnet]
VERBOSE: [DC01]:                            [[ADReplicationSubnet]LondonSubnet] Importing the module MSFT_ADReplicationSubnet in force mode.
VERBOSE: [DC01]:                            [[ADReplicationSubnet]LondonSubnet] Getting replication subnet '10.0.0.0/24'. (ADRS0003)
VERBOSE: [DC01]:                            [[ADReplicationSubnet]LondonSubnet] Replication subnet '10.0.0.0/24' is absent. (ADRS0006)
VERBOSE: [DC01]:                            [[ADReplicationSubnet]LondonSubnet] The replication subnet '10.0.0.0/24' is not in the desired state. (ADRS0009)
VERBOSE: [DC01]: LCM:  [ End    Test     ]  [[ADReplicationSubnet]LondonSubnet]  in 0.2190 seconds.
VERBOSE: [DC01]: LCM:  [ Start  Set      ]  [[ADReplicationSubnet]LondonSubnet]
VERBOSE: [DC01]:                            [[ADReplicationSubnet]LondonSubnet] Importing the module MSFT_ADReplicationSubnet in force mode.
VERBOSE: [DC01]:                            [[ADReplicationSubnet]LondonSubnet] Create the replication subnet '10.0.0.0/24'. (ADRS0001)
      [-] Should compile and apply the MOF without throwing 1.68s
        Expected no exception to be thrown, but an exception "Identity info provided in the extended attribute: 'Site' could not be resolved. Reason: 'Cannot find an object with identity: 'London' under: 'CN=Sites,CN=Configuration,DC=contoso,DC=com'.'." was thrown from C:\sou
rce\ActiveDirectoryDsc\Tests\Integration\MSFT_ADReplicationSubnet.Integration.Tests.ps1:59 char:21
            + ...               Start-DscConfiguration @startDscConfigurationParameters
            +                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~.
        60:                 } | Should -Not -Throw
        at <ScriptBlock>, C:\source\ActiveDirectoryDsc\Tests\Integration\MSFT_ADReplicationSubnet.Integration.Tests.ps1: line 41
VERBOSE: An LCM method call arrived from computer DC01 with user sid S-1-5-21-530133819-3181352061-503517500-500.
WARNING: [DC01]:                            [] The GET operation will be carried against a pending configuration since the latest configuration has not converged yet.
VERBOSE: [DC01]:                            [DSCEngine] Importing the module C:\source\ActiveDirectoryDsc\DscResources\MSFT_ADReplicationSubnet\MSFT_ADReplicationSubnet.psm1 in force mode.
VERBOSE: [DC01]: LCM:  [ Start  Get      ]
VERBOSE: [DC01]:                            [[ADReplicationSubnet]LondonSubnet] Importing the module MSFT_ADReplicationSubnet in force mode.
VERBOSE: [DC01]:                            [[ADReplicationSubnet]LondonSubnet] Getting replication subnet '10.0.0.0/24'. (ADRS0003)
VERBOSE: [DC01]:                            [[ADReplicationSubnet]LondonSubnet] Replication subnet '10.0.0.0/24' is absent. (ADRS0006)
VERBOSE: [DC01]: LCM:  [ End    Get      ]  [[ADReplicationSubnet]LondonSubnet]  in 0.2820 seconds.
VERBOSE: [DC01]: LCM:  [ End    Get      ]    in  0.7200 seconds.
      [+] Should be able to call Get-DscConfiguration without throwing 1.24s
      [+] Should have set the resource and all the parameters should match 7ms
VERBOSE: Perform operation 'Invoke CimMethod' with following parameters, ''methodName' = TestConfiguration,'className' = MSFT_DSCLocalConfigurationManager,'namespaceName' = root/Microsoft/Windows/DesiredStateConfiguration'.
VERBOSE: An LCM method call arrived from computer DC01 with user sid S-1-5-21-530133819-3181352061-503517500-500.
VERBOSE: [DC01]: LCM:  [ Start  Test     ]
WARNING: [DC01]:                            [] The TEST operation will be carried against a pending configuration since the latest configuration has not converged yet.
VERBOSE: [DC01]:                            [DSCEngine] Importing the module C:\source\ActiveDirectoryDsc\DscResources\MSFT_ADReplicationSubnet\MSFT_ADReplicationSubnet.psm1 in force mode.
VERBOSE: [DC01]: LCM:  [ Start  Resource ]  [[ADReplicationSubnet]LondonSubnet]
VERBOSE: [DC01]: LCM:  [ Start  Test     ]  [[ADReplicationSubnet]LondonSubnet]
VERBOSE: [DC01]:                            [[ADReplicationSubnet]LondonSubnet] Importing the module MSFT_ADReplicationSubnet in force mode.
VERBOSE: [DC01]:                            [[ADReplicationSubnet]LondonSubnet] Getting replication subnet '10.0.0.0/24'. (ADRS0003)
VERBOSE: [DC01]:                            [[ADReplicationSubnet]LondonSubnet] Replication subnet '10.0.0.0/24' is absent. (ADRS0006)
VERBOSE: [DC01]:                            [[ADReplicationSubnet]LondonSubnet] The replication subnet '10.0.0.0/24' is not in the desired state. (ADRS0009)
VERBOSE: [DC01]: LCM:  [ End    Test     ]  [[ADReplicationSubnet]LondonSubnet] False in 0.2970 seconds.
VERBOSE: [DC01]: LCM:  [ End    Resource ]  [[ADReplicationSubnet]LondonSubnet]
VERBOSE: [DC01]: LCM:  [ End    Test     ]     Completed processing test operation. The operation returned False.
VERBOSE: [DC01]: LCM:  [ End    Test     ]    in  0.7030 seconds.
VERBOSE: Operation 'Invoke CimMethod' complete.
VERBOSE: Time taken for configuration job to complete is 1.681 seconds
      [-] Should return $true when Test-DscConfiguration is run 1.76s
        Expected strings to be the same, but they were different.
        Expected length: 4
        Actual length:   5
        Strings differ at index 0.
        Expected: 'True'
        But was:  'False'
        82:                 Test-DscConfiguration -Verbose | Should -Be 'True'
        at <ScriptBlock>, C:\source\ActiveDirectoryDsc\Tests\Integration\MSFT_ADReplicationSubnet.Integration.Tests.ps1: line 82

Done. Also realised that it should be [[ADReplicationSubnet]Integration_Test] so have fixed that now and all good.

Copy link
Member

@johlju johlju left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 2 files at r11.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @1800Zeta)


Tests/Integration/MSFT_ADReplicationSubnet.config.ps1, line 151 at r11 (raw file):

Creates a site as part of prerequisits

Wrong description here. :)

@johlju
Copy link
Member

johlju commented Sep 25, 2019

Awesome work on this! 😃 Just one more small comment then this is ready to merge I think. 🙂

@1800Zeta
Copy link
Contributor Author

Thank you @johlju I've updated the line :)

Copy link
Contributor Author

@1800Zeta 1800Zeta left a comment

Choose a reason for hiding this comment

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

Reviewable status: 8 of 9 files reviewed, 1 unresolved discussion (waiting on @johlju)


Tests/Integration/MSFT_ADReplicationSubnet.config.ps1, line 151 at r11 (raw file):

Previously, johlju (Johan Ljunggren) wrote…
Creates a site as part of prerequisits

Wrong description here. :)

Done.

Copy link
Member

@johlju johlju left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 1 files at r12.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@johlju johlju changed the title ADReplicationSubnet Add Integration Testing, Add Description ADReplicationSubnet: Add Integration Testing, Add Description Sep 25, 2019
@johlju johlju merged commit 3e96da3 into dsccommunity:dev Sep 25, 2019
@johlju johlju removed the waiting for code fix A review left open comments, and the pull request is waiting for changes to be pushed by the author. label Sep 25, 2019
@1800Zeta 1800Zeta deleted the ADReplicationSubnet branch October 15, 2019 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants