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

ADUser: Refactor unit tests for resource #467

Closed
johlju opened this issue Aug 4, 2019 · 0 comments · Fixed by #569
Closed

ADUser: Refactor unit tests for resource #467

johlju opened this issue Aug 4, 2019 · 0 comments · Fixed by #569
Assignees
Labels
tests The issue or pull request is about tests only.

Comments

@johlju
Copy link
Member

johlju commented Aug 4, 2019

Details of the scenario you tried and the problem that is occurring

When adding unit test for new functionality in ADUser the existing test was messing up the new tests becuase they did not run it there own context and most likely left mocks that interfered with the new unit tests.

Quick solution was to add those test into a separate context block named 'When running unit tests that need refactor' in the unit tests for Set-TargetResource.

Verbose logs showing the problem

This is the output before moving the existing unit test into a separate context-block as mentioned above. The problem occur when the mock for Set-ADUser was hit.

    Context When the system is in the desired state

      Context When the parameter ThumbnailPhoto is in desired state
VERBOSE: Updating Active Directory user 'TestUser'. (ADU0012)
        [-] Should not throw and call the correct mocks 69ms
          Expected no exception to be thrown, but an exception "You cannot call a method on a null-valued expression." was thrown from C:\Program Files\WindowsPowerShell\Modules\Pester\4.8.0\Functions\Mock.ps1:1241 char:5
              +     & $cmd @BoundParameters @ArgumentList
              +     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~.
          947:                         { Set-TargetResource @setTargetResourceParameters } | Should -Not -Throw
          at <ScriptBlock>, C:\Source\ActiveDirectoryDsc\Tests\Unit\MSFT_ADUser.Tests.ps1: line 947

    Context When the system is not in the desired state

      Context When the parameter ThumbnailPhoto is in desired state
VERBOSE: Updating the property 'thumbnailPhoto' with a new thumbnail photo with MD5 hash 'E3253C13DFF396BE98D6144F0DFA6105'. (ADU0022)
VERBOSE: Updating Active Directory user 'TestUser'. (ADU0012)
        [-] Should not throw and call the correct mocks 79ms
          Expected no exception to be thrown, but an exception "Exception calling "ContainsKey" with "1" argument(s): "Key cannot be null.
          Parameter name: key"" was thrown from C:\Program Files\WindowsPowerShell\Modules\Pester\4.8.0\Functions\Mock.ps1:1241 char:5
              +     & $cmd @BoundParameters @ArgumentList
              +     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~.
          970:                         { Set-TargetResource @setTargetResourceParameters } | Should -Not -Throw
          at <ScriptBlock>, C:\Source\ActiveDirectoryDsc\Tests\Unit\MSFT_ADUser.Tests.ps1: line 970

Suggested solution to the issue

Refactor the unit tests to use context blocks according to unit test template (and for example unit test for ADComputer).

The DSC configuration that is used to reproduce the issue (as detailed as possible)

Not applicable.

The operating system the target node is running

Not applicable.

Version and build of PowerShell the target node is running

Not applicable.

Version of the DSC module that was used ('dev' if using current dev branch)

dev

@johlju johlju added bug The issue is a bug. help wanted The issue is up for grabs for anyone in the community. tests The issue or pull request is about tests only. and removed bug The issue is a bug. labels Aug 4, 2019
@X-Guardian X-Guardian added in progress The issue is being actively worked on by someone. and removed help wanted The issue is up for grabs for anyone in the community. labels Feb 11, 2020
@X-Guardian X-Guardian self-assigned this Feb 11, 2020
johlju pushed a commit that referenced this issue Mar 9, 2020
- ADUser
  - Improve Try/Catch blocks to only cover cmdlet calls.
  - Move the Test-Password function to the ActiveDirectoryDsc.Common module and add unit tests.
  - Reformat code to keep line lengths to less than 120 characters.
  - Fix Password parameter processing when PasswordNeverResets is $true.
  - Remove unnecessary Enabled parameter check.
  - Remove unnecessary Clear explicit parameter check.
  - Add check to only call Set-ADUser if there are properties to change.
  - Refactored Unit Tests - (issue #467)
@johlju johlju removed the in progress The issue is being actively worked on by someone. label Apr 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests The issue or pull request is about tests only.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants