Update staging-lint-check workflow to block PR merge upon failure #823
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,7 +14,7 @@ | |
| import { existsSync, mkdirSync, readdirSync, writeFileSync } from "fs"; | ||
| import { basename, dirname, join, relative } from "path"; | ||
| import { fileURLToPath } from "url"; | ||
| import { execNpmExec, isExecError } from "../../shared/src/exec.js"; | ||
|
|
||
| const __dirname = dirname(fileURLToPath(import.meta.url)); | ||
|
|
||
|
|
@@ -34,37 +34,16 @@ export function extractRulesFromLabels(labels) { | |
| } | ||
|
|
||
| /** | ||
| * Extract rule names from GitHub context | ||
| * Extracts rules from PR labels with test- prefix | ||
| * @param {import('@actions/github-script').AsyncFunctionArguments['context']} context - GitHub Actions context object | ||
| * @returns {string[]} - Array of unique rule names | ||
| */ | ||
| export function extractRuleNames(context) { | ||
| const pr = context.payload.pull_request; | ||
| if (!pr) return []; | ||
|
|
||
| return extractRulesFromLabels(pr.labels ?? []); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -213,12 +192,118 @@ function parseMessages(stdout, stderr) { | |
| return msgs; | ||
| } | ||
|
|
||
| /** | ||
| * Check if the PR has a specific label | ||
| * Reads directly from the event payload — no GitHub API calls needed. | ||
| * @param {import('@actions/github-script').AsyncFunctionArguments['context']} context | ||
| * @param {string} labelName | ||
| * @returns {boolean} | ||
| */ | ||
| export function hasLabel(context, labelName) { | ||
| const labels = context.payload.pull_request?.labels ?? []; | ||
| return labels.some( | ||
| (/** @type {string | {name: string}} */ l) => | ||
| (typeof l === "object" ? l.name : l).toLowerCase() === labelName.toLowerCase(), | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Evaluate blocking conditions after validation completes. | ||
| * Triggers core.setFailed() when merge should be blocked. | ||
| * | ||
| * Blocking scenarios: | ||
| * 1. Command failures — AutoRest or script crashes | ||
| * 2. Validation errors without "errors-acknowledged" label | ||
| * | ||
|
Comment on lines
+214
to
+217
There was a problem hiding this comment. The numbering in the “Blocking scenarios” comment doesn’t match the numbering used in the code below (comment mentions scenarios 1–2, while the implementation refers to “Scenario 2” and “Scenario 3”). This makes the gating logic harder to follow. Align the comment and the inline scenario labels so they refer to the same set of scenarios. |
||
| * @param {object} params | ||
| * @param {import('@actions/github-script').AsyncFunctionArguments['context']} params.context | ||
| * @param {import('@actions/github-script').AsyncFunctionArguments['core']} params.core | ||
| * @param {{ specs: number, errors: number, warnings: number, commandFailures: number }} params.result | ||
| */ | ||
| export function checkBlockingConditions({ context, core, result }) { | ||
| const { errors, warnings, commandFailures } = result; | ||
|
|
||
| // Scenario 2: Command failures always block | ||
| if (commandFailures > 0) { | ||
| core.setFailed( | ||
| `Pipeline execution failed with ${commandFailures} command failure(s). ` + | ||
| "Review the logs and linter-findings artifact for details.", | ||
| ); | ||
| return; | ||
| } | ||
|
|
||
| // Scenario 3: Validation errors block unless acknowledged | ||
| if (errors > 0) { | ||
| if (hasLabel(context, ERRORS_ACKNOWLEDGED_LABEL)) { | ||
| core.warning( | ||
| `Found ${errors} validation error(s) and ${warnings} warning(s). ` + | ||
| `Proceeding because "${ERRORS_ACKNOWLEDGED_LABEL}" label is present. ` + | ||
| "Reviewer approval is still required.", | ||
| ); | ||
| } else { | ||
| core.setFailed( | ||
| `Found ${errors} validation error(s). Action required:\n` + | ||
| " 1. Review the errors in the linter-findings artifact\n" + | ||
| ` 2. Add the "${ERRORS_ACKNOWLEDGED_LABEL}" label to this PR to confirm you have reviewed them\n` + | ||
| " 3. The workflow will automatically re-run when the label is added", | ||
| ); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** Label name used to acknowledge validation errors on a PR. */ | ||
| export const ERRORS_ACKNOWLEDGED_LABEL = "errors-acknowledged"; | ||
|
|
||
| /** | ||
| * Regex pattern matching linter rule source file paths. | ||
| * Covers both spectral and native rule paths. | ||
| */ | ||
| export const RULE_FILE_PATTERN = | ||
| /^packages\/rulesets\/src\/spectral\/(az-arm|az-common|az-dataplane|functions\/.+|utils)\.ts$|^packages\/rulesets\/src\/native\/(legacyRules|functions|rulesets|utilities)\/.+\.ts$/; | ||
|
|
||
| /** | ||
| * Detect whether any linter rule source files were changed in the current PR. | ||
| * Uses the GitHub REST API (pulls.listFiles) via the Octokit client provided by | ||
| * actions/github-script | ||
| * | ||
| * @param {import('@actions/github-script').AsyncFunctionArguments['github']} github - Octokit client | ||
| * @param {import('@actions/github-script').AsyncFunctionArguments['context']} context - GitHub Actions context | ||
| * @returns {Promise<boolean>} - true if rule files were changed | ||
| */ | ||
| export async function detectRuleChanges(github, context) { | ||
| const pr = context.payload.pull_request; | ||
| if (!pr) return false; | ||
|
|
||
| // Paginate to handle PRs with many changed files | ||
| const files = await github.paginate(github.rest.pulls.listFiles, { | ||
|
AkhilaIlla marked this conversation as resolved.
|
||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| pull_number: pr.number, | ||
| per_page: 100, | ||
| }); | ||
|
|
||
| const changedRuleFiles = files.filter( | ||
| (f) => (f.status === "added" || f.status === "modified") && RULE_FILE_PATTERN.test(f.filename), | ||
| ); | ||
|
|
||
| if (changedRuleFiles.length > 0) { | ||
| console.log("Detected rule file changes:"); | ||
| changedRuleFiles.forEach((f) => console.log(` ${f.status}\t${f.filename}`)); | ||
| return true; | ||
| } | ||
|
|
||
| console.log("No rule file changes detected"); | ||
| return false; | ||
| } | ||
|
|
||
| /** | ||
| * Main execution function for GitHub Actions | ||
| * @param {import('@actions/github-script').AsyncFunctionArguments} AsyncFunctionArguments | ||
| */ | ||
| export default async function runInGitHubActions({ github, context, core }) { | ||
| try { | ||
| const ruleChangesDetected = await detectRuleChanges(github, context); | ||
|
|
||
| // Extract rule names from PR | ||
| const selectedRules = extractRuleNames(context); | ||
|
|
||
|
|
@@ -227,7 +312,16 @@ export async function runInGitHubActions({ context, core }) { | |
| ); | ||
|
|
||
| if (selectedRules.length === 0) { | ||
| // Scenario 1: Rule changes without test rules | ||
| if (ruleChangesDetected) { | ||
| core.setFailed( | ||
| "Rule changes detected but no test rules specified. " + | ||
| "Add test-<RuleName> labels to this PR.", | ||
| ); | ||
| return; | ||
| } | ||
|
|
||
| console.log("No linter-related changes found in the PR. Exiting with empty findings file."); | ||
| // Create empty output file | ||
| const outputFile = join( | ||
| process.env.GITHUB_WORKSPACE || "", | ||
|
|
@@ -237,12 +331,17 @@ export async function runInGitHubActions({ context, core }) { | |
| mkdirSync(dirname(outputFile), { recursive: true }); | ||
| writeFileSync( | ||
| outputFile, | ||
| "INFO | Runner | summary | Files scanned: 0, Errors: 0, Warnings: 0, Command failures: 0\n", | ||
| ); | ||
| return; | ||
| } | ||
|
|
||
| const result = await runValidation(selectedRules, process.env, core); | ||
|
|
||
| // Evaluate blocking conditions (scenarios 2 & 3) | ||
| checkBlockingConditions({ context, core, result }); | ||
|
|
||
| return result; | ||
| } catch (error) { | ||
| const errorMessage = error instanceof Error ? error.message : String(error); | ||
| core.setFailed(`Script execution failed: ${errorMessage}`); | ||
|
|
@@ -255,6 +354,7 @@ export async function runInGitHubActions({ context, core }) { | |
| * @param {string[]} selectedRules - Array of rule names to validate | ||
| * @param {any} env - Environment variables object | ||
| * @param {import('@actions/github-script').AsyncFunctionArguments['core'] | null} core - GitHub Actions core object (optional) | ||
| * @returns {Promise<{ specs: number, errors: number, warnings: number, commandFailures: number }>} | ||
| */ | ||
| export async function runValidation(selectedRules, env, core = null) { | ||
| const repoRoot = env.GITHUB_WORKSPACE || process.cwd(); | ||
|
|
@@ -281,7 +381,7 @@ export async function runValidation(selectedRules, env, core = null) { | |
| } else { | ||
| console.error(`ERROR | ${message}`); | ||
| } | ||
| return { specs: 0, errors: 0, warnings: 0, commandFailures: 0 }; | ||
| } | ||
| if (!specs.length) { | ||
| const message = "No matching spec files found"; | ||
|
|
@@ -294,14 +394,15 @@ export async function runValidation(selectedRules, env, core = null) { | |
| mkdirSync(dirname(outputFile), { recursive: true }); | ||
| writeFileSync( | ||
| outputFile, | ||
| "INFO | Runner | summary | Files scanned: 0, Errors: 0, Warnings: 0, Command failures: 0\n", | ||
| ); | ||
| return { specs: 0, errors: 0, warnings: 0, commandFailures: 0 }; | ||
| } | ||
|
|
||
| console.log(`Processing ${specs.length} file(s)`); | ||
| let errors = 0; | ||
| let warnings = 0; | ||
| let commandFailures = 0; | ||
| const outLines = []; | ||
| const seenErrors = new Set(); // Track unique errors to avoid duplicates from $ref resolution | ||
|
|
||
|
|
@@ -311,7 +412,16 @@ export async function runValidation(selectedRules, env, core = null) { | |
| try { | ||
| res = await runAutorest(spec, specRoot, selectedRules, repoRoot); | ||
| } catch (error) { | ||
| commandFailures++; | ||
| const specRelPath = relative(specRoot, spec).replace(/\\/g, "/"); | ||
| const errorMessage = error instanceof Error ? error.message : String(error); | ||
| console.log(`Failed ${spec}: ${errorMessage}`); | ||
| if (isExecError(error)) { | ||
| if (error.stdout) console.log(` stdout: ${error.stdout}`); | ||
| if (error.stderr) console.log(` stderr: ${error.stderr}`); | ||
| if (error.code !== undefined) console.log(` exit code: ${error.code}`); | ||
| } | ||
| outLines.push(`ERROR | CommandFailed | ${specRelPath} | ${errorMessage}`); | ||
| continue; | ||
| } | ||
|
|
||
|
|
@@ -390,22 +500,12 @@ export async function runValidation(selectedRules, env, core = null) { | |
| } | ||
|
|
||
| // Output results | ||
| const summary = `INFO | Runner | summary | Files scanned: ${specs.length}, Errors: ${errors}, Warnings: ${warnings}, Command failures: ${commandFailures}`; | ||
| console.log(summary); | ||
|
|
||
| // Write output file | ||
| mkdirSync(dirname(outputFile), { recursive: true }); | ||
| writeFileSync(outputFile, outLines.concat([summary]).join("\n") + "\n", "utf8"); | ||
|
|
||
| return { specs: specs.length, errors, warnings, commandFailures }; | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: The JSDoc lists scenarios 1-2 (command failures, validation errors), but the inline comments reference "Scenario 2" and "Scenario 3" here, "Scenario 1" in runInGitHubActions.
Consider aligning the numbering - either update JSDoc to include all 3 scenarios or renumber inline comments to match JSDoc. #Resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated