From b075f9df1f849e2675cd91975c2e1026fddbf3dc Mon Sep 17 00:00:00 2001 From: Bill Berry Date: Wed, 1 Apr 2026 12:22:35 -0700 Subject: [PATCH 1/5] feat(build): enforce strict warnings across all linters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - add ShellCheck CI workflow, PowerShell wrapper, and .shellcheckrc config - promote YamlLint warnings to failures for strict warnings compliance - add ShellCheck and YamlLint Pester unit tests with strict mode coverage - integrate ShellCheck into pr-validation and main workflows - update CONTRIBUTING.md warning policy table and package.json scripts Closes #6 🔧 - Generated by Copilot --- .github/workflows/main.yml | 14 +- .github/workflows/pr-validation.yml | 14 +- .github/workflows/shellcheck.yml | 60 ++++ .shellcheckrc | 5 + CONTRIBUTING.md | 31 +- package.json | 4 +- shared/ci/linting/Invoke-ShellCheck.ps1 | 218 ++++++++++++ shared/ci/linting/Invoke-YamlLint.ps1 | 4 +- .../tests/linting/Invoke-ShellCheck.Tests.ps1 | 334 ++++++++++++++++++ .../tests/linting/Invoke-YamlLint.Tests.ps1 | 13 +- 10 files changed, 676 insertions(+), 21 deletions(-) create mode 100644 .github/workflows/shellcheck.yml create mode 100644 .shellcheckrc create mode 100644 shared/ci/linting/Invoke-ShellCheck.ps1 create mode 100644 shared/ci/tests/linting/Invoke-ShellCheck.Tests.ps1 diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 82756de2..eba76272 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -147,7 +147,7 @@ jobs: name: Terraform Validation uses: ./.github/workflows/terraform-validation.yml with: - soft-fail: true + soft-fail: false permissions: contents: read @@ -167,7 +167,7 @@ jobs: name: Go Lint uses: ./.github/workflows/go-lint.yml with: - soft-fail: true + soft-fail: false permissions: contents: read @@ -182,6 +182,15 @@ jobs: contents: read id-token: write + # ShellCheck linting for shell scripts + shellcheck: + name: ShellCheck + uses: ./.github/workflows/shellcheck.yml + with: + soft-fail: false + permissions: + contents: read + # CodeQL security analysis codeql-analysis: name: CodeQL Analysis @@ -213,6 +222,7 @@ jobs: - terraform-tests - go-lint - go-tests + - shellcheck - codeql-analysis name: Release Please runs-on: ubuntu-latest diff --git a/.github/workflows/pr-validation.yml b/.github/workflows/pr-validation.yml index 0e9ccabf..e407ebfd 100644 --- a/.github/workflows/pr-validation.yml +++ b/.github/workflows/pr-validation.yml @@ -167,7 +167,7 @@ jobs: name: Terraform Validation uses: ./.github/workflows/terraform-validation.yml with: - soft-fail: true + soft-fail: false changed-files-only: true permissions: contents: read @@ -189,7 +189,7 @@ jobs: name: Go Lint uses: ./.github/workflows/go-lint.yml with: - soft-fail: true + soft-fail: false changed-files-only: true permissions: contents: read @@ -206,6 +206,16 @@ jobs: contents: read id-token: write + # ShellCheck linting for shell scripts + shellcheck: + name: ShellCheck + uses: ./.github/workflows/shellcheck.yml + with: + soft-fail: false + changed-files-only: true + permissions: + contents: read + # CodeQL security analysis codeql-analysis: name: CodeQL Analysis diff --git a/.github/workflows/shellcheck.yml b/.github/workflows/shellcheck.yml new file mode 100644 index 00000000..e6cb0c3b --- /dev/null +++ b/.github/workflows/shellcheck.yml @@ -0,0 +1,60 @@ +name: ShellCheck + +on: + workflow_call: + inputs: + soft-fail: + description: 'Whether to continue on ShellCheck failures' + required: false + type: boolean + default: false + changed-files-only: + description: 'Only lint when shell files changed' + required: false + type: boolean + default: false + +permissions: + contents: read + +defaults: + run: + shell: pwsh + +jobs: + shellcheck: + name: ShellCheck + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - name: Checkout repository + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + persist-credentials: false + fetch-depth: ${{ inputs.changed-files-only && '0' || '1' }} + + - name: Create logs directory + run: New-Item -ItemType Directory -Force -Path logs | Out-Null + + - name: Install ShellCheck + run: sudo apt-get update && sudo apt-get install -y shellcheck + shell: bash + + - name: Run ShellCheck + id: shellcheck + continue-on-error: ${{ inputs.soft-fail }} + run: | + $params = @{} + if ('${{ inputs.changed-files-only }}' -eq 'true') { + $params['ChangedFilesOnly'] = $true + } + ./shared/ci/linting/Invoke-ShellCheck.ps1 @params + + - name: Upload ShellCheck results + if: always() + uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0 + with: + name: shellcheck-results + path: logs/shellcheck-results.json + if-no-files-found: ignore diff --git a/.shellcheckrc b/.shellcheckrc new file mode 100644 index 00000000..9664f354 --- /dev/null +++ b/.shellcheckrc @@ -0,0 +1,5 @@ +# ShellCheck configuration +# Severity: warning (excludes info and style per warnings_strict policy) +shell=bash +severity=warning +external-sources=true diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c177ddcf..160c5f77 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -151,6 +151,23 @@ npm run test:tf # Terraform module tests (no Azure credentials required) For Terraform and shell script validation, see the [Prerequisites](docs/contributing/prerequisites.md#build-and-validation-requirements) guide. +### Warning Policy + +All CI linters enforce warnings-as-errors. PRs that introduce new warnings will not merge. + +| Linter | Enforcement | Configuration | +|----------------------|-------------------|-----------------------------------------------| +| Markdown (lint:md) | Errors block | .markdownlint-cli2.jsonc | +| PowerShell (lint:ps) | Errors + warnings | shared/ci/linting/Invoke-PSScriptAnalyzer.ps1 | +| YAML (lint:yaml) | Errors + warnings | .yamllint.yml | +| Terraform (lint:tf) | Errors block | .tflint.hcl | +| Go (lint:go) | Errors block | .golangci.yml | +| ShellCheck (lint:sh) | Warnings + errors | .shellcheckrc | +| Python (lint:py) | Errors block | pyproject.toml [tool.ruff] | +| Link check | Errors block | .markdownlint-cli2.jsonc | + +To suppress a specific warning locally, use the linter's inline suppression syntax. Do not change CI configuration to suppress warnings globally without team discussion. + ## Updating External Components Reused externally-maintained components (Helm charts, container images, Terraform providers, Python packages, GitHub Actions) require periodic updates for security patches and compatibility. Dependabot automates updates for Python, Terraform, and GitHub Actions ecosystems. Helm charts and container images require manual updates. @@ -313,14 +330,14 @@ Optionally run the RL end-to-end suite to capture regressions. This is good prac Requirements: -| Requirement | Details | -|-------------|---------| -| Azure CLI | `az` must be installed and authenticated. The Azure ML CLI extension must also be available. | -| Azure subscription context | Set `AZURE_SUBSCRIPTION_ID`, or make sure `az account show` resolves to the subscription you want the test to use. | -| Azure workspace context | Set `AZURE_RESOURCE_GROUP` and `AZUREML_WORKSPACE_NAME`, or make sure `terraform output -json` or `infrastructure/terraform/terraform.tfvars` resolves them. | -| Azure ML compute target | For Azure ML validation, the compute target must resolve from `AZUREML_COMPUTE` or Terraform naming and its provisioning state must be `Succeeded`. | +| Requirement | Details | +|----------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| Azure CLI | `az` must be installed and authenticated. The Azure ML CLI extension must also be available. | +| Azure subscription context | Set `AZURE_SUBSCRIPTION_ID`, or make sure `az account show` resolves to the subscription you want the test to use. | +| Azure workspace context | Set `AZURE_RESOURCE_GROUP` and `AZUREML_WORKSPACE_NAME`, or make sure `terraform output -json` or `infrastructure/terraform/terraform.tfvars` resolves them. | +| Azure ML compute target | For Azure ML validation, the compute target must resolve from `AZUREML_COMPUTE` or Terraform naming and its provisioning state must be `Succeeded`. | | OSMO and Kubernetes access | For OSMO validation, `osmo` and `kubectl` must be installed and authenticated, and the target cluster must expose at least one reachable GPU node. Connect the VPN first for private clusters. | -| MLflow access | The Azure ML workspace used by the tests must expose a working MLflow tracking URI because both validation paths assert metrics and parameters after the run completes. | +| MLflow access | The Azure ML workspace used by the tests must expose a working MLflow tracking URI because both validation paths assert metrics and parameters after the run completes. | Run these commands from the repository root: diff --git a/package.json b/package.json index 9b1a020a..e647c1af 100644 --- a/package.json +++ b/package.json @@ -12,12 +12,14 @@ "lint:md": "markdownlint-cli2 \"**/*.md\"", "lint:md:fix": "markdownlint-cli2 \"**/*.md\" --fix", "lint:ps": "pwsh -File shared/ci/linting/Invoke-PSScriptAnalyzer.ps1", + "lint:py": "uvx ruff check .", "lint:links": "pwsh -File shared/ci/linting/Invoke-LinkLanguageCheck.ps1", "lint:go": "pwsh -File shared/ci/linting/Invoke-GoLint.ps1", + "lint:sh": "pwsh -File shared/ci/linting/Invoke-ShellCheck.ps1", "lint:yaml": "pwsh -File shared/ci/linting/Invoke-YamlLint.ps1", "lint:tf": "pwsh -File shared/ci/linting/Invoke-TFLint.ps1", "lint:tf:validate": "pwsh -File shared/ci/linting/Invoke-TerraformValidation.ps1", - "lint:all": "npm run lint:md && npm run lint:ps && npm run lint:links && npm run lint:yaml && npm run lint:tf && npm run lint:go", + "lint:all": "npm run lint:md && npm run lint:ps && npm run lint:links && npm run lint:yaml && npm run lint:tf && npm run lint:go && npm run lint:sh && npm run lint:py", "format:tables": "markdown-table-formatter \"**/*.md\"", "test:ps": "pwsh -File ./shared/ci/tests/Invoke-PesterTests.ps1", "test:tf": "pwsh -File shared/ci/linting/Invoke-TerraformTest.ps1", diff --git a/shared/ci/linting/Invoke-ShellCheck.ps1 b/shared/ci/linting/Invoke-ShellCheck.ps1 new file mode 100644 index 00000000..61534220 --- /dev/null +++ b/shared/ci/linting/Invoke-ShellCheck.ps1 @@ -0,0 +1,218 @@ +#!/usr/bin/env pwsh +# Copyright (c) Microsoft Corporation. +# SPDX-License-Identifier: MIT + +#Requires -Version 7.0 + +<# +.SYNOPSIS + Runs ShellCheck across shell scripts in the repository. +.DESCRIPTION + Discovers .sh files recursively (excluding .venv/, external/, node_modules/, .git/), + runs ShellCheck with JSON output, and writes results to logs/. Reports violations via + CI annotations and generates a GitHub step summary. +.PARAMETER OutputPath + Path for JSON results. Defaults to logs/shellcheck-results.json. +.PARAMETER ChangedFilesOnly + When set, only run lint if shell files have changed. +#> + +[CmdletBinding()] +param( + [string]$OutputPath, + [switch]$ChangedFilesOnly +) + +Set-StrictMode -Version Latest +$ErrorActionPreference = 'Stop' + +Import-Module (Join-Path $PSScriptRoot "Modules/LintingHelpers.psm1") -Force +Import-Module (Join-Path $PSScriptRoot "../../../scripts/lib/Modules/CIHelpers.psm1") -Force + +function Write-EmptyLintResults { + [CmdletBinding()] + param( + [string]$OutputPath, + [string]$SummaryMessage + ) + + $results = @{ + timestamp = (Get-Date -Format 'o') + shellcheck_version = '' + lint_passed = $true + error_count = 0 + warning_count = 0 + summary = @{ + overall_passed = $true + } + } + + $results | ConvertTo-Json -Depth 10 | Out-File -FilePath $OutputPath -Encoding utf8 + Write-Host "Results written to $OutputPath" + + $summaryContent = @( + '### ShellCheck Results' + '' + $SummaryMessage + ) -join "`n" + Write-CIStepSummary -Content $summaryContent + Write-Host $summaryContent +} + +function Invoke-ShellCheckCore { + [CmdletBinding()] + param( + [string]$OutputPath, + [switch]$ChangedFilesOnly + ) + + $repoRoot = & git rev-parse --show-toplevel 2>$null + if (-not $repoRoot) { + $repoRoot = (Get-Item $PSScriptRoot).Parent.Parent.Parent.FullName + } + + if (-not $OutputPath) { $OutputPath = Join-Path $repoRoot 'logs/shellcheck-results.json' } + + $outputDir = Split-Path $OutputPath -Parent + if (-not (Test-Path $outputDir)) { + New-Item -ItemType Directory -Force -Path $outputDir | Out-Null + } + + # Guard: ChangedFilesOnly + if ($ChangedFilesOnly) { + $changedFiles = @(Get-ChangedFilesFromGit -FileExtensions @('*.sh')) + if ($changedFiles.Count -eq 0) { + Write-Host 'No shell files changed — skipping lint' + Write-EmptyLintResults -OutputPath $OutputPath -SummaryMessage 'No shell files changed — skipping lint.' + return 0 + } + } + + # Check shellcheck on PATH + if (-not (Get-Command shellcheck -ErrorAction SilentlyContinue)) { + Write-CIAnnotation -Level Error -Message 'shellcheck not found on PATH. Install via apt-get or brew.' + return 1 + } + + # Capture version + $scVersionOutput = & shellcheck --version 2>$null + $scVersion = if ($scVersionOutput -match 'version:\s*([\d.]+)') { $Matches[1] } else { 'unknown' } + + # Discover .sh files, excluding directories that should not be linted + $excludeDirs = @('.venv', 'external', 'node_modules', '.git', 'docs/docusaurus') + $allShFiles = @(Get-ChildItem -Path $repoRoot -Filter '*.sh' -Recurse -File | Where-Object { + $relativePath = $_.FullName.Substring($repoRoot.Length + 1) + $excluded = $false + foreach ($dir in $excludeDirs) { + if ($relativePath -like "$dir*" -or $relativePath -like "*/$dir/*" -or $relativePath -like "*\$dir\*") { + $excluded = $true + break + } + } + -not $excluded + }) + + if ($ChangedFilesOnly -and $changedFiles) { + $changedSet = [System.Collections.Generic.HashSet[string]]::new( + [StringComparer]::OrdinalIgnoreCase + ) + foreach ($f in $changedFiles) { + $fullPath = Join-Path $repoRoot $f + [void]$changedSet.Add((Resolve-Path $fullPath -ErrorAction SilentlyContinue).Path) + } + $allShFiles = @($allShFiles | Where-Object { $changedSet.Contains($_.FullName) }) + } + + if ($allShFiles.Count -eq 0) { + Write-Host 'No .sh files found to lint' + Write-EmptyLintResults -OutputPath $OutputPath -SummaryMessage 'No `.sh` files found to lint.' + return 0 + } + + Write-Host "Found $($allShFiles.Count) shell file(s) to lint" + + # Run shellcheck with JSON output on all files + $errorCount = 0 + $warningCount = 0 + $allIssues = @() + + foreach ($file in $allShFiles) { + $relativePath = $file.FullName.Substring($repoRoot.Length + 1).Replace('\', '/') + $jsonOutput = & shellcheck --format=json $file.FullName 2>$null + if ($jsonOutput) { + $issues = $jsonOutput | ConvertFrom-Json + foreach ($issue in $issues) { + $level = switch ($issue.level) { + 'error' { 'Error' } + 'warning' { 'Warning' } + default { $issue.level } + } + + if ($level -eq 'Error') { $errorCount++ } + elseif ($level -eq 'Warning') { $warningCount++ } + + Write-CIAnnotation -Level $level -Message "SC$($issue.code): $($issue.message)" ` + -File $relativePath -Line $issue.line -Col $issue.column + + $allIssues += @{ + file = $relativePath + line = $issue.line + column = $issue.column + level = $issue.level + code = $issue.code + message = $issue.message + } + } + } + } + + $lintPassed = ($errorCount -eq 0 -and $warningCount -eq 0) + + $results = @{ + timestamp = (Get-Date -Format 'o') + shellcheck_version = $scVersion + lint_passed = $lintPassed + error_count = $errorCount + warning_count = $warningCount + files_checked = $allShFiles.Count + issues = $allIssues + summary = @{ + overall_passed = $lintPassed + } + } + + $results | ConvertTo-Json -Depth 10 | Out-File -FilePath $OutputPath -Encoding utf8 + Write-Host "Results written to $OutputPath" + + # Step summary + $status = if ($lintPassed) { '✅ Passed' } else { "❌ Failed ($errorCount error(s), $warningCount warning(s))" } + $summaryContent = @( + '### ShellCheck Results' + '' + "**ShellCheck ($scVersion):** $status" + '' + "| Metric | Count |" + "|--------|-------|" + "| Files Checked | $($allShFiles.Count) |" + "| Errors | $errorCount |" + "| Warnings | $warningCount |" + ) -join "`n" + Write-CIStepSummary -Content $summaryContent + Write-Host $summaryContent + + if (-not $lintPassed) { return 1 } else { return 0 } +} + +#region Main Execution +if ($MyInvocation.InvocationName -ne '.') { + try { + $exitCode = Invoke-ShellCheckCore @PSBoundParameters + exit $exitCode + } + catch { + Write-Error -ErrorAction Continue "Invoke-ShellCheck failed: $($_.Exception.Message)" + Write-CIAnnotation -Level Error -Message $_.Exception.Message + exit 1 + } +} +#endregion Main Execution diff --git a/shared/ci/linting/Invoke-YamlLint.ps1 b/shared/ci/linting/Invoke-YamlLint.ps1 index eab66d29..bb2aa0a0 100644 --- a/shared/ci/linting/Invoke-YamlLint.ps1 +++ b/shared/ci/linting/Invoke-YamlLint.ps1 @@ -180,9 +180,9 @@ No workflow files to lint. "@ Write-CIStepSummary -Content $summary - if ($errorCount -gt 0) { + if ($errorCount -gt 0 -or $warningCount -gt 0) { Set-CIEnv -Name "YAML_LINT_FAILED" -Value "true" - Write-CIAnnotation -Message "actionlint found $errorCount error(s). Fix the issues above." -Level Error + Write-CIAnnotation -Message "actionlint found $errorCount error(s) and $warningCount warning(s). Fix the issues above." -Level Error return 1 } diff --git a/shared/ci/tests/linting/Invoke-ShellCheck.Tests.ps1 b/shared/ci/tests/linting/Invoke-ShellCheck.Tests.ps1 new file mode 100644 index 00000000..66ab8c5e --- /dev/null +++ b/shared/ci/tests/linting/Invoke-ShellCheck.Tests.ps1 @@ -0,0 +1,334 @@ +# Copyright (c) Microsoft Corporation. +# SPDX-License-Identifier: MIT + +#Requires -Version 7.0 +#Requires -Modules @{ ModuleName = 'Pester'; ModuleVersion = '5.0' } + +# Stub function for external tool triggers PSUseApprovedVerbs +[Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseApprovedVerbs', '')] +param() + +BeforeAll { + . $PSScriptRoot/../../linting/Invoke-ShellCheck.ps1 + $ErrorActionPreference = 'Continue' + Import-Module (Join-Path $PSScriptRoot '../Mocks/GitMocks.psm1') -Force + function shellcheck { } +} + +Describe 'Invoke-ShellCheckCore' -Tag 'Unit' { + BeforeAll { Save-CIEnvironment } + AfterAll { Restore-CIEnvironment } + + BeforeEach { + $script:MockFiles = Initialize-MockCIEnvironment -Workspace $TestDrive + $script:TestOutputPath = Join-Path $TestDrive 'logs/shellcheck-results.json' + + Mock git { return $TestDrive } -ParameterFilter { $args[0] -eq 'rev-parse' } + Mock Write-CIAnnotation {} + Mock Write-CIStepSummary {} + + Mock shellcheck { + return 'ShellCheck - shell script analysis tool' + } -ParameterFilter { $args[0] -eq '--version' } + + Mock Get-Command { + return @{ Source = '/usr/bin/shellcheck' } + } -ParameterFilter { $Name -eq 'shellcheck' } + } + + AfterEach { + Restore-CIEnvironment + Remove-MockCIFiles -MockFiles $script:MockFiles + } + + Context 'tool availability' { + It 'Returns 1 when shellcheck is not on PATH' { + Mock Get-Command { return $null } -ParameterFilter { $Name -eq 'shellcheck' } + $shDir = Join-Path $TestDrive 'scripts' + New-Item -ItemType Directory -Force -Path $shDir | Out-Null + '#!/bin/bash' | Set-Content (Join-Path $shDir 'test.sh') + + $result = Invoke-ShellCheckCore -OutputPath $script:TestOutputPath + $result | Should -Be 1 + } + + It 'Writes error annotation when shellcheck not found' { + Mock Get-Command { return $null } -ParameterFilter { $Name -eq 'shellcheck' } + $shDir = Join-Path $TestDrive 'scripts' + New-Item -ItemType Directory -Force -Path $shDir | Out-Null + '#!/bin/bash' | Set-Content (Join-Path $shDir 'test.sh') + + Invoke-ShellCheckCore -OutputPath $script:TestOutputPath + Should -Invoke Write-CIAnnotation -ParameterFilter { + $Level -eq 'Error' -and $Message -like '*shellcheck*not found*' + } + } + } + + Context 'no files found' { + It 'Returns 0 when no .sh files exist' { + $result = Invoke-ShellCheckCore -OutputPath $script:TestOutputPath + $result | Should -Be 0 + } + + It 'Writes empty results JSON when no .sh files exist' { + Invoke-ShellCheckCore -OutputPath $script:TestOutputPath + $script:TestOutputPath | Should -Exist + $json = Get-Content $script:TestOutputPath -Raw | ConvertFrom-Json + $json.lint_passed | Should -BeTrue + $json.error_count | Should -Be 0 + $json.warning_count | Should -Be 0 + } + + It 'Writes step summary when no .sh files found' { + Invoke-ShellCheckCore -OutputPath $script:TestOutputPath + Should -Invoke Write-CIStepSummary -Times 1 + } + } + + Context 'ChangedFilesOnly' { + It 'Returns 0 early when no shell files changed' { + Mock Get-ChangedFilesFromGit { return @() } + $result = Invoke-ShellCheckCore -OutputPath $script:TestOutputPath -ChangedFilesOnly + $result | Should -Be 0 + } + + It 'Writes empty results when no shell files changed' { + Mock Get-ChangedFilesFromGit { return @() } + Invoke-ShellCheckCore -OutputPath $script:TestOutputPath -ChangedFilesOnly + $json = Get-Content $script:TestOutputPath -Raw | ConvertFrom-Json + $json.lint_passed | Should -BeTrue + } + + It 'Calls Get-ChangedFilesFromGit with *.sh extension' { + Mock Get-ChangedFilesFromGit { return @() } + Invoke-ShellCheckCore -OutputPath $script:TestOutputPath -ChangedFilesOnly + Should -Invoke Get-ChangedFilesFromGit -Times 1 -ParameterFilter { + $FileExtensions -contains '*.sh' + } + } + + It 'Lints only changed files when ChangedFilesOnly is set' { + $shDir = Join-Path $TestDrive 'scripts' + New-Item -ItemType Directory -Force -Path $shDir | Out-Null + '#!/bin/bash' | Set-Content (Join-Path $shDir 'changed.sh') + '#!/bin/bash' | Set-Content (Join-Path $shDir 'unchanged.sh') + + Mock Get-ChangedFilesFromGit { return @('scripts/changed.sh') } + Mock shellcheck { return $null } -ParameterFilter { $args[0] -eq '--format=json' } + + Invoke-ShellCheckCore -OutputPath $script:TestOutputPath -ChangedFilesOnly + Should -Invoke shellcheck -Times 1 -ParameterFilter { $args[0] -eq '--format=json' } + } + } + + Context 'lint success' { + BeforeEach { + $shDir = Join-Path $TestDrive 'scripts' + New-Item -ItemType Directory -Force -Path $shDir | Out-Null + '#!/bin/bash' | Set-Content (Join-Path $shDir 'clean.sh') + + Mock shellcheck { return $null } -ParameterFilter { $args[0] -eq '--format=json' } + } + + It 'Returns 0 when no issues found' { + $result = Invoke-ShellCheckCore -OutputPath $script:TestOutputPath + $result | Should -Be 0 + } + + It 'Reports lint_passed as true in JSON' { + Invoke-ShellCheckCore -OutputPath $script:TestOutputPath + $json = Get-Content $script:TestOutputPath -Raw | ConvertFrom-Json + $json.lint_passed | Should -BeTrue + } + + It 'Records files_checked count' { + Invoke-ShellCheckCore -OutputPath $script:TestOutputPath + $json = Get-Content $script:TestOutputPath -Raw | ConvertFrom-Json + $json.files_checked | Should -Be 1 + } + + It 'Writes step summary on success' { + Invoke-ShellCheckCore -OutputPath $script:TestOutputPath + Should -Invoke Write-CIStepSummary -Times 1 + } + } + + Context 'error classification' { + BeforeEach { + $shDir = Join-Path $TestDrive 'scripts' + New-Item -ItemType Directory -Force -Path $shDir | Out-Null + '#!/bin/bash' | Set-Content (Join-Path $shDir 'bad.sh') + } + + It 'Returns 1 when errors found' { + Mock shellcheck { + return '[{"file":"bad.sh","line":2,"column":1,"level":"error","code":2086,"message":"Double quote to prevent globbing"}]' + } -ParameterFilter { $args[0] -eq '--format=json' } + + $result = Invoke-ShellCheckCore -OutputPath $script:TestOutputPath + $result | Should -Be 1 + } + + It 'Counts errors correctly in JSON' { + Mock shellcheck { + return '[{"file":"bad.sh","line":2,"column":1,"level":"error","code":2086,"message":"err1"},{"file":"bad.sh","line":3,"column":1,"level":"error","code":2087,"message":"err2"}]' + } -ParameterFilter { $args[0] -eq '--format=json' } + + Invoke-ShellCheckCore -OutputPath $script:TestOutputPath + $json = Get-Content $script:TestOutputPath -Raw | ConvertFrom-Json + $json.error_count | Should -Be 2 + $json.warning_count | Should -Be 0 + } + + It 'Writes error annotations for each issue' { + Mock shellcheck { + return '[{"file":"bad.sh","line":2,"column":1,"level":"error","code":2086,"message":"Double quote"}]' + } -ParameterFilter { $args[0] -eq '--format=json' } + + Invoke-ShellCheckCore -OutputPath $script:TestOutputPath + Should -Invoke Write-CIAnnotation -ParameterFilter { $Level -eq 'Error' } + } + } + + Context 'warning classification (strict mode)' { + BeforeEach { + $shDir = Join-Path $TestDrive 'scripts' + New-Item -ItemType Directory -Force -Path $shDir | Out-Null + '#!/bin/bash' | Set-Content (Join-Path $shDir 'warn.sh') + } + + It 'Returns 1 when only warnings found' { + Mock shellcheck { + return '[{"file":"warn.sh","line":2,"column":1,"level":"warning","code":2034,"message":"var unused"}]' + } -ParameterFilter { $args[0] -eq '--format=json' } + + $result = Invoke-ShellCheckCore -OutputPath $script:TestOutputPath + $result | Should -Be 1 + } + + It 'Reports lint_passed as false when warnings present' { + Mock shellcheck { + return '[{"file":"warn.sh","line":2,"column":1,"level":"warning","code":2034,"message":"var unused"}]' + } -ParameterFilter { $args[0] -eq '--format=json' } + + Invoke-ShellCheckCore -OutputPath $script:TestOutputPath + $json = Get-Content $script:TestOutputPath -Raw | ConvertFrom-Json + $json.lint_passed | Should -BeFalse + } + + It 'Counts warnings correctly in JSON' { + Mock shellcheck { + return '[{"file":"warn.sh","line":2,"column":1,"level":"warning","code":2034,"message":"w1"},{"file":"warn.sh","line":3,"column":1,"level":"warning","code":2035,"message":"w2"}]' + } -ParameterFilter { $args[0] -eq '--format=json' } + + Invoke-ShellCheckCore -OutputPath $script:TestOutputPath + $json = Get-Content $script:TestOutputPath -Raw | ConvertFrom-Json + $json.warning_count | Should -Be 2 + $json.error_count | Should -Be 0 + } + } + + Context 'mixed errors and warnings' { + BeforeEach { + $shDir = Join-Path $TestDrive 'scripts' + New-Item -ItemType Directory -Force -Path $shDir | Out-Null + '#!/bin/bash' | Set-Content (Join-Path $shDir 'mixed.sh') + + Mock shellcheck { + return '[{"file":"mixed.sh","line":2,"column":1,"level":"error","code":2086,"message":"err"},{"file":"mixed.sh","line":3,"column":1,"level":"warning","code":2034,"message":"warn"}]' + } -ParameterFilter { $args[0] -eq '--format=json' } + } + + It 'Returns 1 with mixed issues' { + $result = Invoke-ShellCheckCore -OutputPath $script:TestOutputPath + $result | Should -Be 1 + } + + It 'Counts errors and warnings separately' { + Invoke-ShellCheckCore -OutputPath $script:TestOutputPath + $json = Get-Content $script:TestOutputPath -Raw | ConvertFrom-Json + $json.error_count | Should -Be 1 + $json.warning_count | Should -Be 1 + } + } + + Context 'JSON export structure' { + BeforeEach { + $shDir = Join-Path $TestDrive 'scripts' + New-Item -ItemType Directory -Force -Path $shDir | Out-Null + '#!/bin/bash' | Set-Content (Join-Path $shDir 'test.sh') + + Mock shellcheck { + return '[{"file":"test.sh","line":2,"column":1,"level":"error","code":2086,"message":"err"}]' + } -ParameterFilter { $args[0] -eq '--format=json' } + } + + It 'Contains expected metadata fields' { + Invoke-ShellCheckCore -OutputPath $script:TestOutputPath + $json = Get-Content $script:TestOutputPath -Raw | ConvertFrom-Json + $json.PSObject.Properties.Name | Should -Contain 'timestamp' + $json.PSObject.Properties.Name | Should -Contain 'shellcheck_version' + $json.PSObject.Properties.Name | Should -Contain 'lint_passed' + $json.PSObject.Properties.Name | Should -Contain 'error_count' + $json.PSObject.Properties.Name | Should -Contain 'warning_count' + $json.PSObject.Properties.Name | Should -Contain 'files_checked' + $json.PSObject.Properties.Name | Should -Contain 'issues' + } + + It 'Includes issue details in issues array' { + Invoke-ShellCheckCore -OutputPath $script:TestOutputPath + $json = Get-Content $script:TestOutputPath -Raw | ConvertFrom-Json + $json.issues.Count | Should -Be 1 + $json.issues[0].code | Should -Be 2086 + $json.issues[0].level | Should -Be 'error' + } + + It 'Includes summary.overall_passed field' { + Invoke-ShellCheckCore -OutputPath $script:TestOutputPath + $json = Get-Content $script:TestOutputPath -Raw | ConvertFrom-Json + $json.summary.overall_passed | Should -BeFalse + } + } + + Context 'directory exclusion' { + BeforeEach { + # Create .sh files in excluded directories + foreach ($dir in @('.venv', 'external', 'node_modules', 'docs/docusaurus')) { + $excludedDir = Join-Path $TestDrive $dir + New-Item -ItemType Directory -Force -Path $excludedDir | Out-Null + '#!/bin/bash' | Set-Content (Join-Path $excludedDir 'excluded.sh') + } + } + + It 'Returns 0 when all .sh files are in excluded directories' { + Mock shellcheck { return $null } -ParameterFilter { $args[0] -eq '--format=json' } + $result = Invoke-ShellCheckCore -OutputPath $script:TestOutputPath + $result | Should -Be 0 + } + } + + Context 'output file creation' { + It 'Creates output directory if it does not exist' { + $nestedOutput = Join-Path $TestDrive 'deep/nested/output.json' + Invoke-ShellCheckCore -OutputPath $nestedOutput + Split-Path $nestedOutput -Parent | Should -Exist + } + + It 'Writes valid JSON to output path' { + $shDir = Join-Path $TestDrive 'scripts' + New-Item -ItemType Directory -Force -Path $shDir | Out-Null + '#!/bin/bash' | Set-Content (Join-Path $shDir 'test.sh') + Mock shellcheck { return $null } -ParameterFilter { $args[0] -eq '--format=json' } + + Invoke-ShellCheckCore -OutputPath $script:TestOutputPath + { Get-Content $script:TestOutputPath -Raw | ConvertFrom-Json } | Should -Not -Throw + } + + It 'Uses default output path when not specified' { + Invoke-ShellCheckCore + $defaultPath = Join-Path $TestDrive 'logs/shellcheck-results.json' + $defaultPath | Should -Exist + } + } +} diff --git a/shared/ci/tests/linting/Invoke-YamlLint.Tests.ps1 b/shared/ci/tests/linting/Invoke-YamlLint.Tests.ps1 index 959b7af7..e64fd17a 100644 --- a/shared/ci/tests/linting/Invoke-YamlLint.Tests.ps1 +++ b/shared/ci/tests/linting/Invoke-YamlLint.Tests.ps1 @@ -140,12 +140,12 @@ Describe 'Invoke-YamlLintCore' -Tag 'Unit' { $result | Should -Be 1 } - It 'Returns 0 when only warnings found' { + It 'Returns 1 when only warnings found (strict mode)' { Mock actionlint { return '{"filepath":"test.yml","line":5,"column":3,"kind":"warning","message":"style issue"}' } $result = Invoke-YamlLintCore -OutputPath $script:TestOutputPath - $result | Should -Be 0 + $result | Should -Be 1 } It 'Writes YAML_LINT_FAILED to CI env file on errors' { @@ -158,15 +158,14 @@ Describe 'Invoke-YamlLintCore' -Tag 'Unit' { $envContent | Should -Match 'true' } - It 'Does not write YAML_LINT_FAILED when no errors' { + It 'Writes YAML_LINT_FAILED when only warnings found (strict mode)' { Mock actionlint { return '{"filepath":"test.yml","line":5,"column":3,"kind":"warning","message":"warn"}' } Invoke-YamlLintCore -OutputPath $script:TestOutputPath - $envContent = Get-Content $env:GITHUB_ENV -Raw -ErrorAction SilentlyContinue - if ($envContent) { - $envContent | Should -Not -Match 'YAML_LINT_FAILED' - } + $envContent = Get-Content $env:GITHUB_ENV -Raw + $envContent | Should -Match 'YAML_LINT_FAILED' + $envContent | Should -Match 'true' } It 'Counts errors and warnings separately' { From 020af6cca5ce4ea8262e8e06be6e0948710ec0d4 Mon Sep 17 00:00:00 2001 From: Bill Berry Date: Fri, 3 Apr 2026 17:10:44 -0700 Subject: [PATCH 2/5] fix(build): fail CI on Pester test failures and fix symlink dot-source paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - remove LASTEXITCODE reset that silently masked all test failures in CI - add explicit exit 1 when Pester reports failed tests - fix test dot-source paths from broken scripts/lib symlinks to shared/lib 🐛 - Generated by Copilot --- .github/workflows/pester-tests.yml | 4 ++-- shared/ci/tests/lib/Get-VerifiedDownload.Tests.ps1 | 4 ++-- shared/ci/tests/lib/terraform-outputs.Tests.ps1 | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/pester-tests.yml b/.github/workflows/pester-tests.yml index bb3cd5a7..076f9976 100644 --- a/.github/workflows/pester-tests.yml +++ b/.github/workflows/pester-tests.yml @@ -149,7 +149,6 @@ jobs: $config = & shared/ci/tests/pester.config.ps1 -CI @configParams $config.Run.Exit = $false $result = Invoke-Pester -Configuration $config - $global:LASTEXITCODE = 0 # Re-import CIHelpers — Pester module isolation unloads it during test execution Import-Module $ciHelpersPath -Force @@ -158,6 +157,7 @@ jobs: $env:PESTER_FAILED = 'true' Set-CIOutput -Name 'pester-failed' -Value 'true' Write-Warning "$($result.FailedCount) test(s) failed" + exit 1 } else { Set-CIOutput -Name 'pester-failed' -Value 'false' @@ -165,11 +165,11 @@ jobs: } } catch { - $global:LASTEXITCODE = 0 $env:PESTER_FAILED = 'true' Import-Module $ciHelpersPath -Force -ErrorAction SilentlyContinue Set-CIOutput -Name 'pester-failed' -Value 'true' Write-Error "Pester execution failed: $_" + exit 1 } - name: Upload test results diff --git a/shared/ci/tests/lib/Get-VerifiedDownload.Tests.ps1 b/shared/ci/tests/lib/Get-VerifiedDownload.Tests.ps1 index 54dc7f80..57e8ae88 100644 --- a/shared/ci/tests/lib/Get-VerifiedDownload.Tests.ps1 +++ b/shared/ci/tests/lib/Get-VerifiedDownload.Tests.ps1 @@ -4,7 +4,7 @@ # SPDX-License-Identifier: MIT BeforeAll { - . $PSScriptRoot/../../../../scripts/lib/Get-VerifiedDownload.ps1 + . $PSScriptRoot/../../../lib/Get-VerifiedDownload.ps1 } Describe 'Get-FileHashValue' { It 'Returns uppercase hash string for valid file' { @@ -582,7 +582,7 @@ Describe 'Invoke-VerifiedDownload' { Describe 'Main Execution Block' -Tag 'Integration' { BeforeAll { - $script:scriptPath = Join-Path $PSScriptRoot '../../../../scripts/lib/Get-VerifiedDownload.ps1' + $script:scriptPath = Join-Path $PSScriptRoot '../../../lib/Get-VerifiedDownload.ps1' } It 'Exits 1 when required parameters are missing' { diff --git a/shared/ci/tests/lib/terraform-outputs.Tests.ps1 b/shared/ci/tests/lib/terraform-outputs.Tests.ps1 index 3e510664..2d98c323 100644 --- a/shared/ci/tests/lib/terraform-outputs.Tests.ps1 +++ b/shared/ci/tests/lib/terraform-outputs.Tests.ps1 @@ -11,7 +11,7 @@ #Requires -Modules @{ ModuleName = 'Pester'; ModuleVersion = '5.0' } BeforeAll { - . $PSScriptRoot/../../../../scripts/lib/terraform-outputs.ps1 + . $PSScriptRoot/../../../lib/terraform-outputs.ps1 } Describe 'Read-TerraformOutputs' -Tag 'Unit' { From 6e3aec9c35e0d26f5a04790921ee722a2426bf9e Mon Sep 17 00:00:00 2001 From: Bill Berry Date: Mon, 6 Apr 2026 09:35:58 -0700 Subject: [PATCH 3/5] fix(build): initialize exit code before CLI invocation in link checker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - default exit code to failure before try block prevents undefined variable error under StrictMode when CLI binary is not installed 🐛 - Generated by Copilot --- shared/ci/linting/Markdown-Link-Check.ps1 | 1 + 1 file changed, 1 insertion(+) diff --git a/shared/ci/linting/Markdown-Link-Check.ps1 b/shared/ci/linting/Markdown-Link-Check.ps1 index 538f48ec..6b1b78b9 100644 --- a/shared/ci/linting/Markdown-Link-Check.ps1 +++ b/shared/ci/linting/Markdown-Link-Check.ps1 @@ -194,6 +194,7 @@ function Invoke-MarkdownLinkCheckCore { Write-Output "Checking $relative" $xmlFile = [System.IO.Path]::GetTempFileName() + '.xml' + $exitCode = 1 try { $commandArgs = $baseArguments + @($relative, '--reporters', 'default,junit', '--junit-output', $xmlFile) $output = & $cli @commandArgs 2>&1 From 7f0844ee9797e042e1d900f1c966aaebabe3e805 Mon Sep 17 00:00:00 2001 From: Bill Berry Date: Tue, 7 Apr 2026 10:54:09 -0700 Subject: [PATCH 4/5] docs: fix stale shared/ci path reference in CONTRIBUTING.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - update PSScriptAnalyzer path from shared/ci/linting/ to scripts/linting/ 📝 - Generated by Copilot --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 160c5f77..9f022d94 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -158,7 +158,7 @@ All CI linters enforce warnings-as-errors. PRs that introduce new warnings will | Linter | Enforcement | Configuration | |----------------------|-------------------|-----------------------------------------------| | Markdown (lint:md) | Errors block | .markdownlint-cli2.jsonc | -| PowerShell (lint:ps) | Errors + warnings | shared/ci/linting/Invoke-PSScriptAnalyzer.ps1 | +| PowerShell (lint:ps) | Errors + warnings | scripts/linting/Invoke-PSScriptAnalyzer.ps1 | | YAML (lint:yaml) | Errors + warnings | .yamllint.yml | | Terraform (lint:tf) | Errors block | .tflint.hcl | | Go (lint:go) | Errors block | .golangci.yml | From 282847ae78e175d8b208a29ad3cbc3f55e185192 Mon Sep 17 00:00:00 2001 From: Bill Berry Date: Tue, 7 Apr 2026 10:56:21 -0700 Subject: [PATCH 5/5] fix(scripts): correct stale CIHelpers import path in Invoke-ShellCheck.ps1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - update relative path from ../../../scripts/lib/ to ../lib/ after shared/ci rename 🔧 - Generated by Copilot --- scripts/linting/Invoke-ShellCheck.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/linting/Invoke-ShellCheck.ps1 b/scripts/linting/Invoke-ShellCheck.ps1 index 613e2fc2..404b0d01 100644 --- a/scripts/linting/Invoke-ShellCheck.ps1 +++ b/scripts/linting/Invoke-ShellCheck.ps1 @@ -27,7 +27,7 @@ Set-StrictMode -Version Latest $ErrorActionPreference = 'Stop' Import-Module (Join-Path $PSScriptRoot "Modules/LintingHelpers.psm1") -Force -Import-Module (Join-Path $PSScriptRoot "../../../scripts/lib/Modules/CIHelpers.psm1") -Force +Import-Module (Join-Path $PSScriptRoot "../lib/Modules/CIHelpers.psm1") -Force function Write-EmptyLintResults { [CmdletBinding()]