From 8757646ae84d44690eb551a400259efffd4bd4ff Mon Sep 17 00:00:00 2001 From: Daniel Jurek Date: Fri, 14 Nov 2025 22:25:09 +0000 Subject: [PATCH 1/5] Add cspell upgrade check in check-spelling.yml --- .../templates/steps/check-spelling.yml | 99 ++++++++++++++++++- 1 file changed, 95 insertions(+), 4 deletions(-) diff --git a/eng/common/pipelines/templates/steps/check-spelling.yml b/eng/common/pipelines/templates/steps/check-spelling.yml index 8d7a716cbdfc..c4bb90a2cde9 100644 --- a/eng/common/pipelines/templates/steps/check-spelling.yml +++ b/eng/common/pipelines/templates/steps/check-spelling.yml @@ -2,16 +2,27 @@ # and some ref (branch, tag, etc.) or commit hash. Only runs on PRs. # ContinueOnError - true: Pipeline warns on spelling error # false: Pipeline fails on spelling error -# TargetBranch - Target ref (e.g. main) to compare to create file change -# list. # CspellConfigPath - Path to cspell.json config location # # This check recognizes the setting of variable "Skip.SpellCheck" # if set to 'true', spellchecking will not be invoked. parameters: - ContinueOnError: true - CspellConfigPath: ./.vscode/cspell.json + - name: ContinueOnError + type: string + default: true + - name: CspellConfigPath + type: string + default: ./.vscode/cspell.json + - name: EnableCspellUpgradeVerification + type: boolean + default: false + - name: CspellUpgradePublicApiCheckoutExpression + type: string + default: /sdk/** + - name: CspellUpgradeArtifactDirectory + type: string + default: $(Build.ArtifactStagingDirectory)/spelling-upgrade-check steps: - ${{ if eq(variables['Build.Reason'], 'PullRequest') }}: @@ -26,3 +37,83 @@ steps: -CspellConfigPath ${{ parameters.CspellConfigPath }} -ExitWithError:(!$${{ parameters.ContinueOnError }}) pwsh: true + + - ${{ if eq(parameters.EnableCspellUpgradeVerification, true) }}: + # If the repo has a spell-check-public-api.ps1 script and there is a change + # to the cspell package-lock.json, verify the upgrade. + - pwsh: | + if ('true' -eq '$(Skip.SpellCheck)') { + Write-Host "Spell check is skipped in this build. Skipping cspell upgrade verification." + Write-Host "##vso[task.setvariable variable=RunCspellUpgradeVerification]false" + exit 0 + } + $publicApiCheckExists = Test-Path 'eng/scripts/spell-check-public-api.ps1' + if (!$publicApiCheckExists) { + Write-Host "Public API spell check script not found. Skipping upgrade checks." + } + Write-Host "Changed Files:" + ./eng/common/scripts/Generate-PR-Diff.ps1 ` + -TargetPath '.' ` + -ArtifactPath '$(ArtifactStagingDirectory)' + $changedFiles = (Get-Content '$(ArtifactStagingDirectory)/diff.json' | ConvertFrom-Json).ChangedFiles + Write-Host $changedFiles + + if ($changedFiles -contains 'eng/common/spelling/package-lock.json') { + Write-Host "Detected change to cspell package-lock.json. Setting variables to run cspell upgrade verification." + New-Item -ItemType Directory -Path '${{ parameters.CspellUpgradeArtifactDirectory }}' -Force | Out-Null + Write-Host "##vso[task.setvariable variable=RunCspellUpgradeVerification]true" + } else { + Write-Host "No changes to cspell package-lock.json detected." + Write-Host "##vso[task.setvariable variable=RunCspellUpgradeVerification]false" + } + displayName: Determine if cspell upgrade verification is needed + + # Using the checkout task interferes with the job's existing checkout. + # sparse-checkout.yml cannot be used here because it doesn't use runtime + # conditions to determine if it should run + - pwsh: | + git clone ` + --no-checkout ` + --filter=tree:0 ` + --branch $(System.PullRequest.TargetBranch) ` + $(Build.Repository.Uri) ` + $(Pipeline.Workspace)/public-api-before + + Set-Location $(Pipeline.Workspace)/public-api-before + git sparse-checkout init + git sparse-checkout set --no-cone '/*' '!/*/' '/eng' '/.vscode' '${{ parameters.CspellUpgradePublicApiCheckoutExpression }}' + git checkout $(System.PullRequest.TargetBranch) + condition: and(succeeded(), eq(variables['RunCspellUpgradeVerification'], 'true')) + displayName: Sparse checkout before state of public API surface area + + - pwsh: | + ./eng/scripts/spell-check-public-api.ps1 > '${{ parameters.CspellUpgradeArtifactDirectory }}/spelling-after.txt' + Get-Content '${{ parameters.CspellUpgradeArtifactDirectory }}/spelling-after.txt' + condition: and(succeeded(), eq(variables['RunCspellUpgradeVerification'], 'true')) + displayName: Get public API spelling errors after cspell upgrade + # It's possible that cspell found errors, don't fail the build + ignoreLASTEXITCODE: true + + - pwsh: | + ./eng/scripts/spell-check-public-api.ps1 > '${{ parameters.CspellUpgradeArtifactDirectory }}/spelling-before.txt' + Get-Content '${{ parameters.CspellUpgradeArtifactDirectory }}/spelling-before.txt' + condition: and(succeeded(), eq(variables['RunCspellUpgradeVerification'], 'true')) + displayName: Get public API spelling errors before cspell upgrade + # It's possible that cspell found errors, don't fail the build + ignoreLASTEXITCODE: true + workingDirectory: $(Pipeline.Workspace)/public-api-before + + # On Linux the diff command exits with a nonzero exit code if there is a + # diff. This step will fail if there are differences. + - pwsh: | + diff --unified ` + ${{ parameters.CspellUpgradeArtifactDirectory }}/spelling-before.txt ` + ${{ parameters.CspellUpgradeArtifactDirectory }}/spelling-after.txt | Tee-Object -FilePath ${{ parameters.CspellUpgradeArtifactDirectory }}/spelling-diff.txt + condition: and(succeeded(), eq(variables['RunCspellUpgradeVerification'], 'true')) + displayName: Compare public API spelling errors before and after cspell upgrade + + - template: /eng/common/pipelines/templates/steps/publish-1es-artifact.yml + parameters: + ArtifactName: cspell-upgrade-check + ArtifactPath: ${{ parameters.CspellUpgradeArtifactDirectory }} + CustomCondition: and(succeededOrFailed(), eq(variables['RunCspellUpgradeVerification'], 'true')) From 108d67fb7a977e4e654e053b8dc114fba6782866 Mon Sep 17 00:00:00 2001 From: Daniel Jurek Date: Fri, 14 Nov 2025 23:32:45 +0000 Subject: [PATCH 2/5] Don't diff against a baseline. Assume spelling is correct before upgrade. --- .../templates/steps/check-spelling.yml | 72 +++---------------- 1 file changed, 8 insertions(+), 64 deletions(-) diff --git a/eng/common/pipelines/templates/steps/check-spelling.yml b/eng/common/pipelines/templates/steps/check-spelling.yml index c4bb90a2cde9..b08e5e3afa14 100644 --- a/eng/common/pipelines/templates/steps/check-spelling.yml +++ b/eng/common/pipelines/templates/steps/check-spelling.yml @@ -9,7 +9,7 @@ parameters: - name: ContinueOnError - type: string + type: boolean default: true - name: CspellConfigPath type: string @@ -17,12 +17,6 @@ parameters: - name: EnableCspellUpgradeVerification type: boolean default: false - - name: CspellUpgradePublicApiCheckoutExpression - type: string - default: /sdk/** - - name: CspellUpgradeArtifactDirectory - type: string - default: $(Build.ArtifactStagingDirectory)/spelling-upgrade-check steps: - ${{ if eq(variables['Build.Reason'], 'PullRequest') }}: @@ -42,7 +36,7 @@ steps: # If the repo has a spell-check-public-api.ps1 script and there is a change # to the cspell package-lock.json, verify the upgrade. - pwsh: | - if ('true' -eq '$(Skip.SpellCheck)') { + if ('true' -eq '$(Skip.SpellCheck)') { Write-Host "Spell check is skipped in this build. Skipping cspell upgrade verification." Write-Host "##vso[task.setvariable variable=RunCspellUpgradeVerification]false" exit 0 @@ -50,6 +44,7 @@ steps: $publicApiCheckExists = Test-Path 'eng/scripts/spell-check-public-api.ps1' if (!$publicApiCheckExists) { Write-Host "Public API spell check script not found. Skipping upgrade checks." + exit 0 } Write-Host "Changed Files:" ./eng/common/scripts/Generate-PR-Diff.ps1 ` @@ -58,62 +53,11 @@ steps: $changedFiles = (Get-Content '$(ArtifactStagingDirectory)/diff.json' | ConvertFrom-Json).ChangedFiles Write-Host $changedFiles - if ($changedFiles -contains 'eng/common/spelling/package-lock.json') { - Write-Host "Detected change to cspell package-lock.json. Setting variables to run cspell upgrade verification." - New-Item -ItemType Directory -Path '${{ parameters.CspellUpgradeArtifactDirectory }}' -Force | Out-Null - Write-Host "##vso[task.setvariable variable=RunCspellUpgradeVerification]true" - } else { + if ($changedFiles -notcontains 'eng/common/spelling/package-lock.json') { Write-Host "No changes to cspell package-lock.json detected." - Write-Host "##vso[task.setvariable variable=RunCspellUpgradeVerification]false" + exit 0 } - displayName: Determine if cspell upgrade verification is needed - - # Using the checkout task interferes with the job's existing checkout. - # sparse-checkout.yml cannot be used here because it doesn't use runtime - # conditions to determine if it should run - - pwsh: | - git clone ` - --no-checkout ` - --filter=tree:0 ` - --branch $(System.PullRequest.TargetBranch) ` - $(Build.Repository.Uri) ` - $(Pipeline.Workspace)/public-api-before - - Set-Location $(Pipeline.Workspace)/public-api-before - git sparse-checkout init - git sparse-checkout set --no-cone '/*' '!/*/' '/eng' '/.vscode' '${{ parameters.CspellUpgradePublicApiCheckoutExpression }}' - git checkout $(System.PullRequest.TargetBranch) - condition: and(succeeded(), eq(variables['RunCspellUpgradeVerification'], 'true')) - displayName: Sparse checkout before state of public API surface area - - - pwsh: | - ./eng/scripts/spell-check-public-api.ps1 > '${{ parameters.CspellUpgradeArtifactDirectory }}/spelling-after.txt' - Get-Content '${{ parameters.CspellUpgradeArtifactDirectory }}/spelling-after.txt' - condition: and(succeeded(), eq(variables['RunCspellUpgradeVerification'], 'true')) - displayName: Get public API spelling errors after cspell upgrade - # It's possible that cspell found errors, don't fail the build - ignoreLASTEXITCODE: true - - - pwsh: | - ./eng/scripts/spell-check-public-api.ps1 > '${{ parameters.CspellUpgradeArtifactDirectory }}/spelling-before.txt' - Get-Content '${{ parameters.CspellUpgradeArtifactDirectory }}/spelling-before.txt' - condition: and(succeeded(), eq(variables['RunCspellUpgradeVerification'], 'true')) - displayName: Get public API spelling errors before cspell upgrade - # It's possible that cspell found errors, don't fail the build - ignoreLASTEXITCODE: true - workingDirectory: $(Pipeline.Workspace)/public-api-before - - # On Linux the diff command exits with a nonzero exit code if there is a - # diff. This step will fail if there are differences. - - pwsh: | - diff --unified ` - ${{ parameters.CspellUpgradeArtifactDirectory }}/spelling-before.txt ` - ${{ parameters.CspellUpgradeArtifactDirectory }}/spelling-after.txt | Tee-Object -FilePath ${{ parameters.CspellUpgradeArtifactDirectory }}/spelling-diff.txt - condition: and(succeeded(), eq(variables['RunCspellUpgradeVerification'], 'true')) - displayName: Compare public API spelling errors before and after cspell upgrade - - template: /eng/common/pipelines/templates/steps/publish-1es-artifact.yml - parameters: - ArtifactName: cspell-upgrade-check - ArtifactPath: ${{ parameters.CspellUpgradeArtifactDirectory }} - CustomCondition: and(succeededOrFailed(), eq(variables['RunCspellUpgradeVerification'], 'true')) + Write-Host "Detected change to cspell package-lock.json. Running upgrade verification." + ./eng/scripts/spell-check-public-api.ps1 + displayName: Verify cspell upgrade From 6a35cab666f1d0b85f803ebf7e124e2929528154 Mon Sep 17 00:00:00 2001 From: Daniel Jurek Date: Mon, 17 Nov 2025 21:40:25 +0000 Subject: [PATCH 3/5] Review feedback --- .../templates/steps/check-spelling.yml | 48 +++++++------------ 1 file changed, 18 insertions(+), 30 deletions(-) diff --git a/eng/common/pipelines/templates/steps/check-spelling.yml b/eng/common/pipelines/templates/steps/check-spelling.yml index b08e5e3afa14..6a486569e7d3 100644 --- a/eng/common/pipelines/templates/steps/check-spelling.yml +++ b/eng/common/pipelines/templates/steps/check-spelling.yml @@ -14,9 +14,9 @@ parameters: - name: CspellConfigPath type: string default: ./.vscode/cspell.json - - name: EnableCspellUpgradeVerification - type: boolean - default: false + - name: ScriptToValidateUpgrade + type: string + default: '' steps: - ${{ if eq(variables['Build.Reason'], 'PullRequest') }}: @@ -32,32 +32,20 @@ steps: -ExitWithError:(!$${{ parameters.ContinueOnError }}) pwsh: true - - ${{ if eq(parameters.EnableCspellUpgradeVerification, true) }}: - # If the repo has a spell-check-public-api.ps1 script and there is a change - # to the cspell package-lock.json, verify the upgrade. - - pwsh: | - if ('true' -eq '$(Skip.SpellCheck)') { - Write-Host "Spell check is skipped in this build. Skipping cspell upgrade verification." - Write-Host "##vso[task.setvariable variable=RunCspellUpgradeVerification]false" - exit 0 - } - $publicApiCheckExists = Test-Path 'eng/scripts/spell-check-public-api.ps1' - if (!$publicApiCheckExists) { - Write-Host "Public API spell check script not found. Skipping upgrade checks." - exit 0 - } - Write-Host "Changed Files:" - ./eng/common/scripts/Generate-PR-Diff.ps1 ` - -TargetPath '.' ` - -ArtifactPath '$(ArtifactStagingDirectory)' - $changedFiles = (Get-Content '$(ArtifactStagingDirectory)/diff.json' | ConvertFrom-Json).ChangedFiles - Write-Host $changedFiles + - pwsh: | + $changedFiles = ./eng/common/scripts/get-changedfiles.ps1 - if ($changedFiles -notcontains 'eng/common/spelling/package-lock.json') { - Write-Host "No changes to cspell package-lock.json detected." - exit 0 - } + if ($changedFiles -notcontains 'eng/common/spelling/package-lock.json') { + Write-Host "No changes to cspell package-lock.json detected." + exit 0 + } - Write-Host "Detected change to cspell package-lock.json. Running upgrade verification." - ./eng/scripts/spell-check-public-api.ps1 - displayName: Verify cspell upgrade + Write-Host "Detected change to cspell package-lock.json. Running upgrade verification." + & '${{ parameters.ScriptToValidateUpgrade }}' + displayName: Verify cspell upgrade + condition: >- + and( + succeeded(), + ne('true', variables['Skip.SpellCheck']), + ne('', parameters.ScriptToValidateUpgrade) + ) From 7e8254ee44c594ca50658492329a8b277dbcc466 Mon Sep 17 00:00:00 2001 From: Daniel Jurek Date: Mon, 17 Nov 2025 21:42:52 +0000 Subject: [PATCH 4/5] Comment --- eng/common/pipelines/templates/steps/check-spelling.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/eng/common/pipelines/templates/steps/check-spelling.yml b/eng/common/pipelines/templates/steps/check-spelling.yml index 6a486569e7d3..9cfeee2d9759 100644 --- a/eng/common/pipelines/templates/steps/check-spelling.yml +++ b/eng/common/pipelines/templates/steps/check-spelling.yml @@ -4,6 +4,13 @@ # false: Pipeline fails on spelling error # CspellConfigPath - Path to cspell.json config location # +# ScriptToValidateUpgrade - Optional script to validate cspell upgrade. This +# is invoked only if package-lock.json for cspell is +# changed. This script should exit with a nonzero exit +# code if the upgrade is invalid. Upgrade check should +# check for errors which would prevent release (i.e. +# public API surface). +# # This check recognizes the setting of variable "Skip.SpellCheck" # if set to 'true', spellchecking will not be invoked. From 70b8dca9b18db5235bf96ac41d929f4cc1c807e9 Mon Sep 17 00:00:00 2001 From: Daniel Jurek Date: Tue, 18 Nov 2025 23:36:06 +0000 Subject: [PATCH 5/5] Review feedback: don't clutter steps unless upgrade script param is used --- .../templates/steps/check-spelling.yml | 27 ++++++++----------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/eng/common/pipelines/templates/steps/check-spelling.yml b/eng/common/pipelines/templates/steps/check-spelling.yml index 9cfeee2d9759..d5bfff5ea2d4 100644 --- a/eng/common/pipelines/templates/steps/check-spelling.yml +++ b/eng/common/pipelines/templates/steps/check-spelling.yml @@ -38,21 +38,16 @@ steps: -CspellConfigPath ${{ parameters.CspellConfigPath }} -ExitWithError:(!$${{ parameters.ContinueOnError }}) pwsh: true + - ${{ if ne('', parameters.ScriptToValidateUpgrade) }}: + - pwsh: | + $changedFiles = ./eng/common/scripts/get-changedfiles.ps1 - - pwsh: | - $changedFiles = ./eng/common/scripts/get-changedfiles.ps1 + if ($changedFiles -notcontains 'eng/common/spelling/package-lock.json') { + Write-Host "No changes to cspell package-lock.json detected." + exit 0 + } - if ($changedFiles -notcontains 'eng/common/spelling/package-lock.json') { - Write-Host "No changes to cspell package-lock.json detected." - exit 0 - } - - Write-Host "Detected change to cspell package-lock.json. Running upgrade verification." - & '${{ parameters.ScriptToValidateUpgrade }}' - displayName: Verify cspell upgrade - condition: >- - and( - succeeded(), - ne('true', variables['Skip.SpellCheck']), - ne('', parameters.ScriptToValidateUpgrade) - ) + Write-Host "Detected change to cspell package-lock.json. Running upgrade verification." + & '${{ parameters.ScriptToValidateUpgrade }}' + displayName: Verify cspell upgrade + condition: and(succeeded(), ne('true', variables['Skip.SpellCheck']))