Stabilize PowerShell CI after Pester 6 release#2203
Open
MSBrett wants to merge 7 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR intends to fully revert the merged commit from PR #2163, removing the FinOps Hub NAT Gateway/private network-mode feature and undoing the associated test-suite changes in order to restore dev to the pre-#2163 state.
Changes:
- Removes NAT Gateway enablement (parameters, UI checkbox, hub type wiring) and deletes NAT Gateway resources/subnet settings from the FinOps Hub template modules.
- Reverts
Deploy-Hub.ps1andDeploy-FinOpsHubaway from the-NetworkModeparameter surface back to the prior private-networking switches/flags. - Updates multiple Pester tests and the docs changelog entry related to the reverted feature.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/templates/finops-hub/modules/Microsoft.FinOpsHubs/Core/infrastructure.bicep | Removes NAT Gateway resources and subnet NAT/defaultOutboundAccess settings (also contains a tagging regression noted in review). |
| src/templates/finops-hub/modules/hub.bicep | Removes enableNatGateway parameter and stops passing it into newHub(). |
| src/templates/finops-hub/modules/fx/hub-types.bicep | Removes natGateway from hub properties and removes the enableNatGateway function parameter/wiring. |
| src/templates/finops-hub/main.bicep | Removes enableNatGateway parameter and stops passing it into the hub module. |
| src/templates/finops-hub/createUiDefinition.json | Removes NAT Gateway checkbox UI, parameter mapping, and resource-type entries for NAT/Public IP. |
| src/scripts/Deploy-Hub.ps1 | Removes -NetworkMode support and simplifies private networking behavior back to -Private. |
| src/powershell/Public/Deploy-FinOpsHub.ps1 | Removes -NetworkMode logic and NAT-related version gating; reverts to -DisablePublicAccess behavior (help text mismatch noted in review). |
| src/powershell/Tests/Unit/Deploy-FinOpsHub.Tests.ps1 | Removes “Network mode” unit-test context and migrates mock assertions to Assert-MockCalled. |
| src/powershell/Tests/Unit/DocsLinks.Tests.ps1 | Simplifies link-validation contexts by removing conditional “else” test blocks. |
| src/powershell/Tests/Initialize-Tests.ps1 | Removes explicit global import of Pester during test initialization. |
| src/powershell/Tests/Unit/Start-FinOpsCostExport.Tests.ps1 | Migrates mock assertions from Should -Invoke to Assert-MockCalled. |
| src/powershell/Tests/Unit/Save-FinOpsHubTemplate.Tests.ps1 | Migrates mock assertions from Should -Invoke to Assert-MockCalled. |
| src/powershell/Tests/Unit/Register-FinOpsHubProviders.Tests.ps1 | Migrates mock assertions from Should -Invoke to Assert-MockCalled. |
| src/powershell/Tests/Unit/New-FinOpsCostExport.Tests.ps1 | Migrates mock assertions from Should -Invoke to Assert-MockCalled. |
| src/powershell/Tests/Unit/New-Directory.Tests.ps1 | Migrates mock assertions from Should -Invoke to Assert-MockCalled. |
| src/powershell/Tests/Unit/Initialize-FinOpsHubDeployment.Tests.ps1 | Migrates mock assertions from Should -Invoke to Assert-MockCalled. |
| src/powershell/Tests/Unit/Get-FinOpsToolkitVersion.Tests.ps1 | Migrates mock assertions from Should -Invoke to Assert-MockCalled. |
| src/powershell/Tests/Unit/Get-FinOpsCostExport.Tests.ps1 | Migrates mock assertions from Should -Invoke to Assert-MockCalled. |
| src/powershell/Tests/Integration/CostExports.Tests.ps1 | Migrates mock assertions from Should -Invoke to Assert-MockCalled. |
| docs-mslearn/toolkit/changelog.md | Removes the unreleased FinOps hubs entry referencing PR #2163 and updates ms.date (ms.date value noted in review). |
Comments suppressed due to low confidence (2)
src/powershell/Public/Deploy-FinOpsHub.ps1:92
- The comment-based help documents a ".PARAMETER EnablePublicAccess", but the function does not define an EnablePublicAccess parameter (it uses -DisablePublicAccess). This makes Get-Help output misleading for users.
.PARAMETER EnablePublicAccess
Optional. Enable public access to the data lake. Default: true.
.PARAMETER VirtualNetworkAddressPrefix
Optional. Address space for the workload. A /26 is required for the workload. Default: "10.20.30.0/26".
docs-mslearn/toolkit/changelog.md:7
- docs-mslearn/toolkit/changelog.md body content was modified in this PR, but ms.date was changed to an older date. Repo guidance (AGENTS.md) expects ms.date to be set to today's date when body content changes to avoid CI conflicts.
ms.author: brettwil
ms.date: 07/07/2026
ms.topic: reference
Comment on lines
171
to
175
| resource vNet 'Microsoft.Network/virtualNetworks@2023-11-01' = if (hub.options.privateRouting) { | ||
| name: hub.routing.networkName | ||
| location: hub.location | ||
| tags: getHubTags(hub, 'Microsoft.Network/virtualNetworks') | ||
| dependsOn: hub.options.natGateway ? [ | ||
| natGateway | ||
| ] : [] | ||
| tags: getHubTags(hub, 'Microsoft.Storage/virtualNetworks') | ||
| properties: { |
Contributor
Author
There was a problem hiding this comment.
This reverts commit 9194469.
This reverts commit e28a97c.
Preserve the reviewed PR2163 product changes in PR2203 while reverting only the unauthorized test-suite edits from the post-approval commits. Pin PR CI to Pester 5.7.1 so the legacy test suite does not float to Pester 6 and fail on Assert-MockCalled/ForEach discovery behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
dev.Assert-MockCalled/ discovery behavior.Why this is needed
Pester 6.0.0 was published upstream on 2026-07-07 at 00:42:45 PDT. Because
.github/workflows/dev.ymlcachedPesterwithout a version, the next PR #2163 PowerShell Tests run resolved the new major version and loggedPester v6.0.0at 2026-07-07 10:06:58 PDT.That exposed existing test-suite compatibility issues (
Assert-MockCalledand empty-ForEachdiscovery behavior). The product/NAT Gateway code was not the cause. The post-approval test edits in PR #2163 attempted to adapt the suite after final approval, which bypassed review. This PR keeps the reviewed product work, removes only those unauthorized test edits, and pins CI back to the previously passing Pester 5.7.1 runtime.Requirements traceability
origin/dev, leaving NAT Gateway work intact..github/workflows/dev.ymlpinsPester:5.7.1.publish.ymlchange and no unrelated product changes in the final PR diff.Validation
pwsh -NoLogo -NoProfile -Command "Invoke-Pester -Output Detailed -Path ./src/powershell/Tests/Unit/*"