Migrate to Pester 5 #532

dan-hughes · 2024-11-24T16:04:03Z

Pull Request (PR) description

Migrate all tests to Pester 5

This Pull Request (PR) fixes the following issues

Fixes Intermittent Integration Test Failures on Windows Server 2022 #510

Task list

Added an entry to the change log under the Unreleased section of the
file CHANGELOG.md. Entry should say what was changed and how that
affects users (if applicable), and reference the issue being resolved
(if applicable).
Resource documentation added/updated in README.md.
Resource parameter descriptions added/updated in README.md, schema.mof
and comment-based help.
Comment-based help added/updated.
Localization strings added/updated in all localization files as appropriate.
Examples appropriately added/updated.
Unit tests added/updated. See DSC Community Testing Guidelines.
Integration tests added/updated (where possible). See DSC Community Testing Guidelines.
New/changed code adheres to DSC Community Style Guidelines.

This change is

johlju

Reviewed 29 of 29 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dan-hughes)

source/Private/Convert-CIDRToSubnetMask.ps1 line 9 at r1 (raw file):

        The array of addresses to that need to be converted.
#>
function Convert-CIDRToSubnetMask

If these are gonna be used in the MOF resources they need to be public functions, not private. Throughout where function is needed in MOF resource.

Code quote:

Convert-CIDRToSubnetMask

tests/TestHelpers/CommonTestHelper.psm1 line 210 at r1 (raw file):

    Test-NetworkTeamIntegrationEnvironment, `
    New-IntegrationLoopbackAdapter, `
    Remove-IntegrationLoopbackAdapter

Maybe here we should keep the export-modulemember since it says what to export even if there are more (private) functions in the module. 🤔

Code quote:

Export-ModuleMember -Function `
    Get-InvalidArgumentRecord, `
    Get-InvalidOperationRecord, `
    Test-NetworkTeamIntegrationEnvironment, `
    New-IntegrationLoopbackAdapter, `
    Remove-IntegrationLoopbackAdapter

dan-hughes · 2024-11-24T16:37:03Z

Reviewed 29 of 29 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dan-hughes)

source/Private/Convert-CIDRToSubnetMask.ps1 line 9 at r1 (raw file):
        The array of addresses to that need to be converted.
#>
function Convert-CIDRToSubnetMask
If these are gonna be used in the MOF resources they need to be public functions, not private. Throughout where function is needed in MOF resource.

Code quote:
Convert-CIDRToSubnetMask
tests/TestHelpers/CommonTestHelper.psm1 line 210 at r1 (raw file):
    Test-NetworkTeamIntegrationEnvironment, `
    New-IntegrationLoopbackAdapter, `
    Remove-IntegrationLoopbackAdapter
Maybe here we should keep the export-modulemember since it says what to export even if there are more (private) functions in the module. 🤔

Code quote:
Export-ModuleMember -Function `
    Get-InvalidArgumentRecord, `
    Get-InvalidOperationRecord, `
    Test-NetworkTeamIntegrationEnvironment, `
    New-IntegrationLoopbackAdapter, `
    Remove-IntegrationLoopbackAdapter

Yeah, give me a minute. I wanted to check the build worked. I'm going to get all the integration tests up and running as there are HQRM fixes required 😊

johlju · 2024-11-24T17:36:48Z

No worries No hurry. I saw it was not draft so when I had a minute left I just browsed through it. 🙂

dan-hughes · 2025-01-05T13:27:51Z

@johlju, ready to go.

johlju

Reviewed 24 of 45 files at r3, 55 of 66 files at r4, 4 of 4 files at r5, 1 of 2 files at r6, 11 of 11 files at r7, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @dan-hughes)

tests/TestHelpers/CommonTestHelper.psm1 line 210 at r1 (raw file):

Previously, dan-hughes (Daniel Hughes) wrote…

Yeah, give me a minute. I wanted to check the build worked. I'm going to get all the integration tests up and running as there are HQRM fixes required 😊

Should we uncomment the Export-ModuleMember block now?

tests/Integration/DSC_NetAdapterName.Integration.Tests.ps1 line 198 at r7 (raw file):

                $_.ConfigurationName -eq "$($script:dscResourceName)_Config_NameOnly"
            }

Looks like there are blank spaces on this line, don't we have a VS Code project setting for automatically trimming white space at end of line on save? 🤔

Code quote:

···········

source/Public/Get-DnsClientServerStaticAddress.ps1 line 16 at r7 (raw file):

        IP address family.
#>
function Get-DnsClientServerStaticAddress

I wonder if we should prefix each public function (commented on another module as well). If there is another exported command with the same name it will be a conflict, so wonder if we should prefix all commands with NetDsc or something to make sure they are unique?

What do you think?

Code quote:

Get-DnsClientServerStaticAddress

dan-hughes

Reviewable status: 95 of 105 files reviewed, 2 unresolved discussions (waiting on @johlju)

tests/Integration/DSC_NetAdapterName.Integration.Tests.ps1 line 198 at r7 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Looks like there are blank spaces on this line, don't we have a VS Code project setting for automatically trimming white space at end of line on save? 🤔

Fixed, not sure why vscode/PSSA was not picking this up. Checked all the integration test files.

source/Public/Get-DnsClientServerStaticAddress.ps1 line 16 at r7 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

I wonder if we should prefix each public function (commented on another module as well). If there is another exported command with the same name it will be a conflict, so wonder if we should prefix all commands with NetDsc or something to make sure they are unique?

What do you think?

Valid point, but instead of a prefix maybe a suffix instead? Then searching for usages across repos may be easier as atleast the start of the function name matches.

…into pester-5

johlju

Reviewed 9 of 10 files at r8, 52 of 52 files at r9, all commit messages.
Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @dan-hughes)

source/Public/Get-DnsClientServerStaticAddress.ps1 line 16 at r7 (raw file):

Previously, dan-hughes (Daniel Hughes) wrote…

Valid point, but instead of a prefix maybe a suffix instead? Then searching for usages across repos may be easier as atleast the start of the function name matches.

This is not relevant anymore since we are moving them back to a private module and not a Public function. But I think it is more best practice to add a prefix than a suffix, so they are not mixed up with commands from another module, but I can see your point of the discovery so I'm not sure. 🤔 But that is something we can think about 'till next time 🙂

source/DSCResources/DSC_DefaultGatewayAddress/DSC_DefaultGatewayAddress.psm1 line 6 at r9 (raw file):

Import-Module -Name (Join-Path -Path $modulePath `
        -ChildPath (Join-Path -Path 'NetworkingDsc.Common' `
            -ChildPath 'NetworkingDsc.Common.psm1'))

DO we need to specify the psm1 file, isn't it enough with the folde NetworkingDsc.Common? If so, throughout.

Code quote:

-ChildPath 'NetworkingDsc.Common.psm1'

source/DSCResources/DSC_NetAdapterLso/DSC_NetAdapterLso.psm1 line 7 at r9 (raw file):

        -ChildPath (Join-Path -Path 'NetworkingDsc.Common' `
            -ChildPath 'NetworkingDsc.Common.psm1'))

Blank spaces here too. Okay.. there are in most files it seems. So thoughtout. 🙂

Code quote:

···········

source/DSCResources/DSC_DnsConnectionSuffix/DSC_DnsConnectionSuffix.psm1 line 7 at r9 (raw file):

        -ChildPath (Join-Path -Path 'NetworkingDsc.Common' `
            -ChildPath 'NetworkingDsc.Common.psm1'))

Here are those blank spaces again 🤔 Strange.

Code quote:

···········

source/DSCResources/DSC_HostsFile/DSC_HostsFile.psm1 line 7 at r9 (raw file):

        -ChildPath (Join-Path -Path 'NetworkingDsc.Common' `
            -ChildPath 'NetworkingDsc.Common.psm1'))

Blank spaces here too.

Code quote:

···········

source/DSCResources/DSC_NetAdapterBinding/DSC_NetAdapterBinding.psm1 line 7 at r9 (raw file):

        -ChildPath (Join-Path -Path 'NetworkingDsc.Common' `
            -ChildPath 'NetworkingDsc.Common.psm1'))

Blank spaces here too.

Code quote:

···········

tests/Unit/NetworkingDsc.Common/Public/Convert-CIDRToSubnetMask.Tests.ps1 line 37 at r9 (raw file):

    Import-Module -Name $script:subModulePath -Force -ErrorAction 'Stop'

    $PSDefaultParameterValues['InModuleScope:ModuleName'] = $script:subModuleName

I was hoping we could import the main module which will import the private module into the module scope and the InModuleScope would see the commands in the private module. But since you made it import here separately it didn't work like I was expecting?

Code quote:

$PSDefaultParameterValues['InModuleScope:ModuleName'] = $script:subModuleName

tests/Integration/DSC_Firewall.Integration.Tests.ps1 line 47 at r9 (raw file):

    Import-Module -Name (Join-Path -Path $PSScriptRoot -ChildPath '..\TestHelpers\CommonTestHelper.psm1')

    $script:subModuleName = 'NetworkingDsc.Common'

Why are we importing the private module in an integration test?

Code quote:

$script:subModuleName = 'NetworkingDsc.Common'

tests/Integration/DSC_Firewall.Integration.Tests.ps1 line 49 at r9 (raw file):

    $script:subModuleName = 'NetworkingDsc.Common'

    $script:parentModule = Get-Module -Name $script:dscModuleName -ListAvailable | Select-Object -First 1

Why do we need to select the first here, why are they more than one module installed when this runs? 🤔

Code quote:

| Select-Object -First 1

source/DSCResources/DSC_NetAdapterAdvancedProperty/DSC_NetAdapterAdvancedProperty.psm1 line 7 at r9 (raw file):

        -ChildPath (Join-Path -Path 'NetworkingDsc.Common' `
            -ChildPath 'NetworkingDsc.Common.psm1'))

Blank spaces here too.

Code quote:

···········

dan-hughes

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

source/DSCResources/DSC_DefaultGatewayAddress/DSC_DefaultGatewayAddress.psm1 line 6 at r9 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

DO we need to specify the psm1 file, isn't it enough with the folde NetworkingDsc.Common? If so, throughout.

Done.

source/DSCResources/DSC_DnsConnectionSuffix/DSC_DnsConnectionSuffix.psm1 line 7 at r9 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Here are those blank spaces again 🤔 Strange.

Done.

source/DSCResources/DSC_HostsFile/DSC_HostsFile.psm1 line 7 at r9 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Blank spaces here too.

Done.

source/DSCResources/DSC_NetAdapterAdvancedProperty/DSC_NetAdapterAdvancedProperty.psm1 line 7 at r9 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Blank spaces here too.

Done.

source/DSCResources/DSC_NetAdapterBinding/DSC_NetAdapterBinding.psm1 line 7 at r9 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Blank spaces here too.

Done.

source/DSCResources/DSC_NetAdapterLso/DSC_NetAdapterLso.psm1 line 7 at r9 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Blank spaces here too. Okay.. there are in most files it seems. So thoughtout. 🙂

Done.

tests/Integration/DSC_Firewall.Integration.Tests.ps1 line 47 at r9 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Why are we importing the private module in an integration test?

Using one of the functions from within the module. This happened in the Pester 4 tests.

tests/Integration/DSC_Firewall.Integration.Tests.ps1 line 49 at r9 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Why do we need to select the first here, why are they more than one module installed when this runs? 🤔

Done.

tests/Unit/NetworkingDsc.Common/Public/Convert-CIDRToSubnetMask.Tests.ps1 line 37 at r9 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

I was hoping we could import the main module which will import the private module into the module scope and the InModuleScope would see the commands in the private module. But since you made it import here separately it didn't work like I was expecting?

It did not work like that. Not sure why though. What you say makes sense but if I change it back Pester does not seem to find the functions.

johlju

Reviewed 28 of 28 files at r10, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @dan-hughes)

dan-hughes added 9 commits November 23, 2024 16:40

Prepare module for Pester 5

3b8fcbe

Remove function provided by DscResource.Test

bfd2559

Update DefaultGatewayAddress

a402179

UPdate DSC_DnsClientGlobalSetting

f720329

Remove .Common module

ec6f608

Move private functions back to resource

59d6f5a

Unpack .Common module to private/public

7c7ec1b

Update DefaultGateway Integration Test

f504d8f

Do not copy modules directory

1ccfa39

johlju requested changes Nov 24, 2024

View reviewed changes

Update Integration tests

705b721

dan-hughes marked this pull request as draft November 24, 2024 16:37

dan-hughes added 7 commits November 24, 2024 16:42

Update friendlyname

7e9bdee

Re-add export-modulemember

e6555e2

Remove .Common module import

24e4481

Move .common functions to public

516bdc8

Add cspell words

ae94442

Update integration tests

4626a18

Update integration tests

2c63258

dan-hughes added 9 commits November 24, 2024 17:47

Disable export on commontesthelper

598e454

Update whitespace

9149bd7

Update tests

abe06c8

Update whitespace

fae416c

Update config file path

295092b

Update integration tests

b51bebd

Update integration tests

a4dd90e

Move adapter creation and cleanup to main describe

5196654

Update tests

4ffcc6d

dan-hughes added 3 commits January 5, 2025 12:38

Remove DisplayGroup parameter from new rule creation

ab7de4d

Use non alias function name

138ee7c

Update changelog

a629299

dan-hughes marked this pull request as ready for review January 5, 2025 12:57

dan-hughes added 3 commits January 5, 2025 13:10

Remove commented code

be7ba0c

Remove Pesterv4 test function

ff1b8f7

Remove commented code

977ecea

johlju requested changes Jan 12, 2025

View reviewed changes

johlju added 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 Jan 12, 2025

dan-hughes added 2 commits January 12, 2025 13:23

Re-enable Export-ModuleMember

1bcfc08

Update formatting

88712ed

dan-hughes commented Jan 12, 2025

View reviewed changes

dan-hughes added 9 commits January 18, 2025 17:58

Suffix public command with NetDsc

01187ac

Fix typo

3f246d9

Reinstate NetworkingDsc.Common

5a2bfa9

Merge branch 'pester-5' of https://github.com/dan-hughes/NetworkingDsc …

0710f66

…into pester-5

Fix HQRM

6843a51

Get-InvalidArgumentRecord is from DscResource.Test

e70f657

Update changelog

1452685

Fix tests

7ccbe08

Use pwsh for unit tests

b6f44aa

johlju requested changes Feb 16, 2025

View reviewed changes

Fix import

90d784e

dan-hughes commented Feb 16, 2025

View reviewed changes

johlju mentioned this pull request Mar 16, 2025

DnsClientNrptGlobal-DnsClientNrptRule: New resources #533

Merged

9 tasks

johlju approved these changes Mar 16, 2025

View reviewed changes

johlju merged commit c165bcc into dsccommunity:main Mar 16, 2025
11 checks passed

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 Mar 16, 2025

dan-hughes deleted the pester-5 branch March 16, 2025 18:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Migrate to Pester 5 #532

Migrate to Pester 5 #532

Uh oh!

dan-hughes commented Nov 24, 2024 •

edited

Loading

Uh oh!

johlju left a comment

Uh oh!

dan-hughes commented Nov 24, 2024

Uh oh!

johlju commented Nov 24, 2024

Uh oh!

dan-hughes commented Jan 5, 2025

Uh oh!

johlju left a comment

Uh oh!

dan-hughes left a comment

Uh oh!

johlju left a comment

Uh oh!

dan-hughes left a comment

Uh oh!

johlju left a comment

Uh oh!

Uh oh!

Uh oh!

Migrate to Pester 5 #532

Migrate to Pester 5 #532

Uh oh!

Conversation

dan-hughes commented Nov 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request (PR) description

This Pull Request (PR) fixes the following issues

Task list

Uh oh!

johlju left a comment

Choose a reason for hiding this comment

Uh oh!

dan-hughes commented Nov 24, 2024

Uh oh!

johlju commented Nov 24, 2024

Uh oh!

dan-hughes commented Jan 5, 2025

Uh oh!

johlju left a comment

Choose a reason for hiding this comment

Uh oh!

dan-hughes left a comment

Choose a reason for hiding this comment

Uh oh!

johlju left a comment

Choose a reason for hiding this comment

Uh oh!

dan-hughes left a comment

Choose a reason for hiding this comment

Uh oh!

johlju left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dan-hughes commented Nov 24, 2024 •

edited

Loading