Skip to content

Commit a2a0bcd

Browse files
authored
Support cross version breaking change validation (Azure#35804)
* Support cross version breaking change validation * Debug SHA env variables * updated code in setting file URL * updated helper functions * Fixed logging issue and optimized tests * refactored CLI * Fixed tests * removed short definition for parameters * Added rename change files to other arrarys * Used full argument name in yml * Used env variables * Break JSON into multiple lines before displaying * use more env variables and revert the message printing * remove additional spaces in yaml * Correct the head sha input
1 parent 1385599 commit a2a0bcd

28 files changed

+3421
-1754
lines changed

.github/workflows/breaking-change-code.yaml

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,13 @@ jobs:
2828
id: swagger-breaking-change-analyze-code
2929
run: |
3030
npm exec --no -- openapi-diff-runner \
31-
--srp "${{ github.workspace }}" \
32-
--number "${{ github.event.pull_request.number }}" \
33-
--sb "${{ github.event.pull_request.head.ref }}" \
34-
--tb "${{ github.event.pull_request.base.ref }}" \
35-
--hc "${{ github.event.pull_request.head.sha }}" \
36-
--repo "${{ github.repository }}"
31+
--spec-repo-path "$GITHUB_WORKSPACE" \
32+
--pr-source-branch "$GITHUB_HEAD_REF" \
33+
--pr-target-branch "$GITHUB_BASE_REF" \
34+
--head-commit "${{ github.event.pull_request.head.sha }}" \
35+
--pr-number "${{ github.event.pull_request.number }}" \
36+
--source-repo "${{ github.event.pull_request.head.repo.full_name }}" \
37+
--target-repo "$GITHUB_REPOSITORY"
3738
3839
# Upload artifact for 'BreakingChangeReviewRequired' label
3940
- if: |
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
name: "[TEST-IGNORE] Breaking Change(Cross-Version) - Analyze Code"
2+
3+
on: pull_request
4+
5+
permissions:
6+
contents: read
7+
8+
jobs:
9+
validateBreakingChange:
10+
name: "[TEST-IGNORE] Breaking Change(Cross-Version) - Analyze Code"
11+
runs-on: ubuntu-24.04
12+
13+
steps:
14+
- name: Checkout repo
15+
uses: actions/checkout@v4
16+
with:
17+
fetch-depth: 0
18+
19+
- name: Setup Node and install deps
20+
uses: ./.github/actions/setup-node-install-deps
21+
22+
- name: Setup .NET 6 SDK
23+
uses: actions/setup-dotnet@v4
24+
with:
25+
dotnet-version: "6.0.x"
26+
27+
- name: Breaking Change(Cross-Version) - Analyze Code
28+
id: breaking-change-cross-version-analyze-code
29+
run: |
30+
npm exec --no -- openapi-diff-runner \
31+
--run-type "CrossVersion" \
32+
--spec-repo-path "$GITHUB_WORKSPACE" \
33+
--pr-source-branch "$GITHUB_HEAD_REF" \
34+
--pr-target-branch "$GITHUB_BASE_REF" \
35+
--head-commit "${{ github.event.pull_request.head.sha }}" \
36+
--pr-number "${{ github.event.pull_request.number }}" \
37+
--source-repo "${{ github.event.pull_request.head.repo.full_name }}" \
38+
--target-repo "$GITHUB_REPOSITORY"
39+
40+
# Upload artifact for 'BreakingChangeReviewRequired' label
41+
- if: |
42+
always() &&
43+
(steps.breaking-change-cross-version-analyze-code.outputs.breakingChangeReviewLabelName != '')
44+
name: Upload artifact with BreakingChangeReviewRequiredLabel label
45+
uses: ./.github/actions/add-label-artifact
46+
with:
47+
name: "${{ steps.breaking-change-cross-version-analyze-code.outputs.breakingChangeReviewLabelName }}"
48+
value: "${{ steps.breaking-change-cross-version-analyze-code.outputs.breakingChangeReviewLabelValue == 'true' }}"
49+
50+
# Upload artifact for 'VersioningReviewRequired' label
51+
- if: |
52+
always() &&
53+
(steps.breaking-change-cross-version-analyze-code.outputs.versioningReviewLabelName != '')
54+
name: Upload artifact with VersioningReviewRequiredLabel label
55+
uses: ./.github/actions/add-label-artifact
56+
with:
57+
name: "${{ steps.breaking-change-cross-version-analyze-code.outputs.versioningReviewLabelName }}"
58+
value: "${{ steps.breaking-change-cross-version-analyze-code.outputs.versioningReviewLabelValue == 'true' }}"
59+
60+
# Upload artifact with issue number if labels are present and PR number is valid
61+
- if: |
62+
always() &&
63+
(steps.breaking-change-cross-version-analyze-code.outputs.breakingChangeReviewLabelName != '' ||
64+
steps.breaking-change-cross-version-analyze-code.outputs.versioningReviewLabelName != '') &&
65+
github.event.pull_request.number > 0
66+
name: Upload artifact with issue number
67+
uses: ./.github/actions/add-empty-artifact
68+
with:
69+
name: "issue-number"
70+
value: "${{ github.event.pull_request.number }}"

eng/tools/openapi-diff-runner/src/command-helpers.ts

Lines changed: 46 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1,59 +1,59 @@
11
import path from "node:path";
22
import { existsSync, mkdirSync, readFileSync, writeFileSync, rmSync } from "node:fs";
3-
import { fileURLToPath } from "node:url";
43
import {
54
BreakingChangesCheckType,
65
Context,
76
BreakingChangeReviewRequiredLabel,
87
VersioningReviewRequiredLabel,
98
} from "./types/breaking-change.js";
109
import { ResultMessageRecord } from "./types/message.js";
11-
import { getArgumentValue } from "./utils/common-utils.js";
1210
import { createOadMessageProcessor } from "./utils/oad-message-processor.js";
1311
import { createPullRequestProperties } from "./utils/pull-request.js";
1412
import { getChangedFilesStatuses, swagger } from "@azure-tools/specs-shared/changed-files";
15-
import { logMessage, setOutput } from "./log.js";
13+
import { BREAKING_CHANGES_CHECK_TYPES } from "@azure-tools/specs-shared/breaking-change";
14+
import { logError, LogLevel, logMessage, setOutput } from "./log.js";
1615

1716
/**
18-
* Parse the arguments.
19-
* @returns The runner command input.
17+
* Interface for parsed CLI arguments
2018
*/
21-
export function initContext(): Context {
22-
const __filename: string = fileURLToPath(import.meta.url);
23-
const __dirname: string = path.dirname(__filename);
19+
export interface ParsedCliArguments {
20+
localSpecRepoPath: string;
21+
targetRepo: string;
22+
sourceRepo: string;
23+
prNumber: string;
24+
runType: BreakingChangesCheckType;
25+
baseBranch: string;
26+
headCommit: string;
27+
prSourceBranch: string;
28+
prTargetBranch: string;
29+
}
2430

25-
// Get the arguments passed to the script
26-
const args: string[] = process.argv.slice(2);
27-
const localSpecRepoPath: string = path.resolve(
28-
getArgumentValue(args, "--srp", path.join(__dirname, "..")),
29-
);
31+
/**
32+
* Create context from parsed CLI arguments
33+
*/
34+
export function createContextFromParsedArgs(
35+
parsedArgs: ParsedCliArguments,
36+
workingFolder: string,
37+
logFileFolder: string,
38+
): Context {
3039
const swaggerDirs: string[] = ["specification", "dev"];
31-
const repo: string = getArgumentValue(args, "--repo", "azure/azure-rest-api-specs");
32-
const prNumber: string = getArgumentValue(args, "--number", "");
33-
const runType = getArgumentValue(args, "--rt", "SameVersion") as BreakingChangesCheckType;
34-
const workingFolder: string = path.join(localSpecRepoPath, "..");
35-
const logFileFolder: string = path.join(workingFolder, "out/logs");
36-
37-
// Create the log file folder if it does not exist
38-
if (!existsSync(logFileFolder)) {
39-
mkdirSync(logFileFolder, { recursive: true });
40-
}
41-
42-
const prUrl = `https://github.com/${repo}/pull/${prNumber}`;
40+
const prUrl = `https://github.com/${parsedArgs.targetRepo}/pull/${parsedArgs.prNumber}`;
4341
const oadMessageProcessorContext = createOadMessageProcessor(logFileFolder, prUrl);
42+
4443
return {
45-
localSpecRepoPath,
44+
localSpecRepoPath: parsedArgs.localSpecRepoPath,
4645
workingFolder,
4746
swaggerDirs,
4847
logFileFolder,
49-
baseBranch: getArgumentValue(args, "--bb", "main"),
50-
runType,
51-
checkName: getBreakingChangeCheckName(runType),
52-
headCommit: getArgumentValue(args, "--hc", "HEAD"),
53-
repo,
54-
prNumber,
55-
prSourceBranch: getArgumentValue(args, "--sb", ""),
56-
prTargetBranch: getArgumentValue(args, "--tb", ""),
48+
baseBranch: parsedArgs.baseBranch,
49+
runType: parsedArgs.runType,
50+
checkName: getBreakingChangeCheckName(parsedArgs.runType),
51+
headCommit: parsedArgs.headCommit,
52+
targetRepo: parsedArgs.targetRepo,
53+
sourceRepo: parsedArgs.sourceRepo,
54+
prNumber: parsedArgs.prNumber,
55+
prSourceBranch: parsedArgs.prSourceBranch,
56+
prTargetBranch: parsedArgs.prTargetBranch,
5757
oadMessageProcessorContext,
5858
prUrl,
5959
};
@@ -65,9 +65,10 @@ export function initContext(): Context {
6565
* Appropriate labels are added to this set by applyRules() function.
6666
*/
6767
export const BreakingChangeLabelsToBeAdded = new Set<string>();
68-
export let defaultBreakingChangeBaseBranch = "main";
6968
function getBreakingChangeCheckName(runType: BreakingChangesCheckType): string {
70-
return runType === "SameVersion" ? "Swagger BreakingChange" : "BreakingChange(Cross-Version)";
69+
return runType === BREAKING_CHANGES_CHECK_TYPES.SAME_VERSION
70+
? "Swagger BreakingChange"
71+
: "BreakingChange(Cross-Version)";
7172
}
7273

7374
/**
@@ -124,7 +125,6 @@ export async function getSwaggerDiffs(
124125
additions: string[];
125126
modifications: string[];
126127
deletions: string[];
127-
renames: { from: string; to: string }[];
128128
total: number;
129129
}> {
130130
try {
@@ -143,25 +143,23 @@ export async function getSwaggerDiffs(
143143
(rename) => swagger(rename.from) && swagger(rename.to),
144144
);
145145

146+
// Add renamed files to the additions array and deletions array
147+
filteredAdditions.push(...filteredRenames.map((rename) => rename.to));
148+
filteredDeletions.push(...filteredRenames.map((rename) => rename.from));
149+
146150
return {
147151
additions: filteredAdditions,
148152
modifications: filteredModifications,
149153
deletions: filteredDeletions,
150-
renames: filteredRenames,
151-
total:
152-
filteredAdditions.length +
153-
filteredModifications.length +
154-
filteredDeletions.length +
155-
filteredRenames.length,
154+
total: filteredAdditions.length + filteredModifications.length + filteredDeletions.length,
156155
};
157156
} catch (error) {
158-
console.error("Error getting categorized changed files:", error);
157+
logError(`Error getting categorized changed files: ${error}`);
159158
// Return empty result on error
160159
return {
161160
additions: [],
162161
modifications: [],
163162
deletions: [],
164-
renames: [],
165163
total: 0,
166164
};
167165
}
@@ -172,11 +170,6 @@ export async function getSwaggerDiffs(
172170
* TargetBranches is a set of branches and treat each of them like a service team master branch.
173171
*/
174172
export async function buildPrInfo(context: Context): Promise<void> {
175-
/**
176-
* For PR target branch not in `targetBranches`. prepare for switch to master branch,
177-
* if not the switching to master below would failed
178-
*/
179-
defaultBreakingChangeBaseBranch = context.baseBranch;
180173
const prInfo = await createPullRequestProperties(
181174
context,
182175
context.runType === "CrossVersion" ? "cross-version" : "same-version",
@@ -216,15 +209,15 @@ export function changeBaseBranch(context: Context): void {
216209
* Log the full list of OAD messages to console
217210
*/
218211
export function logFullOadMessagesList(msgs: ResultMessageRecord[]): void {
219-
logMessage("---- Full list of messages ----");
212+
logMessage("---- Full list of messages ----", LogLevel.Group);
220213
logMessage("[");
221214
// Printing the messages one by one because the console.log appears to elide the messages with "... X more items"
222215
// after approximately 292 messages.
223216
for (const msg of msgs) {
224217
logMessage(JSON.stringify(msg, null, 4) + ",");
225218
}
226219
logMessage("]");
227-
logMessage("---- End of full list of messages ----");
220+
logMessage("---- End of full list of messages ----", LogLevel.EndGroup);
228221
}
229222

230223
/**
@@ -265,7 +258,7 @@ export function cleanDummySwagger(): void {
265258
* Return true if the type indicates the same version breaking change
266259
*/
267260
export function isSameVersionBreakingType(type: BreakingChangesCheckType): boolean {
268-
return type === "SameVersion";
261+
return type === BREAKING_CHANGES_CHECK_TYPES.SAME_VERSION;
269262
}
270263

271264
/**

0 commit comments

Comments
 (0)