Skip to content

Commit c8927d2

Browse files
clee2000clee2000
authored
[mergebot] Set mergebot to be configured via config (#5312)
For repos that want to merge via `@pytorchbot merge`, the repo should put `mergebot: True` in their `pytorch-probot.yml` In preparation for letting torchao trigger their own merges through bot Risks: If we can't query for the config for some reason, it will not merge todo before this gets merged: Add `mergebot: True` in pytorch's config file
1 parent cca2400 commit c8927d2

File tree

4 files changed

+144
-2
lines changed

4 files changed

+144
-2
lines changed

‎torchci/lib/bot/pytorchBot.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
import { Probot } from "probot";
22
import { getInputArgs } from "./cliParser";
33
import PytorchBotHandler from "./pytorchBotHandler";
4+
import { CachedConfigTracker } from "./utils";
45

56
function pytorchBot(app: Probot): void {
7+
const cachedConfigTracker = new CachedConfigTracker(app);
8+
69
app.on("issue_comment.created", async (ctx) => {
710
const commentBody = ctx.payload.comment.body;
811
const owner = ctx.payload.repository.owner.login;
@@ -20,6 +23,7 @@ function pytorchBot(app: Probot): void {
2023
commentId,
2124
commentBody,
2225
useReactions: true,
26+
cachedConfigTracker,
2327
});
2428
const is_pr_comment = ctx.payload.issue.pull_request != null;
2529
const skipUsers = [
@@ -57,6 +61,7 @@ function pytorchBot(app: Probot): void {
5761
commentId,
5862
commentBody: reviewBody ?? "",
5963
useReactions: false,
64+
cachedConfigTracker,
6065
});
6166
if (reviewBody == null) {
6267
return;

‎torchci/lib/bot/pytorchBotHandler.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { cherryPickClassifications } from "./Constants";
77
import PytorchBotLogger from "./pytorchbotLogger";
88
import {
99
addLabels,
10+
CachedConfigTracker,
1011
hasApprovedPullRuns,
1112
hasWritePermissions as _hasWP,
1213
isFirstTimeContributor,
@@ -27,6 +28,7 @@ export interface PytorchbotParams {
2728
commentId: number;
2829
commentBody: string;
2930
useReactions: boolean;
31+
cachedConfigTracker: CachedConfigTracker;
3032
}
3133

3234
const PR_COMMENTED = "commented";
@@ -45,6 +47,7 @@ class PytorchBotHandler {
4547
login: string;
4648
commentBody: string;
4749
headSha: string | undefined;
50+
cachedConfigTracker: CachedConfigTracker;
4851

4952
forceMergeMessagePat = new RegExp("^\\s*\\S+\\s+\\S+.*");
5053

@@ -60,6 +63,7 @@ class PytorchBotHandler {
6063
this.commentId = params.commentId;
6164
this.commentBody = params.commentBody;
6265
this.useReactions = params.useReactions;
66+
this.cachedConfigTracker = params.cachedConfigTracker;
6367

6468
this.logger = new PytorchBotLogger(params);
6569
}
@@ -243,6 +247,15 @@ The explanation needs to be clear on why this is needed. Here are some good exam
243247
rebase: string | boolean,
244248
ic: boolean
245249
) {
250+
const config: any = await this.cachedConfigTracker.loadConfig(this.ctx);
251+
if (config == null || !config["mergebot"]) {
252+
await this.handleConfused(
253+
true,
254+
"Mergebot is not configured for this repository. Please use the merge button provided by GitHub."
255+
);
256+
return;
257+
}
258+
246259
const extra_data = {
247260
forceMessage,
248261
rebase,

‎torchci/test/mergeCommands.test.ts‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ describe("merge-bot", () => {
1313
beforeEach(() => {
1414
probot = utils.testProbot();
1515
probot.load(pytorchBot);
16+
utils.mockConfig("pytorch-probot.yml", "mergebot: True");
1617
});
1718

1819
afterEach(() => {
@@ -1499,3 +1500,119 @@ some other text lol
14991500
handleScope(scope);
15001501
});
15011502
});
1503+
1504+
describe("merge-bot not supported repo", () => {
1505+
let probot: probot.Probot;
1506+
1507+
beforeEach(() => {
1508+
probot = utils.testProbot();
1509+
probot.load(pytorchBot);
1510+
});
1511+
1512+
afterEach(() => {
1513+
nock.cleanAll();
1514+
jest.restoreAllMocks();
1515+
});
1516+
1517+
test("no config", async () => {
1518+
const event = requireDeepCopy("./fixtures/pull_request_comment.json");
1519+
1520+
event.payload.comment.body = "@pytorchbot merge";
1521+
event.payload.repository.owner.login = "pytorch";
1522+
event.payload.repository.name = "pytorch";
1523+
1524+
const owner = event.payload.repository.owner.login;
1525+
const repo = event.payload.repository.name;
1526+
const pr_number = event.payload.issue.number;
1527+
const comment_number = event.payload.comment.id;
1528+
const scopes = [
1529+
nock("https://api.github.com")
1530+
.get(`/repos/${owner}/${repo}/contents/.github%2Fpytorch-probot.yml`)
1531+
.reply(200, {
1532+
message: "Not Found",
1533+
documentation_url:
1534+
"https://docs.github.com/rest/repos/contents#get-repository-content",
1535+
})
1536+
.post(
1537+
`/repos/${owner}/${repo}/issues/comments/${comment_number}/reactions`,
1538+
(body) => {
1539+
expect(JSON.stringify(body)).toContain('{"content":"confused"}');
1540+
return true;
1541+
}
1542+
)
1543+
.reply(200, {}),
1544+
utils.mockPostComment(`${owner}/${repo}`, pr_number, [
1545+
"Mergebot is not configured for this repository",
1546+
]),
1547+
];
1548+
1549+
await probot.receive(event);
1550+
handleScope(scopes);
1551+
});
1552+
1553+
test("config does not have mergebot key", async () => {
1554+
const event = requireDeepCopy("./fixtures/pull_request_comment.json");
1555+
1556+
event.payload.comment.body = "@pytorchbot merge";
1557+
event.payload.repository.owner.login = "pytorch";
1558+
event.payload.repository.name = "pytorch";
1559+
1560+
const owner = event.payload.repository.owner.login;
1561+
const repo = event.payload.repository.name;
1562+
const pr_number = event.payload.issue.number;
1563+
const comment_number = event.payload.comment.id;
1564+
utils.mockConfig("pytorch-probot.yml", "hello: true", `${owner}/${repo}`);
1565+
const scopes = [
1566+
nock("https://api.github.com")
1567+
.post(
1568+
`/repos/${owner}/${repo}/issues/comments/${comment_number}/reactions`,
1569+
(body) => {
1570+
expect(JSON.stringify(body)).toContain('{"content":"confused"}');
1571+
return true;
1572+
}
1573+
)
1574+
.reply(200, {}),
1575+
utils.mockPostComment(`${owner}/${repo}`, pr_number, [
1576+
"Mergebot is not configured for this repository",
1577+
]),
1578+
];
1579+
1580+
await probot.receive(event);
1581+
handleScope(scopes);
1582+
});
1583+
1584+
test("config mergebot key set to false", async () => {
1585+
const event = requireDeepCopy("./fixtures/pull_request_comment.json");
1586+
1587+
event.payload.comment.body = "@pytorchbot merge";
1588+
event.payload.repository.owner.login = "pytorch";
1589+
event.payload.repository.name = "pytorch";
1590+
1591+
const owner = event.payload.repository.owner.login;
1592+
const repo = event.payload.repository.name;
1593+
const pr_number = event.payload.issue.number;
1594+
const comment_number = event.payload.comment.id;
1595+
utils.mockConfig(
1596+
"pytorch-probot.yml",
1597+
"mergebot: False",
1598+
`${owner}/${repo}`
1599+
);
1600+
const scopes = [
1601+
nock("https://api.github.com")
1602+
.post(
1603+
`/repos/${owner}/${repo}/issues/comments/${comment_number}/reactions`,
1604+
(body) => {
1605+
expect(JSON.stringify(body)).toContain('{"content":"confused"}');
1606+
return true;
1607+
}
1608+
)
1609+
.reply(200, {}),
1610+
utils.mockPostComment(`${owner}/${repo}`, pr_number, [
1611+
"Mergebot is not configured for this repository",
1612+
]),
1613+
];
1614+
1615+
await probot.receive(event);
1616+
handleScope(scopes);
1617+
});
1618+
});

‎torchci/test/utils.ts‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,22 @@ export function testOctokit(): Octokit {
2525
export function mockConfig(
2626
fileName: string,
2727
content: string,
28-
repoKey: string
28+
repoKey: string | RegExp = ".*"
2929
): void {
3030
const configPayload = require("./fixtures/config.json");
3131
configPayload["content"] = Buffer.from(content).toString("base64");
3232
configPayload["name"] = fileName;
3333
configPayload["path"] = `.github/${fileName}`;
3434
nock("https://api.github.com")
3535
.get(
36-
`/repos/${repoKey}/contents/${encodeURIComponent(`.github/${fileName}`)}`
36+
// The use of regex here means that if the repokey or the filename contain
37+
// regex special characters, they will be viewed as regex. The main one
38+
// to worry about is `.` but I think it will cause minimal problems
39+
RegExp(
40+
`/repos/${repoKey}/contents/${encodeURIComponent(
41+
`.github/${fileName}`
42+
)}`
43+
)
3744
)
3845
.times(2)
3946
.reply(200, content);

0 commit comments

Comments
 (0)