diff --git a/.github/workflows/src/arm-auto-signoff/arm-auto-signoff-status.js b/.github/workflows/src/arm-auto-signoff/arm-auto-signoff-status.js index 63a01865463b..cd10aae27d75 100644 --- a/.github/workflows/src/arm-auto-signoff/arm-auto-signoff-status.js +++ b/.github/workflows/src/arm-auto-signoff/arm-auto-signoff-status.js @@ -105,8 +105,15 @@ export async function getLabelActionImpl({ owner, repo, issue_number, head_sha, labelNames.includes(ArmAutoSignoffLabel.ArmAutoSignedOff) || labelNames.includes(ArmAutoSignoffLabel.ArmAutoSignedOffIncrementalTSP) || labelNames.includes(ArmAutoSignoffLabel.ArmAutoSignedOffTrivialTest); + + // Track if ARMSignedOff was auto-added (IncrementalTSP present) vs manually added + const hasAutoAddedArmSignedOff = + // need to consider the legacy label + labelNames.includes(ArmAutoSignoffLabel.ArmAutoSignedOff) || + labelNames.includes(ArmAutoSignoffLabel.ArmAutoSignedOffIncrementalTSP); core.info(`Labels: ${inspect(labelNames)}`); core.info(`Has auto signed-off labels: ${hasAutoSignedOffLabels}`); + core.info(`Has auto-added ARMSignedOff: ${hasAutoAddedArmSignedOff}`); // permissions: { actions: read } /** @type {WorkflowRun[]} */ @@ -136,12 +143,18 @@ export async function getLabelActionImpl({ owner, repo, issue_number, head_sha, return noneResult; } + // Only remove ARMSignedOff if it was auto-added (IncrementalTSP present) + // Preserve manually-added ARMSignedOff labels return { ...noneResult, labelActions: { ...noneLabelActions, - [ArmAutoSignoffLabel.ArmSignedOff]: LabelAction.Remove, - [ArmAutoSignoffLabel.ArmAutoSignedOffIncrementalTSP]: LabelAction.Remove, + [ArmAutoSignoffLabel.ArmSignedOff]: hasAutoAddedArmSignedOff + ? LabelAction.Remove + : LabelAction.None, + [ArmAutoSignoffLabel.ArmAutoSignedOffIncrementalTSP]: hasAutoAddedArmSignedOff + ? LabelAction.Remove + : LabelAction.None, [ArmAutoSignoffLabel.ArmAutoSignedOffTrivialTest]: LabelAction.Remove, }, }; @@ -221,9 +234,11 @@ export async function getLabelActionImpl({ owner, repo, issue_number, head_sha, // Add ARMSignOff label only when the PR is identified as an incremental typespec // As the trivial changes sign-off is being released in test mode + // Only remove ARMSignedOff if it was auto-added (IncrementalTSP present) + // Preserve manually-added ARMSignedOff labels const armSignOffAction = autoIncrementalTSP ? LabelAction.Add - : hasAutoSignedOffLabels + : hasAutoAddedArmSignedOff ? LabelAction.Remove : LabelAction.None; const autoIncrementalTSPAction = autoIncrementalTSP ? LabelAction.Add : LabelAction.Remove; diff --git a/.github/workflows/test/arm-auto-signoff/arm-auto-signoff-status.test.js b/.github/workflows/test/arm-auto-signoff/arm-auto-signoff-status.test.js index e1857dd82539..244e0969290c 100644 --- a/.github/workflows/test/arm-auto-signoff/arm-auto-signoff-status.test.js +++ b/.github/workflows/test/arm-auto-signoff/arm-auto-signoff-status.test.js @@ -175,24 +175,6 @@ describe("getLabelActionImpl", () => { ).resolves.toEqual(createRemoveManagedLabelsResult("abc123", 123)); }); - it("removes ARMAutoSignedOff-Trivial-Test if analysis no longer trivial", async () => { - const github = createMockGithub({ incrementalTypeSpec: false, isTrivial: false }); - github.rest.issues.listLabelsOnIssue.mockResolvedValue({ - data: [{ name: managedLabels.autoSignedOffTrivialTest }], - }); - - await expect( - getLabelActionImpl({ - owner: "TestOwner", - repo: "TestRepo", - issue_number: 123, - head_sha: "abc123", - github: github, - core: core, - }), - ).resolves.toEqual(createRemoveManagedLabelsResult("abc123", 123)); - }); - it("removes label if analysis artifacts missing (fail closed)", async () => { const github = createMockGithub({ incrementalTypeSpec: true }); github.rest.issues.listLabelsOnIssue.mockResolvedValue({ @@ -560,4 +542,223 @@ describe("getLabelActionImpl", () => { }), ).resolves.toEqual(createRemoveManagedLabelsResult("abc123", 123)); }); + + it("preserves existing ARMSignedOff and adds trivial test label when PR qualifies by trivial changes only", async () => { + const github = createMockGithub({ incrementalTypeSpec: false, isTrivial: true }); + + // PR already has ARMSignedOff (manually added) and ARMReview, but no auto-signoff labels + github.rest.issues.listLabelsOnIssue.mockResolvedValue({ + data: [{ name: "ARMSignedOff" }, { name: "ARMReview" }], + }); + + // All required checks pass + github.rest.repos.listCommitStatusesForRef.mockResolvedValue({ + data: [ + { + context: "Swagger LintDiff", + state: CommitStatusState.SUCCESS, + }, + { + context: "Swagger Avocado", + state: CommitStatusState.SUCCESS, + }, + ], + }); + + const result = await getLabelActionImpl({ + owner: "TestOwner", + repo: "TestRepo", + issue_number: 123, + head_sha: "abc123", + github: github, + core: core, + }); + + // ARMSignedOff should remain unchanged (LabelAction.None) since it already exists + // and is not managed by auto-signoff when hasAutoSignedOffLabels is false + expect(result.labelActions[managedLabels.armSignedOff]).toBe(LabelAction.None); + // Trivial test label should be added + expect(result.labelActions[managedLabels.autoSignedOffTrivialTest]).toBe(LabelAction.Add); + // Incremental TSP label should be removed (not applicable) + expect(result.labelActions[managedLabels.autoSignedOffIncrementalTsp]).toBe(LabelAction.Remove); + }); + + it("preserves manual ARMSignedOff when trivial test label present and no longer qualifies", async () => { + const github = createMockGithub({ incrementalTypeSpec: false, isTrivial: false }); + + // PR has manually added ARMSignedOff + trivial test label (from previous run) + github.rest.issues.listLabelsOnIssue.mockResolvedValue({ + data: [ + { name: "ARMSignedOff" }, + { name: managedLabels.autoSignedOffTrivialTest }, + { name: "ARMReview" }, + ], + }); + + const result = await getLabelActionImpl({ + owner: "TestOwner", + repo: "TestRepo", + issue_number: 123, + head_sha: "abc123", + github: github, + core: core, + }); + + // ARMSignedOff was manually added (no IncrementalTSP), so preserve it + expect(result.labelActions[managedLabels.armSignedOff]).toBe(LabelAction.None); + // Trivial test label should be removed (no longer qualifies) + expect(result.labelActions[managedLabels.autoSignedOffTrivialTest]).toBe(LabelAction.Remove); + // Incremental TSP label was never present, so no-op + expect(result.labelActions[managedLabels.autoSignedOffIncrementalTsp]).toBe(LabelAction.None); + }); + + it("removes all auto-signoff labels when IncrementalTSP was present and no longer qualifies", async () => { + const github = createMockGithub({ incrementalTypeSpec: false, isTrivial: false }); + + // PR had auto-added ARMSignedOff via IncrementalTSP + github.rest.issues.listLabelsOnIssue.mockResolvedValue({ + data: [ + { name: "ARMSignedOff" }, + { name: managedLabels.autoSignedOffIncrementalTsp }, + { name: "ARMReview" }, + ], + }); + + const result = await getLabelActionImpl({ + owner: "TestOwner", + repo: "TestRepo", + issue_number: 123, + head_sha: "abc123", + github: github, + core: core, + }); + + // ARMSignedOff was auto-added (IncrementalTSP present), so remove it + expect(result.labelActions[managedLabels.armSignedOff]).toBe(LabelAction.Remove); + // Both auto-signoff labels should be removed + expect(result.labelActions[managedLabels.autoSignedOffTrivialTest]).toBe(LabelAction.Remove); + expect(result.labelActions[managedLabels.autoSignedOffIncrementalTsp]).toBe(LabelAction.Remove); + }); + + it("transitions from trivial to incremental typespec correctly", async () => { + const github = createMockGithub({ incrementalTypeSpec: true, isTrivial: false }); + + // PR previously had trivial test label, now qualifies via incremental typespec + github.rest.issues.listLabelsOnIssue.mockResolvedValue({ + data: [{ name: managedLabels.autoSignedOffTrivialTest }, { name: "ARMReview" }], + }); + + github.rest.repos.listCommitStatusesForRef.mockResolvedValue({ + data: [ + { context: "Swagger LintDiff", state: CommitStatusState.SUCCESS }, + { context: "Swagger Avocado", state: CommitStatusState.SUCCESS }, + ], + }); + + const result = await getLabelActionImpl({ + owner: "TestOwner", + repo: "TestRepo", + issue_number: 123, + head_sha: "abc123", + github: github, + core: core, + }); + + // Now qualifies via incremental typespec, so add ARMSignedOff + expect(result.labelActions[managedLabels.armSignedOff]).toBe(LabelAction.Add); + expect(result.labelActions[managedLabels.autoSignedOffIncrementalTsp]).toBe(LabelAction.Add); + // Trivial test label should be removed (not applicable) + expect(result.labelActions[managedLabels.autoSignedOffTrivialTest]).toBe(LabelAction.Remove); + }); + + it("transitions from incremental typespec to trivial correctly", async () => { + const github = createMockGithub({ incrementalTypeSpec: false, isTrivial: true }); + + // PR previously had incremental typespec labels, now qualifies only via trivial + github.rest.issues.listLabelsOnIssue.mockResolvedValue({ + data: [ + { name: "ARMSignedOff" }, + { name: managedLabels.autoSignedOffIncrementalTsp }, + { name: "ARMReview" }, + ], + }); + + github.rest.repos.listCommitStatusesForRef.mockResolvedValue({ + data: [ + { context: "Swagger LintDiff", state: CommitStatusState.SUCCESS }, + { context: "Swagger Avocado", state: CommitStatusState.SUCCESS }, + ], + }); + + const result = await getLabelActionImpl({ + owner: "TestOwner", + repo: "TestRepo", + issue_number: 123, + head_sha: "abc123", + github: github, + core: core, + }); + + // IncrementalTSP was present, so ARMSignedOff should be removed (it was auto-added) + expect(result.labelActions[managedLabels.armSignedOff]).toBe(LabelAction.Remove); + // Incremental TSP label should be removed + expect(result.labelActions[managedLabels.autoSignedOffIncrementalTsp]).toBe(LabelAction.Remove); + // Trivial test label should be added + expect(result.labelActions[managedLabels.autoSignedOffTrivialTest]).toBe(LabelAction.Add); + }); + + it("adds both labels when PR qualifies for both incremental typespec and trivial", async () => { + const github = createMockGithub({ incrementalTypeSpec: true, isTrivial: true }); + + github.rest.issues.listLabelsOnIssue.mockResolvedValue({ + data: [{ name: "ARMReview" }], + }); + + github.rest.repos.listCommitStatusesForRef.mockResolvedValue({ + data: [ + { context: "Swagger LintDiff", state: CommitStatusState.SUCCESS }, + { context: "Swagger Avocado", state: CommitStatusState.SUCCESS }, + ], + }); + + const result = await getLabelActionImpl({ + owner: "TestOwner", + repo: "TestRepo", + issue_number: 123, + head_sha: "abc123", + github: github, + core: core, + }); + + // Incremental typespec takes precedence for ARMSignedOff + expect(result.labelActions[managedLabels.armSignedOff]).toBe(LabelAction.Add); + expect(result.labelActions[managedLabels.autoSignedOffIncrementalTsp]).toBe(LabelAction.Add); + // Trivial test label should also be added + expect(result.labelActions[managedLabels.autoSignedOffTrivialTest]).toBe(LabelAction.Add); + }); + + it("removes only trivial test label when only TrivialTest present and no longer qualifies", async () => { + const github = createMockGithub({ incrementalTypeSpec: false, isTrivial: false }); + + // PR only has trivial test label (no ARMSignedOff, no IncrementalTSP) + github.rest.issues.listLabelsOnIssue.mockResolvedValue({ + data: [{ name: managedLabels.autoSignedOffTrivialTest }, { name: "ARMReview" }], + }); + + const result = await getLabelActionImpl({ + owner: "TestOwner", + repo: "TestRepo", + issue_number: 123, + head_sha: "abc123", + github: github, + core: core, + }); + + // No IncrementalTSP, so ARMSignedOff action is None + expect(result.labelActions[managedLabels.armSignedOff]).toBe(LabelAction.None); + // Trivial test label should be removed + expect(result.labelActions[managedLabels.autoSignedOffTrivialTest]).toBe(LabelAction.Remove); + // Incremental TSP was never present, so no-op + expect(result.labelActions[managedLabels.autoSignedOffIncrementalTsp]).toBe(LabelAction.None); + }); });