Skip to content

Commit

Permalink
[bots] PR gets ciflow/trunk on import (#5836)
Browse files Browse the repository at this point in the history
Continuation of #5795

There was a case where the pr body didn't get edited when the PR was
imported, so we should also handle this case, which I identify by seeing
if the comment got added

I think the rule is :
* ghimport -> pr body gets edited
* normal import using button -> pr body doesn't get edited

Random but I think ghimport uses 2 spaces after the period, but the
facebook-github-bot uses 1
  • Loading branch information
clee2000 authored Oct 28, 2024
1 parent d36c29d commit 4999506
Show file tree
Hide file tree
Showing 2 changed files with 159 additions and 3 deletions.
51 changes: 48 additions & 3 deletions torchci/lib/bot/autoLabelCodevTrunk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@
import { Context, Probot } from "probot";
import { addNewLabels, canRunWorkflows } from "./autoLabelBot";
import { CODEV_INDICATOR } from "./codevNoWritePermBot";
import { isPyTorchPyTorch } from "./utils";
import {
hasApprovedPullRuns,
hasWritePermissions,
isPyTorchPyTorch,
} from "./utils";

const CIFLOW_TRUNK_LABEL = "ciflow/trunk";

Expand All @@ -13,7 +17,6 @@ async function doCodevLabeling(
) {
const owner = context.payload.repository.owner.login;
const repo = context.payload.repository.name;
const prAuthor = context.payload.pull_request.user.login;
const body = context.payload.pull_request.body;
const existingLabels = context.payload.pull_request.labels.map(
(e) => e["name"]
Expand Down Expand Up @@ -46,13 +49,55 @@ function myBot(app: Probot): void {
});

app.on("pull_request.edited", async (context) => {
// Apply `ciflow/trunk` to PRs that have just been imported.
// Apply `ciflow/trunk` to PRs that have just been imported. It is unclear
// if the PR body is edited.
if (context.payload.changes.body?.from.match(CODEV_INDICATOR)) {
// Already exists, no need to add again
return;
}
await doCodevLabeling(context);
});

app.on("issue_comment.created", async (context) => {
// Apply `ciflow/trunk` to PRs that have just been imported
if (
context.payload.comment.user.login == "facebook-github-bot" &&
context.payload.comment.body.match("has imported this pull request") &&
context.payload.issue.pull_request
) {
const owner = context.payload.repository.owner.login;
const repo = context.payload.repository.name;
const existingLabels = context.payload.issue.labels.map((e) => e["name"]);

if (!isPyTorchPyTorch(owner, repo)) {
return;
}

const asPR = await context.octokit.pulls.get({
owner,
repo,
pull_number: context.payload.issue.number,
});

// Here we can't use the usual canRunWorkflows because the context type is
// different, so we have to check the permissions manually
if (
(await hasApprovedPullRuns(
context.octokit,
asPR.data.base.repo.owner.login,
asPR.data.base.repo.name,
asPR.data.head.sha
)) ||
(await hasWritePermissions(context, asPR.data.user?.login ?? ""))
) {
await addNewLabels(
existingLabels,
[CIFLOW_TRUNK_LABEL],
context as any
);
}
}
});
}

export default myBot;
111 changes: 111 additions & 0 deletions torchci/test/autoLabelCodevTrunk.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,4 +175,115 @@ describe("autoLabelCodevTrunkBot", () => {

scope.done();
});

test("PR does not add ciflow/trunk on random comment (wrong content)", async () => {
const event = requireDeepCopy("./fixtures/pull_request_comment.json");
event.payload.body = "random";
event.payload.comment.user.login = "facebook-github-bot";

nock("https://api.github.com")
.post("/app/installations/2/access_tokens")
.reply(200, { token: "test" });

const scope = nock("https://api.github.com");
await probot.receive(event);

scope.done();
});

test("PR does not add ciflow/trunk on random comment (wrong user)", async () => {
const event = requireDeepCopy("./fixtures/pull_request_comment.json");
event.payload.comment.body =
"@clee2000 has imported this pull request. If you are a Meta employee, you can view this diff [on Phabricator](https://www.internalfb.com/diff/D64475068).";
event.payload.comment.user.login = "random user";

nock("https://api.github.com")
.post("/app/installations/2/access_tokens")
.reply(200, { token: "test" });

const scope = nock("https://api.github.com");
await probot.receive(event);

scope.done();
});

test("PR does not add ciflow/trunk on comment (no permissions)", async () => {
const event = requireDeepCopy("./fixtures/pull_request_comment.json");
event.payload.comment.body =
"@clee2000 has imported this pull request. If you are a Meta employee, you can view this diff [on Phabricator](https://www.internalfb.com/diff/D64475068).";
event.payload.comment.user.login = "facebook-github-bot";
const owner = "clee2000";
const repo = "random-testing";
event.payload.repository.owner.login = owner;
event.payload.repository.name = repo;
const repoFullName = `${owner}/${repo}`;
const author = event.payload.issue.user.login;
const headSha = "random";

nock("https://api.github.com")
.post("/app/installations/2/access_tokens")
.reply(200, { token: "test" });

const scope = [
utils.mockGetPR(repoFullName, event.payload.issue.number, {
user: { login: author },
head: { sha: headSha },
base: { repo: { name: repo, owner: { login: owner } } },
}),
utils.mockPermissions(repoFullName, author, "read"),
utils.mockApprovedWorkflowRuns(repoFullName, headSha, false),
];
await probot.receive(event);

handleScope(scope);
});

test("PR adds ciflow/trunk on comment", async () => {
const event = requireDeepCopy("./fixtures/pull_request_comment.json");
event.payload.comment.body =
"@clee2000 has imported this pull request. If you are a Meta employee, you can view this diff [on Phabricator](https://www.internalfb.com/diff/D64475068).";
event.payload.comment.user.login = "facebook-github-bot";
const owner = "clee2000";
const repo = "random-testing";
event.payload.repository.owner.login = owner;
event.payload.repository.name = repo;
const repoFullName = `${owner}/${repo}`;
const author = event.payload.issue.user.login;
const headSha = "random";

nock("https://api.github.com")
.post("/app/installations/2/access_tokens")
.reply(200, { token: "test" });

const scope = [
utils.mockGetPR(repoFullName, event.payload.issue.number, {
user: { login: author },
head: { sha: headSha },
base: { repo: { name: repo, owner: { login: owner } } },
}),
utils.mockApprovedWorkflowRuns(repoFullName, headSha, true),
utils.mockAddLabels(
["ciflow/trunk"],
repoFullName,
event.payload.issue.number
),
];
await probot.receive(event);

handleScope(scope);
});

test("Does not add ciflow/trunk label on comment if not a PR", async () => {
const event = requireDeepCopy("./fixtures/issue_comment.json");
event.payload.comment.body =
"@clee2000 has imported this pull request. If you are a Meta employee, you can view this diff [on Phabricator](https://www.internalfb.com/diff/D64475068).";
event.payload.comment.user.login = "facebook-github-bot";
delete event.payload.issue.pull_request;
event.payload.comment.body = "@zhouzhuojie has imported this pull request.";

const scope = nock("https://api.github.com");
await probot.receive(event);

scope.done();
});
});

0 comments on commit 4999506

Please sign in to comment.