Resolve PowerShell lint errors and enable CI enforcement by flanakin · Pull Request #1988 · microsoft/finops-toolkit

flanakin · 2026-02-11T11:16:20Z

🛠️ Description

This PR fixes all 12 PowerShell lint errors and adds configuration to prevent future issues. It also fixes a critical CI issue where lint/unit test failures were not actually failing the GitHub Action.

Lint fixes:

Trailing whitespace: Invoke-Rest.ps1, Save-FinOpsHubTemplate.ps1, Start-FinOpsCostExport.ps1
UTF-8 BOM encoding: Get-FinOpsCostExport.ps1, Remove-FinOpsCostExport.ps1, Start-FinOpsCostExport.ps1
OutputType mismatch: Split-AzureResourceId.ps1 - return type changed from @{} to [AzureResourceIdInfo]@{}
ShouldProcess: Initialize-FinOpsHubDeployment.ps1, New-FinOpsCostExport.ps1, Start-FinOpsCostExport.ps1
Write-Host usage: Remove-FinOpsHub.ps1 - changed to Write-Information
Global variable: Initialize-Tests.ps1 - refactored to use Get-FinOpsHubRequiredResourceProvider function

Configuration improvements:

.editorconfig: Added charset = utf-8-bom for .ps1, .psm1, .psd1 files
.vscode/settings.json: Added files.encoding: utf8bom and files.trimTrailingWhitespace: true for PowerShell

CI fix:

.build/BuildHelper/Start-PesterTest.ps1: Added $pesterArgs.Run.Exit = $true so Pester returns non-zero exit code on test failures, which will now fail the GitHub Action

📋 Checklist

🔬 How did you test this change?

🤏 Lint tests

🤞 PS -WhatIf / az validate

👍 Manually deployed + verified

💪 Unit tests

🙌 Integration tests

🙋‍♀️ Do any of the following that apply?

🚨 This is a breaking change.

🤏 The change is less than 20 lines of code.

📑 Did you update `docs/changelog.md`?

✅ Updated changelog (required for dev PRs)

➡️ Will add log in a future PR (feature branch PRs only)

❎ Log not needed (small/internal change)

📖 Did you update documentation?

✅ Public docs in docs (required for dev)

✅ Public docs in docs-mslearn (required for dev)

✅ Internal dev docs in docs-wiki (required for dev)

✅ Internal dev docs in src (required for dev)

➡️ Will add docs in a future PR (feature branch PRs only)

❎ Docs not needed (small/internal change)

🤖 Generated with Claude Code

- Fix trailing whitespace in Invoke-Rest.ps1, Save-FinOpsHubTemplate.ps1, Start-FinOpsCostExport.ps1 - Add UTF-8 BOM encoding to Get-FinOpsCostExport.ps1, Remove-FinOpsCostExport.ps1, Start-FinOpsCostExport.ps1 - Fix OutputType mismatch in Split-AzureResourceId.ps1 - Add ShouldProcess support to Initialize-FinOpsHubDeployment.ps1, New-FinOpsCostExport.ps1, Start-FinOpsCostExport.ps1 - Replace Write-Host with Write-Information in Remove-FinOpsHub.ps1 - Refactor global variable to function in Initialize-Tests.ps1 Configuration improvements: - Add charset=utf-8-bom for PowerShell files in .editorconfig - Add files.encoding and trimTrailingWhitespace for PowerShell in VS Code settings - Enable Pester Run.Exit to fail CI on test failures Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Copilot

Pull request overview

This PR targets PowerShell code quality and CI reliability in the FinOps toolkit by resolving PowerShell lint issues (formatting/encoding and PSScriptAnalyzer rules) and ensuring test failures correctly fail the build.

Changes:

Updated several PowerShell cmdlets to align with lint rules (ShouldProcess support, OutputType consistency, and replacing Write-Host usage).
Normalized PowerShell file encoding/whitespace handling via .editorconfig and VS Code workspace settings.
Fixed CI/Pester invocation so failed tests produce a non-zero exit code.

Reviewed changes

Copilot reviewed 14 out of 16 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/powershell/Tests/Unit/New-FinOpsCostExport.Tests.ps1	Updates assertions for trimmed default storage path behavior.
src/powershell/Tests/Unit/Deploy-FinOpsHub.Tests.ps1	Improves mocks to match actual function calls/signatures.
src/powershell/Tests/Integration/Hubs.Tests.ps1	Removes dependency on a global variable; uses helper function instead.
src/powershell/Tests/Initialize-Tests.ps1	Replaces a global test variable with a helper function returning required RPs.
src/powershell/Public/Start-FinOpsCostExport.ps1	Adds `SupportsShouldProcess` handling and formatting fixes.
src/powershell/Public/Remove-FinOpsHub.ps1	Replaces `Write-Host` with `Write-Information` for deletion output.
src/powershell/Public/Remove-FinOpsCostExport.ps1	Updates file encoding (BOM).
src/powershell/Public/New-FinOpsCostExport.ps1	Adds `SupportsShouldProcess` and gates creation/run behavior under `ShouldProcess`.
src/powershell/Public/Initialize-FinOpsHubDeployment.ps1	Wraps provider registration call behind `ShouldProcess`.
src/powershell/Public/Get-FinOpsCostExport.ps1	Updates file encoding (BOM).
src/powershell/Private/Split-AzureResourceId.ps1	Adjusts return type to match declared `OutputType`.
src/powershell/Private/Save-FinOpsHubTemplate.ps1	Trims whitespace-only line.
src/powershell/Private/Invoke-Rest.ps1	Removes trailing whitespace.
.vscode/settings.json	Enforces PowerShell encoding and trimming behavior in VS Code.
.editorconfig	Sets PowerShell file charset to UTF-8 BOM and standardizes indentation.
.build/BuildHelper/Start-PesterTest.ps1	Ensures Pester exits non-zero on test failures (`Run.Exit = $true`).

Copilot · 2026-02-11T11:23:05Z

src/powershell/Private/Split-AzureResourceId.ps1

    if ($Id)
    {
        Write-Verbose "Parsing resource ID: '$Id'"

        if (-not $Id)
        {
-            return @{ ResourceId = $null }
+            return [AzureResourceIdInfo]@{ ResourceId = $null }
        }


Split-AzureResourceId has an outer if ($Id) guard, but then immediately checks if (-not $Id) inside that block. That inner branch is unreachable, and passing an empty string / $null currently returns no object despite [OutputType([AzureResourceIdInfo])] and [AllowNull()]/[AllowEmptyString()]. Consider handling null/empty at the top (returning an AzureResourceIdInfo with null fields) and removing the redundant inner check.

Copilot · 2026-02-11T11:23:05Z

src/powershell/Public/Start-FinOpsCostExport.ps1

    {
        Write-Verbose "Exporting $($StartDate) - $($EndDate)"
    }
+
+    if (-not $PSCmdlet.ShouldProcess($Name, 'Run cost export'))


The verbose messaging around $StartDate looks inverted: when $StartDate is set, it currently says it's exporting dates configured on the export definition, and when $StartDate is not set it prints "Exporting - " (blank start/end). Swap the branches so the messages match the actual behavior.

Copilot · 2026-02-11T11:23:05Z

src/powershell/Public/Remove-FinOpsHub.ps1

+        Write-Information "The following resources will be deleted:"
+        $resources | ForEach-Object { Write-Information "- $($_.ResourceId)" }


Switching from Write-Host to Write-Information changes default visibility (Information stream is typically suppressed), so users may no longer see the list of resources being deleted. If you want to preserve the previous always-visible behavior, consider using Write-Information -InformationAction Continue (or another stream like Write-Verbose/Write-Warning, depending on intent).

Copilot · 2026-02-11T11:23:06Z

src/powershell/Public/New-FinOpsCostExport.ps1

+        if ($PSCmdlet.ShouldProcess($Name, 'Create cost export'))
        {
-            Write-Error "Unable to create export $Name in scope $Scope. Error: $($createResponse.Content.error.message) ($($createResponse.Content.error.code))" -ErrorAction Stop
-            return
-        }
+            $createResponse = Invoke-Rest -Method PUT -Uri $uri -Body $properties @commandDetails
+            if ($createResponse.Failure)
+            {


New-FinOpsCostExport now declares SupportsShouldProcess, but only the export creation is guarded by ShouldProcess. Resource-provider registration (Register-AzResourceProvider) earlier in the function can still run even when -WhatIf is specified. To honor -WhatIf, wrap provider registration in ShouldProcess as well or pass -WhatIf:$WhatIfPreference through to the registration cmdlet.

Copilot · 2026-02-11T11:23:06Z

.build/BuildHelper/Start-PesterTest.ps1

    $pesterArgs = New-PesterConfiguration
    $pesterArgs.Output.Verbosity = 'Detailed'
+    $pesterArgs.Run.Exit = $true


PR description indicates a changelog update is "not needed", but this PR changes user-facing PowerShell command behavior (ShouldProcess/OutputType/WhatIf semantics) and CI behavior. Per the repo guidance for PRs targeting dev, external-facing changes should be captured in docs-mslearn/toolkit/changelog.md (see docs-wiki/Branching-strategy.md around the dev requirements).

flanakin and others added 2 commits February 11, 2026 02:24

Fix PowerShell unit tests

931aa67

Copilot AI review requested due to automatic review settings February 11, 2026 11:16

flanakin requested a review from arthurclares as a code owner February 11, 2026 11:16

Copilot started reviewing on behalf of flanakin February 11, 2026 11:17 View session

microsoft-github-policy-service bot added the Needs: Review 👀 PR that is ready to be reviewed label Feb 11, 2026

microsoft-github-policy-service bot assigned arthurclares Feb 11, 2026

Copilot AI reviewed Feb 11, 2026

View reviewed changes

flanakin changed the title ~~fix: Resolve PowerShell lint errors and enable CI enforcement~~ Resolve PowerShell lint errors and enable CI enforcement Feb 11, 2026

flanakin added this to the v14 milestone Feb 13, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Resolve PowerShell lint errors and enable CI enforcement#1988

Resolve PowerShell lint errors and enable CI enforcement#1988
flanakin wants to merge 2 commits intodevfrom
flanakin/ps-lint-fixes

flanakin commented Feb 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		Write-Information "The following resources will be deleted:"
		$resources \| ForEach-Object { Write-Information "- $($_.ResourceId)" }

Conversation

flanakin commented Feb 11, 2026

🛠️ Description

Lint fixes:

Configuration improvements:

CI fix:

📋 Checklist

🔬 How did you test this change?

🙋‍♀️ Do any of the following that apply?

📑 Did you update docs/changelog.md?

📖 Did you update documentation?

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

📑 Did you update `docs/changelog.md`?