Skip to content

Commit

Permalink
[disablebot] Exclusion for oncall: pt2 as removable platform label (#…
Browse files Browse the repository at this point in the history
…5578)

The disable bot automatically tries to delete labels it thinks is no
longer relevant. Usually it makes sense, like removing module: windows
if the platforms change to not include windows. Unfortunately it also
puts oncall: pt2 in this category, but oncall: pt2 can also be a test
owner label, so it should stay
  • Loading branch information
clee2000 authored Aug 20, 2024
1 parent 0f8ffc7 commit 0c3a263
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 14 deletions.
13 changes: 9 additions & 4 deletions torchci/lib/bot/verifyDisableTestIssueBot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,17 @@ async function getValidationComment(
return [0, ""];
}

// Returns the platform labels that are expected, and invalid labels that we do not expect to be there
export function getExpectedPlatformLabels(
// Returns the platform module labels that are expected, and invalid labels that we do not expect to be there
export function getExpectedPlatformModuleLabels(
platforms: string[],
labels: string[]
): [string[], string[]] {
let supportedPlatformLabels = Array.from(supportedPlatforms.values()).flat();
let supportedPlatformLabels = Array.from(supportedPlatforms.values())
.flat()
// Quick hack to make sure oncall: pt2 doesn't get deleted.
// TODO: figure out a better way to differentiate between labels that should
// stay and labels that shouldn't
.filter((label) => label.startsWith("module: "));
let existingPlatformLabels = labels.filter((label) =>
supportedPlatformLabels.includes(label)
);
Expand Down Expand Up @@ -302,7 +307,7 @@ export default function verifyDisableTestIssueBot(app: Probot): void {
} else {
// check labels, add labels as needed
let [expectedPlatformLabels, invalidPlatformLabels] =
getExpectedPlatformLabels(platformsToSkip, labels);
getExpectedPlatformModuleLabels(platformsToSkip, labels);
let labelsSet = new Set(labels);
if (!expectedPlatformLabels.every((label) => labelsSet.has(label))) {
await context.octokit.issues.addLabels({
Expand Down
31 changes: 21 additions & 10 deletions torchci/test/verifyDisableTestIssue.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -325,32 +325,43 @@ describe("verify-disable-test-issue", () => {
expect(comment.includes("ERROR")).toBeFalsy();
});

test("various getExpectedPlatformLabels tests", async () => {
expect(await bot.getExpectedPlatformLabels(["linux"], ["random"])).toEqual([
[],
[],
]);
test("various getExpectedPlatformModuleLabels tests", async () => {
expect(
await bot.getExpectedPlatformLabels(["inductor"], ["random"])
await bot.getExpectedPlatformModuleLabels(["linux"], ["random"])
).toEqual([[], []]);
expect(
await bot.getExpectedPlatformModuleLabels(["inductor"], ["random"])
).toEqual([["oncall: pt2"], []]);
expect(
await bot.getExpectedPlatformLabels(["linux"], ["random", "module: rocm"])
await bot.getExpectedPlatformModuleLabels(
["linux"],
["random", "module: rocm"]
)
).toEqual([[], ["module: rocm"]]);
expect(
await bot.getExpectedPlatformLabels(["rocm"], ["random", "module: rocm"])
await bot.getExpectedPlatformModuleLabels(
["rocm"],
["random", "module: rocm"]
)
).toEqual([["module: rocm"], []]);
expect(
await bot.getExpectedPlatformLabels(
await bot.getExpectedPlatformModuleLabels(
["dynamo", "inductor"],
["random", "module: rocm"]
)
).toEqual([["oncall: pt2"], ["module: rocm"]]);
expect(
await bot.getExpectedPlatformLabels(
await bot.getExpectedPlatformModuleLabels(
["linux", "rocm"],
["random", "module: rocm"]
)
).toEqual([[], ["module: rocm"]]);
expect(
await bot.getExpectedPlatformModuleLabels(
["linux", "rocm"],
["random", "module: rocm", "oncall: pt2"]
)
).toEqual([[], ["module: rocm"]]);
});
});

Expand Down

0 comments on commit 0c3a263

Please sign in to comment.