Skip to content

Comments

Resolve PowerShell lint errors and enable CI enforcement#1988

Merged
flanakin merged 6 commits intodevfrom
flanakin/ps-lint-fixes
Feb 24, 2026
Merged

Resolve PowerShell lint errors and enable CI enforcement#1988
flanakin merged 6 commits intodevfrom
flanakin/ps-lint-fixes

Conversation

@flanakin
Copy link
Collaborator

🛠️ 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

flanakin and others added 2 commits February 11, 2026 02:24
- 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 AI review requested due to automatic review settings February 11, 2026 11:16
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs: Review 👀 PR that is ready to be reviewed label Feb 11, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

@flanakin 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 flanakin added this to the v14 milestone Feb 13, 2026
@flanakin
Copy link
Collaborator Author

🤖 [AI][Claude] PR Update Summary

Addressed: 5 thread(s)

  • ✅ Implemented: 4
  • 🤔 Needs discussion: 1

Changes:

  • Removed unreachable dead code in Split-AzureResourceId.ps1
  • Fixed inverted verbose logging in Start-FinOpsCostExport.ps1
  • Wrapped Register-AzResourceProvider in ShouldProcess in New-FinOpsCostExport.ps1
  • Added changelog entry for user-facing fixes

Discussion: Write-Information in Remove-FinOpsHub.ps1 is intentional per PowerShell best practices (replacing Write-Host lint violation).

@flanakin flanakin enabled auto-merge (squash) February 23, 2026 09:36
- Skip Write-Error for throttled responses in Start-FinOpsCostExport to prevent termination when ErrorActionPreference is Stop
- Add missing Initialize-FinOpsHubDeployment mocks in Deploy-FinOpsHub test contexts

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@flanakin
Copy link
Collaborator Author

flanakin commented Feb 23, 2026

The 2 failing checks are resolved by PR #2021

@flanakin flanakin merged commit 939bb42 into dev Feb 24, 2026
4 checks passed
@flanakin flanakin deleted the flanakin/ps-lint-fixes branch February 24, 2026 06:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs: Review 👀 PR that is ready to be reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants