From a062454b7d7abea22b9d5187dc4101703b730e38 Mon Sep 17 00:00:00 2001 From: Scott Beddall Date: Wed, 1 Oct 2025 02:04:54 +0000 Subject: [PATCH 1/4] start progression on not setting RPaaSException label --- .github/workflows/src/summarize-checks/labelling.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.github/workflows/src/summarize-checks/labelling.js b/.github/workflows/src/summarize-checks/labelling.js index 3a1050885255..a254fac00e34 100644 --- a/.github/workflows/src/summarize-checks/labelling.js +++ b/.github/workflows/src/summarize-checks/labelling.js @@ -506,7 +506,7 @@ export async function processImpactAssessment(labelContext, impactAssessment) { ciNewRPNamespaceWithoutRpaaSLabel.shouldBePresent = impactAssessment.rpaasRPMissing || false; const rpaasExceptionLabel = new Label("RPaaSException", labelContext.present); - rpaasExceptionLabel.shouldBePresent = impactAssessment.rpaasExceptionRequired || false; + rpaasExceptionLabel.shouldBePresent = ("RPaaSException" in labelContext.present) const ciRpaasRPNotInPrivateRepoLabel = new Label( "CI-RpaaSRPNotInPrivateRepo", @@ -541,7 +541,7 @@ export async function processImpactAssessment(labelContext, impactAssessment) { labelContext, armReviewLabel.shouldBePresent, impactAssessment.rpaasRPMissing, - impactAssessment.rpaasExceptionRequired, + rpaasExceptionLabel.shouldBePresent, impactAssessment.rpaasRpNotInPrivateRepo, ); } @@ -555,7 +555,6 @@ export async function processImpactAssessment(labelContext, impactAssessment) { rpassReviewRequiredLabel.applyStateChange(labelContext.toAdd, labelContext.toRemove); newRPNamespaceLabel.applyStateChange(labelContext.toAdd, labelContext.toRemove); ciNewRPNamespaceWithoutRpaaSLabel.applyStateChange(labelContext.toAdd, labelContext.toRemove); - rpaasExceptionLabel.applyStateChange(labelContext.toAdd, labelContext.toRemove); ciRpaasRPNotInPrivateRepoLabel.applyStateChange(labelContext.toAdd, labelContext.toRemove); // this is the only labelling that was part of original pipelinebot logic, it handles the rotation of From b52f49ab0e785660fb45e2ae940d4ed4352fbe59 Mon Sep 17 00:00:00 2001 From: Scott Beddall Date: Wed, 1 Oct 2025 02:20:33 +0000 Subject: [PATCH 2/4] remove rpassExceptionRequired from impactAssessment --- .../summarize-impact/src/ImpactAssessment.ts | 1 - eng/tools/summarize-impact/src/impact.ts | 19 +++---------------- 2 files changed, 3 insertions(+), 17 deletions(-) diff --git a/eng/tools/summarize-impact/src/ImpactAssessment.ts b/eng/tools/summarize-impact/src/ImpactAssessment.ts index e6c331ad052c..2c42a3f76f78 100644 --- a/eng/tools/summarize-impact/src/ImpactAssessment.ts +++ b/eng/tools/summarize-impact/src/ImpactAssessment.ts @@ -3,7 +3,6 @@ export type ImpactAssessment = { dataPlaneRequired: boolean; suppressionReviewRequired: boolean; isNewApiVersion: boolean; - rpaasExceptionRequired: boolean; rpaasRpNotInPrivateRepo: boolean; rpaasChange: boolean; newRP: boolean; diff --git a/eng/tools/summarize-impact/src/impact.ts b/eng/tools/summarize-impact/src/impact.ts index cb9401e7d64c..7ed61cb12842 100644 --- a/eng/tools/summarize-impact/src/impact.ts +++ b/eng/tools/summarize-impact/src/impact.ts @@ -120,7 +120,7 @@ export async function evaluateImpact( // doesn't necessarily need to be in the PR context. // Uses the previous outputs that DID need to be a PR context, but otherwise only examines targetBranch and those // output labels. - const { ciNewRPNamespaceWithoutRpaaSLabelShouldBePresent, rpaasExceptionLabelShouldBePresent } = + const ciNewRPNamespaceWithoutRpaaSLabelShouldBePresent = await processNewRpNamespaceWithoutRpaasLabel( context, labelContext, @@ -149,7 +149,6 @@ export async function evaluateImpact( rpaasRpNotInPrivateRepo: ciRpaasRPNotInPrivateRepoLabelShouldBePresent, resourceManagerRequired: resourceManagerLabelShouldBePresent, dataPlaneRequired: dataPlaneShouldBePresent, - rpaasExceptionRequired: rpaasExceptionLabelShouldBePresent, typeSpecChanged: typeSpecLabelShouldBePresent, isNewApiVersion: newApiVersion, isDraft: context.isDraft, @@ -626,10 +625,7 @@ async function processNewRpNamespaceWithoutRpaasLabel( resourceManagerLabelShouldBePresent: boolean, newRPNamespaceLabelShouldBePresent: boolean, rpaasLabelShouldBePresent: boolean, -): Promise<{ - ciNewRPNamespaceWithoutRpaaSLabelShouldBePresent: boolean; - rpaasExceptionLabelShouldBePresent: boolean; -}> { +): Promise { console.log("ENTER definition processNewRpNamespaceWithoutRpaasLabel"); const ciNewRPNamespaceWithoutRpaaSLabel = new Label( "CI-NewRPNamespaceWithoutRPaaS", @@ -638,8 +634,6 @@ async function processNewRpNamespaceWithoutRpaasLabel( // By default this label should not be present. We may determine later in this function that it should be present after all. ciNewRPNamespaceWithoutRpaaSLabel.shouldBePresent = false; - const rpaasExceptionLabel = new Label("RPaaSException", labelContext.present); - let skip = false; if (!resourceManagerLabelShouldBePresent) { @@ -664,18 +658,11 @@ async function processNewRpNamespaceWithoutRpaasLabel( ciNewRPNamespaceWithoutRpaaSLabel.shouldBePresent = true; } - rpaasExceptionLabel.shouldBePresent = - rpaasExceptionLabel.present && ciNewRPNamespaceWithoutRpaaSLabel.shouldBePresent; ciNewRPNamespaceWithoutRpaaSLabel.applyStateChange(labelContext.toAdd, labelContext.toRemove); - rpaasExceptionLabel.applyStateChange(labelContext.toAdd, labelContext.toRemove); console.log("RETURN definition processNewRpNamespaceWithoutRpaasLabel"); - return { - ciNewRPNamespaceWithoutRpaaSLabelShouldBePresent: - ciNewRPNamespaceWithoutRpaaSLabel.shouldBePresent, - rpaasExceptionLabelShouldBePresent: rpaasExceptionLabel.shouldBePresent as boolean, - }; + return ciNewRPNamespaceWithoutRpaaSLabel.shouldBePresent; } export const getRPaaSFolderList = async ( From 111b27af7e816f2b6a3257bbc6b8ba8e8802b689 Mon Sep 17 00:00:00 2001 From: Scott Beddall Date: Wed, 1 Oct 2025 03:41:04 +0000 Subject: [PATCH 3/4] additional tests, updates to formatting, a couple other removals from the impact assessment contract --- .../src/summarize-checks/labelling.js | 6 +- .../test/summarize-checks/labelling.test.js | 28 +++++--- .../summarize-checks/summarize-checks.test.js | 72 ++++++++++++++++++- eng/tools/summarize-impact/src/impact.ts | 1 - .../summarize-impact/test/impact.test.ts | 6 -- 5 files changed, 92 insertions(+), 21 deletions(-) diff --git a/.github/workflows/src/summarize-checks/labelling.js b/.github/workflows/src/summarize-checks/labelling.js index a254fac00e34..6968dab417cf 100644 --- a/.github/workflows/src/summarize-checks/labelling.js +++ b/.github/workflows/src/summarize-checks/labelling.js @@ -51,7 +51,6 @@ import { * @property {boolean} dataPlaneRequired - Whether a data plane review is required. * @property {boolean} suppressionReviewRequired - Whether a suppression review is required. * @property {boolean} isNewApiVersion - Whether this PR introduces a new API version. - * @property {boolean} rpaasExceptionRequired - Whether an RPaaS exception is required. * @property {boolean} rpaasRpNotInPrivateRepo - Whether the RPaaS RP is not present in the private repo. * @property {boolean} rpaasChange - Whether this PR includes RPaaS changes. * @property {boolean} newRP - Whether this PR introduces a new resource provider. @@ -505,8 +504,7 @@ export async function processImpactAssessment(labelContext, impactAssessment) { ); ciNewRPNamespaceWithoutRpaaSLabel.shouldBePresent = impactAssessment.rpaasRPMissing || false; - const rpaasExceptionLabel = new Label("RPaaSException", labelContext.present); - rpaasExceptionLabel.shouldBePresent = ("RPaaSException" in labelContext.present) + const rpaasExceptionLabelPresent = "RPaaSException" in labelContext.present; const ciRpaasRPNotInPrivateRepoLabel = new Label( "CI-RpaaSRPNotInPrivateRepo", @@ -541,7 +539,7 @@ export async function processImpactAssessment(labelContext, impactAssessment) { labelContext, armReviewLabel.shouldBePresent, impactAssessment.rpaasRPMissing, - rpaasExceptionLabel.shouldBePresent, + rpaasExceptionLabelPresent, impactAssessment.rpaasRpNotInPrivateRepo, ); } diff --git a/.github/workflows/test/summarize-checks/labelling.test.js b/.github/workflows/test/summarize-checks/labelling.test.js index 3e81694bfed7..21b2dc9e8ba0 100644 --- a/.github/workflows/test/summarize-checks/labelling.test.js +++ b/.github/workflows/test/summarize-checks/labelling.test.js @@ -17,7 +17,6 @@ describe("update labels", () => { rpaasRpNotInPrivateRepo: false, resourceManagerRequired: true, dataPlaneRequired: false, - rpaasExceptionRequired: false, typeSpecChanged: true, isNewApiVersion: false, isDraft: false, @@ -37,7 +36,6 @@ describe("update labels", () => { rpaasRpNotInPrivateRepo: false, resourceManagerRequired: false, dataPlaneRequired: false, - rpaasExceptionRequired: false, typeSpecChanged: true, isNewApiVersion: false, isDraft: false, @@ -57,7 +55,6 @@ describe("update labels", () => { rpaasRpNotInPrivateRepo: false, resourceManagerRequired: true, dataPlaneRequired: false, - rpaasExceptionRequired: false, typeSpecChanged: true, isNewApiVersion: false, isDraft: false, @@ -77,7 +74,6 @@ describe("update labels", () => { rpaasRpNotInPrivateRepo: false, resourceManagerRequired: true, dataPlaneRequired: false, - rpaasExceptionRequired: false, typeSpecChanged: true, isNewApiVersion: true, isDraft: false, @@ -103,7 +99,6 @@ describe("update labels", () => { rpaasRpNotInPrivateRepo: false, resourceManagerRequired: true, dataPlaneRequired: false, - rpaasExceptionRequired: false, typeSpecChanged: true, isNewApiVersion: true, isDraft: false, @@ -123,7 +118,25 @@ describe("update labels", () => { rpaasRpNotInPrivateRepo: false, resourceManagerRequired: true, dataPlaneRequired: false, - rpaasExceptionRequired: false, + typeSpecChanged: true, + isNewApiVersion: true, + isDraft: false, + targetBranch: "a-test-branch", + }, + }, + { + description: "Shouldn't remove RPaasException label accidentally", + existingLabels: ["other-label", "CI-NewRPNamespaceWithoutRPaaS", "RPaaSException"], + expectedLabelsToAdd: ["TypeSpec", "resource-manager"], + expectedLabelsToRemove: [], + impactAssessment: { + suppressionReviewRequired: false, + rpaasChange: false, + newRP: false, + rpaasRPMissing: true, + rpaasRpNotInPrivateRepo: false, + resourceManagerRequired: true, + dataPlaneRequired: false, typeSpecChanged: true, isNewApiVersion: true, isDraft: false, @@ -150,7 +163,6 @@ describe("update labels", () => { rpaasRpNotInPrivateRepo: false, resourceManagerRequired: true, dataPlaneRequired: false, - rpaasExceptionRequired: false, typeSpecChanged: true, isNewApiVersion: true, isDraft: false, @@ -180,7 +192,6 @@ describe("update labels", () => { rpaasRpNotInPrivateRepo: false, resourceManagerRequired: true, dataPlaneRequired: false, - rpaasExceptionRequired: false, typeSpecChanged: true, isNewApiVersion: false, isDraft: false, @@ -211,7 +222,6 @@ describe("update labels", () => { rpaasRpNotInPrivateRepo: false, resourceManagerRequired: true, dataPlaneRequired: false, - rpaasExceptionRequired: false, typeSpecChanged: true, isNewApiVersion: false, isDraft: false, diff --git a/.github/workflows/test/summarize-checks/summarize-checks.test.js b/.github/workflows/test/summarize-checks/summarize-checks.test.js index 830a4879a72c..9c6dd9c777cf 100644 --- a/.github/workflows/test/summarize-checks/summarize-checks.test.js +++ b/.github/workflows/test/summarize-checks/summarize-checks.test.js @@ -433,6 +433,41 @@ describe("Summarize Checks Unit Tests", () => { expect(output).toEqual(expectedOutput); }); + it("should generate error summary when CI-NewRPNamespaceWithoutRPaaS is present, and RPaaSException is as well.", async () => { + const repo = "azure-rest-api-specs"; + const targetBranch = "main"; + const labelNames = ["CI-NewRPNamespaceWithoutRPaaS", "RPaaSException"]; + const fyiCheckRuns = []; + const expectedCommentBody = `

Next Steps to Merge

✅ All automated merging requirements have been met! To get your PR merged, see aka.ms/azsdk/specreview/merge.

Comment generated by summarize-checks workflow run.`; + const expectedCheckOutput = { + name: "Automated merging requirements met", + result: "SUCCESS", + summary: `✅ All automated merging requirements have been met.
To merge this PR, refer to aka.ms/azsdk/specreview/merge.
For help, consult comments on this PR and see [aka.ms/azsdk/pr-getting-help](https://aka.ms/azsdk/pr-getting-help).`, + }; + + const requiredCheckRuns = [ + { + name: "SpellCheck", + status: "COMPLETED", + conclusion: "SUCCESS", + checkInfo: getCheckInfo("SpellCheck"), + }, + ]; + + const [commentBody, automatedCheckOutput] = await createNextStepsComment( + mockCore, + repo, + labelNames, + targetBranch, + requiredCheckRuns, + fyiCheckRuns, + true, // assessmentCompleted + ); + + expect(commentBody).toEqual(expectedCommentBody); + expect(automatedCheckOutput).toEqual(expectedCheckOutput); + }); + it("should generate pending summary when checks are partially in progress", async () => { const repo = "azure-rest-api-specs"; const targetBranch = "main"; @@ -1116,6 +1151,42 @@ describe("Summarize Checks Unit Tests", () => { expect(automatedCheckOutput).toEqual(expectedCheckOutput); }); + it("should generate error summary when CI-NewRPNamespaceWithoutRPaaS is present, but RPaaSException is not.", async () => { + const repo = "azure-rest-api-specs"; + const targetBranch = "main"; + const labelNames = ["CI-NewRPNamespaceWithoutRPaaS"]; + const fyiCheckRuns = []; + const expectedCommentBody = `

Next Steps to Merge

Next steps that must be taken to merge this PR:
  • ❌ This PR has CI-NewRPNamespaceWithoutRPaaS label. This means it is introducing a new RP (Resource Provider) namespace that has not been onboarded with RPaaS. Merging this PR to the main branch is blocked as RPaaS is required for new RPs.
    To resolve, use RPaaS to onboard the new RP. To apply for exception, see aka.ms/RPaaSException.


Comment generated by summarize-checks workflow run.`; + const expectedCheckOutput = { + name: "Automated merging requirements met", + result: "FAILURE", + summary: + "❌ This PR cannot be merged because some requirements are not met. See the details.", + }; + + const requiredCheckRuns = [ + { + name: "SpellCheck", + status: "COMPLETED", + conclusion: "SUCCESS", + checkInfo: getCheckInfo("SpellCheck"), + }, + ]; + + const [commentBody, automatedCheckOutput] = await createNextStepsComment( + mockCore, + repo, + labelNames, + targetBranch, + requiredCheckRuns, + fyiCheckRuns, + true, // assessmentCompleted + ); + + expect(commentBody).toEqual(expectedCommentBody); + expect(automatedCheckOutput).toEqual(expectedCheckOutput); + }); + it("should generate error summary when checks are in error state", async () => { const repo = "azure-rest-api-specs"; const targetBranch = "main"; @@ -1165,7 +1236,6 @@ describe("Summarize Checks Unit Tests", () => { dataPlaneRequired: true, suppressionReviewRequired: false, isNewApiVersion: true, - rpaasExceptionRequired: false, rpaasRpNotInPrivateRepo: true, rpaasChange: false, newRP: true, diff --git a/eng/tools/summarize-impact/src/impact.ts b/eng/tools/summarize-impact/src/impact.ts index 7ed61cb12842..992c9e4b4bfa 100644 --- a/eng/tools/summarize-impact/src/impact.ts +++ b/eng/tools/summarize-impact/src/impact.ts @@ -658,7 +658,6 @@ async function processNewRpNamespaceWithoutRpaasLabel( ciNewRPNamespaceWithoutRpaaSLabel.shouldBePresent = true; } - ciNewRPNamespaceWithoutRpaaSLabel.applyStateChange(labelContext.toAdd, labelContext.toRemove); console.log("RETURN definition processNewRpNamespaceWithoutRpaasLabel"); diff --git a/eng/tools/summarize-impact/test/impact.test.ts b/eng/tools/summarize-impact/test/impact.test.ts index 36acfd4a0835..41c030f05964 100644 --- a/eng/tools/summarize-impact/test/impact.test.ts +++ b/eng/tools/summarize-impact/test/impact.test.ts @@ -47,7 +47,6 @@ describe("Impact Detection - default fixture", () => { rpaasRpNotInPrivateRepo: false, resourceManagerRequired: false, dataPlaneRequired: false, - rpaasExceptionRequired: false, typeSpecChanged: false, isNewApiVersion: false, isDraft: false, @@ -71,7 +70,6 @@ describe("Impact Detection - default fixture", () => { rpaasRpNotInPrivateRepo: false, resourceManagerRequired: false, dataPlaneRequired: false, - rpaasExceptionRequired: false, typeSpecChanged: true, isNewApiVersion: false, isDraft: false, @@ -95,7 +93,6 @@ describe("Impact Detection - default fixture", () => { rpaasRpNotInPrivateRepo: false, resourceManagerRequired: true, dataPlaneRequired: false, - rpaasExceptionRequired: false, typeSpecChanged: false, isNewApiVersion: false, isDraft: false, @@ -121,7 +118,6 @@ describe("Impact Detection - default fixture", () => { rpaasRpNotInPrivateRepo: false, resourceManagerRequired: false, dataPlaneRequired: true, - rpaasExceptionRequired: false, typeSpecChanged: true, isNewApiVersion: true, isDraft: false, @@ -148,7 +144,6 @@ describe("Impact Detection - default fixture", () => { rpaasRpNotInPrivateRepo: true, resourceManagerRequired: true, dataPlaneRequired: false, - rpaasExceptionRequired: false, typeSpecChanged: true, isNewApiVersion: true, isDraft: false, @@ -175,7 +170,6 @@ describe("Impact Detection - default fixture", () => { rpaasRpNotInPrivateRepo: false, resourceManagerRequired: true, dataPlaneRequired: false, - rpaasExceptionRequired: false, typeSpecChanged: true, isNewApiVersion: true, isDraft: false, From 050af581cde2c44ca5db02a508c9f5159abb8622 Mon Sep 17 00:00:00 2001 From: Scott Beddall <45376673+scbedd@users.noreply.github.com> Date: Wed, 1 Oct 2025 11:05:11 -0700 Subject: [PATCH 4/4] Update .github/workflows/test/summarize-checks/summarize-checks.test.js Co-authored-by: Mike Harder --- .../workflows/test/summarize-checks/summarize-checks.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test/summarize-checks/summarize-checks.test.js b/.github/workflows/test/summarize-checks/summarize-checks.test.js index 9c6dd9c777cf..77a902e65c1d 100644 --- a/.github/workflows/test/summarize-checks/summarize-checks.test.js +++ b/.github/workflows/test/summarize-checks/summarize-checks.test.js @@ -433,7 +433,7 @@ describe("Summarize Checks Unit Tests", () => { expect(output).toEqual(expectedOutput); }); - it("should generate error summary when CI-NewRPNamespaceWithoutRPaaS is present, and RPaaSException is as well.", async () => { + it("should generate success summary when CI-NewRPNamespaceWithoutRPaaS is present, and RPaaSException is as well.", async () => { const repo = "azure-rest-api-specs"; const targetBranch = "main"; const labelNames = ["CI-NewRPNamespaceWithoutRPaaS", "RPaaSException"];