Adds xADComputer Resource #96

iainbrighton · 2016-05-19T12:28:42Z

Ignores DSCResource.Tests
Updates readme and adds example
Adds unit test coverage
Fixes Add xADComputer #69
Fixes Bug in xADDomainController #67

This change is

…variable

…into dev Conflicts: README.md

…into dev # Conflicts: # README.md

…into Issue69 # Conflicts: # DSCResources/MSFT_xADUser/MSFT_xADUser.psm1 # README.md

…urce

iainbrighton · 2016-05-19T12:46:51Z

One for you @PlagueHO!

PlagueHO · 2016-05-19T19:08:33Z

@iainbrighton - You're a legend! I'll review tonight (unless someone else gets there first)! 👍

PlagueHO · 2016-05-20T07:58:15Z

Reviewed 1 of 2 files at r1, 11 of 11 files at r2.
Review status: all files reviewed at latest revision, 5 unresolved discussions.

README.md, line 233 [r2] (raw file):

### Unreleased

You should add a mention of fixing the verbose log messages and the changes (minor) to xADGroup and xADUser.

DSCResources/MSFT_xADCommon/MSFT_xADCommon.ps1, line 303 [r2] (raw file):

            if ($member -notin $ExistingMembers)
            {
                Write-Verbose -Message ($localizedString.MemberNotInDesiredState -f $member);;

Nitpick: Even though you didn't add it, can we remove the 2nd semicolon at the end of the line?

Tests/Unit/MSFT_xADComputer.Tests.ps1, line 4 [r2] (raw file):

$Global:DSCResourceName    = 'MSFT_xADComputer' # Example MSFT_xFirewall

#region HEADER

Can you convert the template into the new version (v1.1.0) Unit Test Template?

Tests/Unit/MSFT_xADComputer.Tests.ps1, line 28 [r2] (raw file):

    # The InModuleScope command allows you to perform white-box unit testing on the internal
    # (non-exported) code of a Script Module.
    InModuleScope $Global:DSCResourceName {

You should probably get rid of the InModuleScope block as it isn't required. The templates got changed a few weeks ago to remove this so that it was only included when private functions needed to be tested.

Tests/Unit/MSFT_xADComputer.Tests.ps1, line 56 [r2] (raw file):

        $testDomainController = 'TESTDC';
        $testCredential = New-Object System.Management.Automation.PSCredential 'DummyUser', (ConvertTo-SecureString 'DummyPassword' -AsPlainText -Force);

PSSA is probably going to throw a hissy fit about this line so we'd need to add a [Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSAvoidUsingConvertToSecureStringWithPlainText", "")]

Comments from Reviewable

iainbrighton · 2016-05-20T21:51:06Z

Review status: all files reviewed at latest revision, 5 unresolved discussions.

README.md, line 233 [r2] (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

You should add a mention of fixing the verbose log messages and the changes (minor) to xADGroup and xADUser.

Done

DSCResources/MSFT_xADCommon/MSFT_xADCommon.ps1, line 303 [r2] (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Nitpick: Even though you didn't add it, can we remove the 2nd semicolon at the end of the line?

Done (it probably was me at some point!)

Tests/Unit/MSFT_xADComputer.Tests.ps1, line 4 [r2] (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you convert the template into the new version (v1.1.0) Unit Test Template?

I attempted to update the template and _everything_ blew up. Now, this might be due to not being able to install the AD RSAT tools on the latest Win 10 slow ring build. I will try in a VM over the weekend/early next week.

Tests/Unit/MSFT_xADComputer.Tests.ps1, line 28 [r2] (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

You should probably get rid of the InModuleScope block as it isn't required. The templates got changed a few weeks ago to remove this so that it was only included when private functions needed to be tested.

Acknowledged.

Tests/Unit/MSFT_xADComputer.Tests.ps1, line 56 [r2] (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

PSSA is probably going to throw a hissy fit about this line so we'd need to add a [Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSAvoidUsingConvertToSecureStringWithPlainText", "")]

Can the attribute be placed at the beginning of a .PS1 file (outside of a function)? Not checked, but will try and see what the PSSA says..

Comments from Reviewable

Removes extraneous semicolon

iainbrighton · 2016-05-23T15:43:26Z

Review status: 10 of 12 files reviewed at latest revision, 5 unresolved discussions.

Tests/Unit/MSFT_xADComputer.Tests.ps1, line 4 [r2] (raw file):

Previously, iainbrighton (Iain Brighton) wrote…

I attempted to update the template and everything blew up. Now, this might be due to not being able to install the AD RSAT tools on the latest Win 10 slow ring build. I will try in a VM over the weekend/early next week.

@PlagueHO Removing the `InModuleScope` scriptblock causes me a real problem and results in **every** test failing. All I receive are `The term 'xxx-TargetResource' is not recognized as the name of a cmdlet`. Here's an example:

Describing MSFT_xADComputer\Get-TargetResource
 [-] Returns a 'System.Collections.Hashtable' object type 9.01s
   The term 'Get-TargetResource' is not recognized as the name of a cmdlet, function, script file, or operable program.
Check the spelling of the name, or if a path was included, verify that the path is correct and try again.
   at line: 65 in C:\Users\iainb\Dropbox\PowerShell\DSC\Resources\xActiveDirectory\Tests\Unit\MSFT_xADComputer.Tests.ps1
 [-] Returns 'Ensure' is 'Present' when user account exists 161ms
   The term 'Get-TargetResource' is not recognized as the name of a cmdlet, function, script file, or operable program.
Check the spelling of the name, or if a path was included, verify that the path is correct and try again.
   at line: 73 in C:\Users\iainb\Dropbox\PowerShell\DSC\Resources\xActiveDirectory\Tests\Unit\MSFT_xADComputer.Tests.ps1
 [-] Returns 'Ensure' is 'Absent' when user account does not exist 165ms
   The term 'Get-TargetResource' is not recognized as the name of a cmdlet, function, script file, or operable program.
Check the spelling of the name, or if a path was included, verify that the path is correct and try again.
   at line: 81 in C:\Users\iainb\Dropbox\PowerShell\DSC\Resources\xActiveDirectory\Tests\Unit\MSFT_xADComputer.Tests.ps1
 [-] Calls 'Get-ADComputer' with 'Server' parameter when 'DomainController' specified 163ms
   The term 'Get-TargetResource' is not recognized as the name of a cmdlet, function, script file, or operable program.
Check the spelling of the name, or if a path was included, verify that the path is correct and try again.
   at line: 89 in C:\Users\iainb\Dropbox\PowerShell\DSC\Resources\xActiveDirectory\Tests\Unit\MSFT_xADComputer.Tests.ps1
 [-] Calls 'Get-ADComputer' with 'Credential' parameter when 'DomainAdministratorCredential' specified 164ms
   The term 'Get-TargetResource' is not recognized as the name of a cmdlet, function, script file, or operable program.
Check the spelling of the name, or if a path was included, verify that the path is correct and try again.
   at line: 97 in C:\Users\iainb\Dropbox\PowerShell\DSC\Resources\xActiveDirectory\Tests\Unit\MSFT_xADComputer.Tests.ps1

I've not had a real chance to dig into what the test helper(s) are doing, but it looks like the module's not imported correctly. It might be something to do with my setup though. Ideas?

Comments from Reviewable

… check

PlagueHO · 2016-05-24T06:28:01Z

Superb work on this BTW! 👍

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

@iainbrighton - You're a legend! I'll review tonight (unless someone else gets there first)! 👍

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

Tests/Unit/MSFT_xADComputer.Tests.ps1, line 4 [r2] (raw file):

Previously, iainbrighton (Iain Brighton) wrote…

@PlagueHO Removing the InModuleScope scriptblock causes me a real problem and results in every test failing. All I receive are The term 'xxx-TargetResource' is not recognized as the name of a cmdlet. Here's an example:

Describing MSFT_xADComputer\Get-TargetResource
 [-] Returns a 'System.Collections.Hashtable' object type 9.01s
   The term 'Get-TargetResource' is not recognized as the name of a cmdlet, function, script file, or operable program.
Check the spelling of the name, or if a path was included, verify that the path is correct and try again.
   at line: 65 in C:\Users\iainb\Dropbox\PowerShell\DSC\Resources\xActiveDirectory\Tests\Unit\MSFT_xADComputer.Tests.ps1
 [-] Returns 'Ensure' is 'Present' when user account exists 161ms
   The term 'Get-TargetResource' is not recognized as the name of a cmdlet, function, script file, or operable program.
Check the spelling of the name, or if a path was included, verify that the path is correct and try again.
   at line: 73 in C:\Users\iainb\Dropbox\PowerShell\DSC\Resources\xActiveDirectory\Tests\Unit\MSFT_xADComputer.Tests.ps1
 [-] Returns 'Ensure' is 'Absent' when user account does not exist 165ms
   The term 'Get-TargetResource' is not recognized as the name of a cmdlet, function, script file, or operable program.
Check the spelling of the name, or if a path was included, verify that the path is correct and try again.
   at line: 81 in C:\Users\iainb\Dropbox\PowerShell\DSC\Resources\xActiveDirectory\Tests\Unit\MSFT_xADComputer.Tests.ps1
 [-] Calls 'Get-ADComputer' with 'Server' parameter when 'DomainController' specified 163ms
   The term 'Get-TargetResource' is not recognized as the name of a cmdlet, function, script file, or operable program.
Check the spelling of the name, or if a path was included, verify that the path is correct and try again.
   at line: 89 in C:\Users\iainb\Dropbox\PowerShell\DSC\Resources\xActiveDirectory\Tests\Unit\MSFT_xADComputer.Tests.ps1
 [-] Calls 'Get-ADComputer' with 'Credential' parameter when 'DomainAdministratorCredential' specified 164ms
   The term 'Get-TargetResource' is not recognized as the name of a cmdlet, function, script file, or operable program.
Check the spelling of the name, or if a path was included, verify that the path is correct and try again.
   at line: 97 in C:\Users\iainb\Dropbox\PowerShell\DSC\Resources\xActiveDirectory\Tests\Unit\MSFT_xADComputer.Tests.ps1

I've not had a real chance to dig into what the test helper(s) are doing, but it looks like the module's not imported correctly. It might be something to do with my setup though. Ideas?

I'm scratching my head on this. It "should" work just fine. Let me have a dig about in some of my modules and see if I can replicate the problem. I might clone your repo and see if it happens with mine. Not good - don't want to be telling people to remove the InModuleScope if it in fact requires it!

Tests/Unit/MSFT_xADComputer.Tests.ps1, line 56 [r2] (raw file):

Previously, iainbrighton (Iain Brighton) wrote…

Can the attribute be placed at the beginning of a .PS1 file (outside of a function)? Not checked, but will try and see what the PSSA says..

I think it can be - at least the documentation says it can be: https://github.com/PowerShell/PSScriptAnalyzer#suppressing-rules

Although I think outside a function you need to add:
[Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSAvoidUsingConvertToSecureStringWithPlainText", "", Scope="Function")]

Comments from Reviewable

iainbrighton · 2016-05-24T06:39:15Z

Thinking about this (on the train), isn't the module imported inside the TestHelper scope? This is a different scope to where the Pester tests are executed and might be why they can't be found..?

PlagueHO · 2016-05-24T08:39:40Z

@iainbrighton - oh my - you could very well be correct. That would make a lot of sense. I say we use InModuleScope until we figure out a solution.

If there is no solution then we need to revert the Test.Templates and the TestGuidelines.

@KarolKaczmarek, @narrieta - can you think of a way around this problem: If the DSC Resource being tested is being imported by the Test.Helper.psm1 module, not by the actual unit test script, then it is not in the scope of the unit test script - so the tests needs to be in an InModuleScope block.

narrieta · 2016-05-24T20:38:12Z

@PlagueHO - in my opinion the module to test should be imported within the scope of the test script, rather than in the scope of Test.Helper.psm1. Importing at the global scope (Import-Module -Global) may also work, though I think the first option would be preferable.

One question regarding the import: Initialize-TestEnvironment explicitly removes the module before importing it. Is Import-Module -Force not enough?

One observation about Initialize-TestEnvironment: when I tried to execute the unit tests for ADComputer my PowerShell session seemed to hang so I had to kill the process. From then on, all the new PowerShell instances I started were extremely slow. The issue turned out to be duplicated entries in PSModulePath. Initialize-TestEnvironment changes the module path at the machine level; consider changing it at the process level and checking for duplicates. The execution policy should probably also be changed at the process level.

PlagueHO · 2016-05-25T07:46:41Z

@narrieta - thank you for looking at this and the info.

Regarding the Import-Module -Force - I think you're right, using the -Force parameter should be enough.

As for the Initialize-TestEnvironment problems, there is actually an issue in place that relates to this. I'm looking at doing some work on this but was just waiting to get a bit more feed back before proceeding. But it looks like the issues are really mounting up with this stuff so I might just go ahead and PR a solution and let people comment on that. I do agree though, the PSModulePath should only be changed at the process level - but only for the unit tests. For the integration tests to work this still requires the machine level changes because the LCM runs under a different process.

Thanks again for commenting on this - sorry @iainbrighton for hijacking this PR.

iainbrighton · 2016-05-25T20:16:14Z

@PlagueHO @narrieta No problem! If the import in the TestHelper.psm1 is changed to Import-Module -Name $DSCModuleFile.FullName -Force -Scope Global, then I can see the publicly exported *-TargetResource functions.

However, I can't seem to then mock the internal calls to Get-TargetResource from the Test- and Set-TargetResource functions without the InModuleScope [sigh].

narrieta · 2016-05-25T20:25:07Z

@iainbrighton @PlagueHO OK, too bad -Global doesn't work. I would recommend then importing the module to test from the test script instead of from TestHelper.psm1. It may be a small inconvenience but then only the code that actually needs access to the privates of the module needs to use InModuleScope (instead of the entire code of the test). Especially if we are doing this in the template that new test should follow.

PlagueHO · 2016-05-26T08:13:06Z

@narrieta, @iainbrighton - I've moved this issue out into a new issue in DSCResource.tests. Thanks heaps for looking into all of this.

TravisEz13 · 2016-05-26T23:21:56Z

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

Tests/Unit/MSFT_xADComputer.Tests.ps1, line 4 [r2] (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

I'm scratching my head on this. It "should" work just fine. Let me have a dig about in some of my modules and see if I can replicate the problem. I might clone your repo and see if it happens with mine. Not good - don't want to be telling people to remove the InModuleScope if it in fact requires it!

Why did we remove InModuleScope? Resources with private functions won't be able to be tested without this.

Comments from Reviewable

TravisEz13 · 2016-05-27T00:12:12Z

Review status: all files reviewed at latest revision, 2 unresolved discussions.

Tests/Unit/MSFT_xADComputer.Tests.ps1, line 4 [r2] (raw file):

Previously, TravisEz13 (Travis Plunk) wrote…

Why did we remove InModuleScope? Resources with private functions won't be able to be tested without this.

I got my answer offline.

Comments from Reviewable

PlagueHO · 2016-05-27T08:48:32Z

Review status: all files reviewed at latest revision, 2 unresolved discussions.

Tests/Unit/MSFT_xADComputer.Tests.ps1, line 4 [r2] (raw file):

Previously, TravisEz13 (Travis Plunk) wrote…

I got my answer offline.

Here are the original discussions for a bit more context: https://github.com/PowerShell/DscResources/issues/115 https://github.com/PowerShell/DscResources/issues/123

Comments from Reviewable

TravisEz13 · 2016-05-30T00:55:37Z

I reviewed, two discussions waiting on @iainbrighton and @PlagueHO to say they are satisfied with the resolution.

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

@narrieta, @iainbrighton - I've moved this issue out into a new issue in DSCResource.tests. Thanks heaps for looking into all of this.

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

Comments from Reviewable

iainbrighton · 2016-05-31T08:07:37Z

Review status: all files reviewed at latest revision, 2 unresolved discussions.

Tests/Unit/MSFT_xADComputer.Tests.ps1, line 56 [r2] (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

I think it can be - at least the documentation says it can be: https://github.com/PowerShell/PSScriptAnalyzer#suppressing-rules

Although I think outside a function you need to add:
[Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSAvoidUsingConvertToSecureStringWithPlainText", "", Scope="Function")]

I think using an empty [PSCredential] is the "cleaner" way of solving this (rather than suppressing the rule)?

Comments from Reviewable

PlagueHO · 2016-05-31T08:37:54Z

Review status: all files reviewed at latest revision, 2 unresolved discussions.

Tests/Unit/MSFT_xADComputer.Tests.ps1, line 56 [r2] (raw file):

Previously, iainbrighton (Iain Brighton) wrote…

I think using an empty [PSCredential] is the "cleaner" way of solving this (rather than suppressing the rule)?

I'm all for the cleaner way!

Comments from Reviewable

mgreenegit and others added 15 commits February 15, 2016 20:36

xADDomainController - verbose message was using incorrect variable

bb95fc2

xADDomainController - test function verbose output calling incorrect …

478d848

…variable

ReadMe - updated with notes on changes to xADDomainController

a2e6add

Merge branch 'dev' of https://github.com/mgreenegit/xActiveDirectory …

d76b0c7

…into dev Conflicts: README.md

Merge branch 'dev' of https://github.com/PowerShell/xActiveDirectory …

29eb7d3

…into dev # Conflicts: # README.md

Initial xADComputer implementation

df27a05

Ignores DSCResource.Tests folder

b4d490a

Updates MSFT_xADComputer test coverage

a4af0c1

Reduces line width

a5db1dd

Merge branch 'dev' of https://github.com/PowerShell/xActiveDirectory …

ebf1861

…into Issue69 # Conflicts: # DSCResources/MSFT_xADUser/MSFT_xADUser.psm1 # README.md

Adds missing SPN test

e5a44db

Updates .gitignore

69a9812

Updates readme and example

d30897f

Updates test template header

2682ba0

Fixes membership parameter assertion in MSFT_xADGroup\Test-TargetReso…

f90dd1f

…urce

msftclas added the cla-already-signed label May 19, 2016

iainbrighton mentioned this pull request May 19, 2016

Correct Verbose messages in xADDomainController - Fixes #67 #95

Closed

Updates readme with xDomainController fix

8153e25

Removes extraneous semicolon

iainbrighton added 2 commits May 23, 2016 16:44

Replaces ConvertTo-SecureString with empty credentials to bypass PSSA…

bd5defd

… check

Updates readme to trigger new build

c5a9043

This was referenced May 25, 2016

Remove Remove-Module from Initialize-TestEnvironment PowerShell/DscResource.Tests#57

Closed

Initialize-TestEnvironment should change PSModulePath at Process level for Unit Tests PowerShell/DscResource.Tests#58

Closed

PlagueHO mentioned this pull request May 26, 2016

Move Import-Module from Initialize-TestEnvironment to *.tests.ps1 PowerShell/DscResource.Tests#59

Closed

TravisEz13 merged commit f26c7a1 into dsccommunity:dev Jun 7, 2016

iainbrighton deleted the Issue69 branch July 8, 2016 21:04

kwirkykat mentioned this pull request Aug 18, 2016

PR # 68 should be ported to the current code base and brought to current contribution standards #91

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds xADComputer Resource #96

Adds xADComputer Resource #96

iainbrighton commented May 19, 2016 •

edited by SteveL-MSFT

Loading

iainbrighton commented May 19, 2016

PlagueHO commented May 19, 2016

PlagueHO commented May 20, 2016

iainbrighton commented May 20, 2016

iainbrighton commented May 23, 2016

PlagueHO commented May 24, 2016

iainbrighton commented May 24, 2016 via email

PlagueHO commented May 24, 2016

narrieta commented May 24, 2016

PlagueHO commented May 25, 2016

iainbrighton commented May 25, 2016

narrieta commented May 25, 2016

PlagueHO commented May 26, 2016

TravisEz13 commented May 26, 2016

TravisEz13 commented May 27, 2016

PlagueHO commented May 27, 2016

TravisEz13 commented May 30, 2016

iainbrighton commented May 31, 2016

PlagueHO commented May 31, 2016

Adds xADComputer Resource #96

Adds xADComputer Resource #96

Conversation

iainbrighton commented May 19, 2016 • edited by SteveL-MSFT Loading

iainbrighton commented May 19, 2016

PlagueHO commented May 19, 2016

PlagueHO commented May 20, 2016

iainbrighton commented May 20, 2016

iainbrighton commented May 23, 2016

PlagueHO commented May 24, 2016

iainbrighton commented May 24, 2016 via email

PlagueHO commented May 24, 2016

narrieta commented May 24, 2016

PlagueHO commented May 25, 2016

iainbrighton commented May 25, 2016

narrieta commented May 25, 2016

PlagueHO commented May 26, 2016

TravisEz13 commented May 26, 2016

TravisEz13 commented May 27, 2016

PlagueHO commented May 27, 2016

TravisEz13 commented May 30, 2016

iainbrighton commented May 31, 2016

PlagueHO commented May 31, 2016

iainbrighton commented May 19, 2016 •

edited by SteveL-MSFT

Loading