diff --git a/torchci/lib/bot/autoLabelBot.ts b/torchci/lib/bot/autoLabelBot.ts index 5623ae96e2..b200c1ef1e 100644 --- a/torchci/lib/bot/autoLabelBot.ts +++ b/torchci/lib/bot/autoLabelBot.ts @@ -1,6 +1,6 @@ -import { minimatch } from "minimatch"; import { Context, Probot } from "probot"; import { addLabelErrComment, hasRequiredLabels } from "./checkLabelsUtils"; +import { getLabelsFromLabelerConfig } from "./labelerConfigUtils"; import { addLabels, CachedIssueTracker, @@ -13,6 +13,8 @@ import { LabelToLabelConfigTracker, } from "./utils"; +export { getLabelsFromLabelerConfig }; + // List of regex patterns for assigning labels to both Pull Requests and Issues const IssueAndPRRegexToLabel: [RegExp, string][] = [ [/rocm/gi, "module: rocm"], @@ -140,27 +142,6 @@ const notUserFacingPatterns: RegExp[] = [ const notUserFacingPatternExceptions: RegExp[] = [/tools\/autograd/g]; -export async function getLabelsFromLabelerConfig( - context: Context, - labelerConfigTracker: CachedLabelerConfigTracker, - changed_files: string[] -): Promise { - const config = await labelerConfigTracker.loadLabelsConfig(context); - - const labels = []; - - for (const [label, globs] of Object.entries(config)) { - if ( - globs.some((glob: string) => - changed_files.some((file: string) => minimatch(file, glob)) - ) - ) { - labels.push(label); - } - } - return labels; -} - export async function getLabelsFromLabelToLabelConfig( context: Context, labelToLabelConfigTracker: LabelToLabelConfigTracker, @@ -469,7 +450,12 @@ function myBot(app: Probot): void { }); app.on( - ["pull_request.opened", "pull_request.edited", "pull_request.synchronize"], + [ + "pull_request.opened", + "pull_request.edited", + "pull_request.synchronize", + "pull_request.ready_for_review", + ], async (context) => { const owner = context.payload.repository.owner.login; if (!isPyTorchbotSupportedOrg(owner)) { @@ -512,10 +498,13 @@ function myBot(app: Probot): void { } } + const isDraft = context.payload.pull_request.draft; + var labelsFromLabelerConfig = await getLabelsFromLabelerConfig( context, labelerConfigTracker, - filesChanged + filesChanged, + isDraft ); labelsToAdd.push(...labelsFromLabelerConfig); diff --git a/torchci/lib/bot/labelerConfigUtils.ts b/torchci/lib/bot/labelerConfigUtils.ts new file mode 100644 index 0000000000..228e8164c2 --- /dev/null +++ b/torchci/lib/bot/labelerConfigUtils.ts @@ -0,0 +1,117 @@ +import { minimatch } from "minimatch"; +import { Context } from "probot"; +import { CachedLabelerConfigTracker } from "./utils"; + +/** Legacy rules are a list of path globs; extended rules add optional draft behavior. */ +export type LabelerRule = + | string[] + | { + globs: string[]; + /** false = skip label while PR is draft; true = only when draft; omit = always apply when globs match */ + draft?: boolean; + }; + +export function labelerRuleSkipReason( + rawRule: unknown +): "invalid_draft" | "invalid_shape" | null { + if ( + rawRule !== null && + typeof rawRule === "object" && + "globs" in rawRule && + Array.isArray((rawRule as { globs: unknown }).globs) && + (rawRule as { globs: unknown[] }).globs.every((x) => typeof x === "string") + ) { + const r = rawRule as { globs: string[]; draft?: unknown }; + if ("draft" in r && typeof r.draft !== "boolean") { + return "invalid_draft"; + } + } + return "invalid_shape"; +} + +export function normalizeLabelerRule(rule: unknown): LabelerRule | null { + if (Array.isArray(rule) && rule.every((x) => typeof x === "string")) { + return rule as string[]; + } + if ( + rule !== null && + typeof rule === "object" && + "globs" in rule && + Array.isArray((rule as { globs: unknown }).globs) && + (rule as { globs: unknown[] }).globs.every((x) => typeof x === "string") + ) { + const r = rule as { globs: string[]; draft?: unknown }; + if ("draft" in r && typeof r.draft !== "boolean") { + return null; + } + const out: { globs: string[]; draft?: boolean } = { globs: r.globs }; + if (typeof r.draft === "boolean") { + out.draft = r.draft; + } + return out; + } + return null; +} + +export function globsFromRule(rule: LabelerRule): string[] { + return Array.isArray(rule) ? rule : rule.globs; +} + +export function draftConstraintAllowsLabel( + rule: LabelerRule, + isDraft: boolean +): boolean { + const draftOpt = Array.isArray(rule) ? undefined : rule.draft; + if (draftOpt === undefined) { + return true; + } + if (draftOpt === false) { + return !isDraft; + } + return isDraft; +} + +export async function getLabelsFromLabelerConfig( + context: Context, + labelerConfigTracker: CachedLabelerConfigTracker, + changed_files: string[], + isDraft: boolean = false +): Promise { + const config = await labelerConfigTracker.loadLabelsConfig(context); + const labels: string[] = []; + + for (const [label, rawRule] of Object.entries(config)) { + const rule = normalizeLabelerRule(rawRule); + if (rule === null) { + const skipReason = labelerRuleSkipReason(rawRule); + if (skipReason === "invalid_draft") { + context.log( + { + label, + rawRule, + draft: (rawRule as { draft?: unknown }).draft, + }, + "getLabelsFromLabelerConfig: invalid draft type (expected boolean), skipping" + ); + } else { + context.log( + { label, rawRule }, + "getLabelsFromLabelerConfig: unknown rule shape, skipping" + ); + } + continue; + } + if (!draftConstraintAllowsLabel(rule, isDraft)) { + continue; + } + const globs = globsFromRule(rule); + if ( + globs.some((glob: string) => + changed_files.some((file: string) => minimatch(file, glob)) + ) + ) { + labels.push(label); + } + } + return labels; +} diff --git a/torchci/test/autoLabelBot.test.ts b/torchci/test/autoLabelBot.test.ts index 2c53f75dc8..1ada8bf514 100644 --- a/torchci/test/autoLabelBot.test.ts +++ b/torchci/test/autoLabelBot.test.ts @@ -1183,6 +1183,113 @@ describe("auto-label-bot: labeler.yml config", () => { scope2.done(); handleScope(checkLabelsScope); }); + + test("labeler draft:false skips ciflow-style label on draft PR", async () => { + const event = requireDeepCopy("./fixtures/pull_request.opened"); + event.payload.pull_request.draft = true; + const prFiles = requireDeepCopy("./fixtures/pull_files"); + prFiles["items"] = [{ filename: "torch/_dynamo/blah.py" }]; + const repoFullName = "zhouzhuojie/gha-ci-playground"; + const prNumber = 31; + const scope = mockChangedFiles(prFiles, prNumber, repoFullName); + const config = ` +"module: dynamo": +- torch/_dynamo/** + +"ciflow/inductor": + globs: + - torch/_dynamo/** + draft: false +`; + utils.mockConfig( + "pytorch-probot.yml", + "labeler_config: labeler.yml", + repoFullName + ); + utils.mockConfig("labeler.yml", config, repoFullName); + utils.mockHasApprovedWorkflowRun(repoFullName); + const scope2 = utils.mockAddLabels( + ["module: dynamo"], + repoFullName, + prNumber + ); + const checkLabelsScope = mockCheckLabelsComment(repoFullName, prNumber); + await probot.receive(event); + scope.done(); + scope2.done(); + handleScope(checkLabelsScope); + }); + + test("labeler draft:false applies ciflow-style label when PR is not draft", async () => { + const event = requireDeepCopy("./fixtures/pull_request.opened"); + event.payload.pull_request.draft = false; + const prFiles = requireDeepCopy("./fixtures/pull_files"); + prFiles["items"] = [{ filename: "torch/_dynamo/blah.py" }]; + const repoFullName = "zhouzhuojie/gha-ci-playground"; + const prNumber = 31; + const scope = mockChangedFiles(prFiles, prNumber, repoFullName); + const config = ` +"module: dynamo": +- torch/_dynamo/** + +"ciflow/inductor": + globs: + - torch/_dynamo/** + draft: false +`; + utils.mockConfig( + "pytorch-probot.yml", + "labeler_config: labeler.yml", + repoFullName + ); + utils.mockConfig("labeler.yml", config, repoFullName); + utils.mockHasApprovedWorkflowRun(repoFullName); + const scope2 = utils.mockAddLabels( + ["module: dynamo", "ciflow/inductor"], + repoFullName, + prNumber + ); + const checkLabelsScope = mockCheckLabelsComment(repoFullName, prNumber); + await probot.receive(event); + scope.done(); + scope2.done(); + handleScope(checkLabelsScope); + }); + + test("labeler draft:false applies ciflow-style label on ready_for_review", async () => { + const event = requireDeepCopy("./fixtures/pull_request.opened"); + event.payload.action = "ready_for_review"; + event.payload.pull_request.draft = false; + const prFiles = requireDeepCopy("./fixtures/pull_files"); + prFiles["items"] = [{ filename: "torch/_dynamo/blah.py" }]; + const repoFullName = "zhouzhuojie/gha-ci-playground"; + const prNumber = 31; + const scope = mockChangedFiles(prFiles, prNumber, repoFullName); + const config = ` +"module: dynamo": +- torch/_dynamo/** + +"ciflow/inductor": + globs: + - torch/_dynamo/** + draft: false +`; + utils.mockConfig( + "pytorch-probot.yml", + "labeler_config: labeler.yml", + repoFullName + ); + utils.mockConfig("labeler.yml", config, repoFullName); + utils.mockHasApprovedWorkflowRun(repoFullName); + const scope2 = utils.mockAddLabels( + ["module: dynamo", "ciflow/inductor"], + repoFullName, + prNumber + ); + await probot.receive(event); + scope.done(); + scope2.done(); + }); }); describe("auto-label-bot: label-to-label.yml config", () => { diff --git a/torchci/test/labelerConfigUtils.test.ts b/torchci/test/labelerConfigUtils.test.ts new file mode 100644 index 0000000000..cb22cdffe8 --- /dev/null +++ b/torchci/test/labelerConfigUtils.test.ts @@ -0,0 +1,133 @@ +import { + draftConstraintAllowsLabel, + getLabelsFromLabelerConfig, + normalizeLabelerRule, +} from "lib/bot/labelerConfigUtils"; + +describe("labelerConfigUtils", () => { + describe("normalizeLabelerRule", () => { + test("accepts legacy glob array", () => { + expect(normalizeLabelerRule(["a/**", "b/**"])).toEqual(["a/**", "b/**"]); + }); + + test("accepts object with globs and draft", () => { + expect(normalizeLabelerRule({ globs: ["x/**"], draft: false })).toEqual({ + globs: ["x/**"], + draft: false, + }); + }); + + test("rejects non-boolean draft", () => { + expect(normalizeLabelerRule({ globs: ["x/**"], draft: "false" })).toBeNull(); + }); + + test("rejects invalid shapes", () => { + expect(normalizeLabelerRule(null)).toBeNull(); + expect(normalizeLabelerRule({ globs: "bad" })).toBeNull(); + }); + }); + + describe("draftConstraintAllowsLabel", () => { + test("legacy array has no draft constraint", () => { + expect(draftConstraintAllowsLabel(["**"], true)).toBe(true); + expect(draftConstraintAllowsLabel(["**"], false)).toBe(true); + }); + + test("draft false applies only when not draft", () => { + const rule = { globs: ["**"], draft: false as const }; + expect(draftConstraintAllowsLabel(rule, true)).toBe(false); + expect(draftConstraintAllowsLabel(rule, false)).toBe(true); + }); + + test("draft true applies only on drafts", () => { + const rule = { globs: ["**"], draft: true as const }; + expect(draftConstraintAllowsLabel(rule, true)).toBe(true); + expect(draftConstraintAllowsLabel(rule, false)).toBe(false); + }); + }); + + describe("getLabelsFromLabelerConfig", () => { + test("skips label when draft:false and PR is draft", async () => { + const tracker = { + loadLabelsConfig: async () => ({ + "ciflow/x": { + globs: ["torch/**"], + draft: false, + }, + }), + }; + const context = { log: jest.fn() } as any; + const labels = await getLabelsFromLabelerConfig( + context, + tracker as any, + ["torch/a.py"], + true + ); + expect(labels).toEqual([]); + }); + + test("adds label when draft:false and PR is not draft", async () => { + const tracker = { + loadLabelsConfig: async () => ({ + "ciflow/x": { + globs: ["torch/**"], + draft: false, + }, + }), + }; + const context = { log: jest.fn() } as any; + const labels = await getLabelsFromLabelerConfig( + context, + tracker as any, + ["torch/a.py"], + false + ); + expect(labels).toEqual(["ciflow/x"]); + }); + + test("skips rule with invalid draft type and logs distinct message", async () => { + const tracker = { + loadLabelsConfig: async () => ({ + "ciflow/x": { + globs: ["torch/**"], + draft: "false", + }, + }), + }; + const context = { log: jest.fn() } as any; + const labels = await getLabelsFromLabelerConfig( + context, + tracker as any, + ["torch/a.py"], + false + ); + expect(labels).toEqual([]); + expect(context.log).toHaveBeenCalledWith( + { + label: "ciflow/x", + rawRule: { globs: ["torch/**"], draft: "false" }, + draft: "false", + }, + "getLabelsFromLabelerConfig: invalid draft type (expected boolean), skipping" + ); + }); + + test("defaults isDraft to false when omitted (3-arg call)", async () => { + const tracker = { + loadLabelsConfig: async () => ({ + "ciflow/x": { + globs: ["torch/**"], + draft: false, + }, + }), + }; + const context = { log: jest.fn() } as any; + const labels = await getLabelsFromLabelerConfig( + context, + tracker as any, + ["torch/a.py"] + ); + expect(labels).toEqual(["ciflow/x"]); + }); + }); +});