Skip to content

Commit 39dc5db

Browse files
authored
[bots] Testing utilities: Partial type, some helper functions for flakybot (#6155)
A few testing utilities to hopefully making writing bot tests easier * Use Partial type for testing helper functions that generate dummy data https://www.typescriptlang.org/docs/handbook/utility-types.html#partialtype * Add mockCreateIssue to mock creating an issue on Github, and use it in flakybot * Add mockGetRawTestFile for flakybot for mocking retrieving the raw test file for oncall labels * Use handleScope in flakybot tests
1 parent 21cb495 commit 39dc5db

File tree

2 files changed

+107
-149
lines changed

2 files changed

+107
-149
lines changed

torchci/test/disableFlakyBot.test.ts

Lines changed: 84 additions & 136 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,12 @@ const nonFlakyTestZ = {
102102
num_red: 0,
103103
};
104104

105+
function mockGetRawTestFile(file: string, content: string) {
106+
return nock("https://raw.githubusercontent.com")
107+
.get(`/pytorch/pytorch/main/test/${file}`)
108+
.reply(200, Buffer.from(content));
109+
}
110+
105111
describe("Disable Flaky Test Bot Across Jobs", () => {
106112
const octokit = utils.testOctokit();
107113

@@ -112,28 +118,22 @@ describe("Disable Flaky Test Bot Across Jobs", () => {
112118
});
113119

114120
test("Create new issue", async () => {
115-
const scope = nock("https://raw.githubusercontent.com")
116-
.get(`/pytorch/pytorch/main/test/${flakyTestAcrossJobA.file}`)
117-
.reply(
118-
200,
119-
Buffer.from(`# Owner(s): ["module: fft"]\nimport blah;\nrest of file`)
120-
);
121-
const scope2 = nock("https://api.github.com")
122-
.post("/repos/pytorch/pytorch/issues", (body) => {
123-
expect(body.title).toEqual(
124-
"DISABLED test_conv1d_vs_scipy_mode_same_cuda_complex64 (__main__.TestConvolutionNNDeviceTypeCUDA)"
125-
);
126-
expect(body.labels).toEqual([
127-
"skipped",
128-
"module: flaky-tests",
129-
"module: fft",
130-
"module: rocm",
131-
"triaged",
132-
]);
133-
expect(JSON.stringify(body.body)).toContain("Platforms: ");
134-
return true;
135-
})
136-
.reply(200, {});
121+
const scope = mockGetRawTestFile(
122+
flakyTestAcrossJobA.file,
123+
`# Owner(s): ["module: fft"]\nimport blah;\nrest of file`
124+
);
125+
const scope2 = utils.mockCreateIssue(
126+
"pytorch/pytorch",
127+
"DISABLED test_conv1d_vs_scipy_mode_same_cuda_complex64 (__main__.TestConvolutionNNDeviceTypeCUDA)",
128+
["Platforms: "],
129+
[
130+
"skipped",
131+
"module: flaky-tests",
132+
"module: fft",
133+
"module: rocm",
134+
"triaged",
135+
]
136+
);
137137

138138
await disableFlakyTestBot.handleFlakyTest(flakyTestAcrossJobA, [], octokit);
139139

@@ -266,26 +266,22 @@ describe("Disable Flaky Test Bot Integration Tests", () => {
266266
});
267267

268268
test("previously undetected flaky test should create an issue", async () => {
269-
const scope = nock("https://raw.githubusercontent.com")
270-
.get(`/pytorch/pytorch/main/test/${flakyTestA.file}`)
271-
.reply(
272-
200,
273-
Buffer.from(`# Owner(s): ["module: fft"]\nimport blah;\nrest of file`)
274-
);
275-
const scope2 = nock("https://api.github.com")
276-
.post("/repos/pytorch/pytorch/issues", (body) => {
277-
expect(body.title).toEqual("DISABLED test_a (__main__.suite_a)");
278-
expect(body.labels).toEqual([
279-
"skipped",
280-
"module: flaky-tests",
281-
"module: fft",
282-
"module: windows",
283-
"triaged",
284-
]);
285-
expect(JSON.stringify(body.body)).toContain("Platforms: ");
286-
return true;
287-
})
288-
.reply(200, {});
269+
const scope = mockGetRawTestFile(
270+
flakyTestA.file,
271+
`# Owner(s): ["module: fft"]\nimport blah;\nrest of file`
272+
);
273+
const scope2 = utils.mockCreateIssue(
274+
"pytorch/pytorch",
275+
"DISABLED test_a (__main__.suite_a)",
276+
["Platforms: "],
277+
[
278+
"skipped",
279+
"module: flaky-tests",
280+
"module: fft",
281+
"module: windows",
282+
"triaged",
283+
]
284+
);
289285

290286
await disableFlakyTestBot.handleFlakyTest(flakyTestA, [], octokit);
291287

@@ -294,26 +290,22 @@ describe("Disable Flaky Test Bot Integration Tests", () => {
294290
});
295291

296292
test("previously undetected flaky test should create an issue on main", async () => {
297-
const scope = nock("https://raw.githubusercontent.com")
298-
.get(`/pytorch/pytorch/main/test/${flakyTestB.file}`)
299-
.reply(
300-
200,
301-
Buffer.from(`# Owner(s): ["module: fft"]\nimport blah;\nrest of file`)
302-
);
303-
const scope2 = nock("https://api.github.com")
304-
.post("/repos/pytorch/pytorch/issues", (body) => {
305-
expect(body.title).toEqual("DISABLED test_b (__main__.suite_b)");
306-
expect(body.labels).toEqual([
307-
"skipped",
308-
"module: flaky-tests",
309-
"module: fft",
310-
"module: windows",
311-
"triaged",
312-
]);
313-
expect(JSON.stringify(body.body)).toContain("Platforms: ");
314-
return true;
315-
})
316-
.reply(200, {});
293+
const scope = mockGetRawTestFile(
294+
flakyTestB.file,
295+
`# Owner(s): ["module: fft"]\nimport blah;\nrest of file`
296+
);
297+
const scope2 = utils.mockCreateIssue(
298+
"pytorch/pytorch",
299+
"DISABLED test_b (__main__.suite_b)",
300+
["Platforms:"],
301+
[
302+
"skipped",
303+
"module: flaky-tests",
304+
"module: fft",
305+
"module: windows",
306+
"triaged",
307+
]
308+
);
317309

318310
await disableFlakyTestBot.handleFlakyTest(flakyTestB, [], octokit);
319311

@@ -567,33 +559,23 @@ describe("Disable Flaky Test Bot Unit Tests", () => {
567559
});
568560

569561
test("getTestOwnerLabels: owned test file should return proper module and be triaged", async () => {
570-
const scope = nock("https://raw.githubusercontent.com/")
571-
.get(`/pytorch/pytorch/main/test/${flakyTestA.file}`)
572-
.reply(
573-
200,
574-
Buffer.from(`# Owner(s): ["module: fft"]\nimport blah;\nrest of file`)
575-
);
576-
562+
const scope = mockGetRawTestFile(
563+
flakyTestA.file,
564+
`# Owner(s): ["module: fft"]\nimport blah;\nrest of file`
565+
);
577566
const { labels, additionalErrMessage } =
578567
await disableFlakyTestBot.getTestOwnerLabels(flakyTestA);
579568
expect(additionalErrMessage).toEqual(undefined);
580569
expect(labels).toEqual(["module: fft", "module: windows", "triaged"]);
581570

582-
if (!scope.isDone()) {
583-
console.error("pending mocks: %j", scope.pendingMocks());
584-
}
585-
scope.done();
571+
handleScope(scope);
586572
});
587573

588574
test("getTestOwnerLabels: owned test file should route to oncall and NOT be triaged", async () => {
589-
const scope = nock("https://raw.githubusercontent.com/")
590-
.get(`/pytorch/pytorch/main/test/${flakyTestA.file}`)
591-
.reply(
592-
200,
593-
Buffer.from(
594-
`# Owner(s): ["oncall: distributed"]\nimport blah;\nrest of file`
595-
)
596-
);
575+
const scope = mockGetRawTestFile(
576+
flakyTestA.file,
577+
`# Owner(s): ["oncall: distributed"]\nimport blah;\nrest of file`
578+
);
597579

598580
const { labels } = await disableFlakyTestBot.getTestOwnerLabels(flakyTestA);
599581
expect(labels).toEqual([
@@ -602,50 +584,35 @@ describe("Disable Flaky Test Bot Unit Tests", () => {
602584
"triaged",
603585
]);
604586

605-
if (!scope.isDone()) {
606-
console.error("pending mocks: %j", scope.pendingMocks());
607-
}
608-
scope.done();
587+
handleScope(scope);
609588
});
610589

611590
test("getTestOwnerLabels: un-owned test file should return module: unknown", async () => {
612-
const scope = nock("https://raw.githubusercontent.com/")
613-
.get(`/pytorch/pytorch/main/test/${flakyTestA.file}`)
614-
.reply(
615-
200,
616-
Buffer.from(
617-
`# Owner(s): ["module: unknown"]\nimport blah;\nrest of file`
618-
)
619-
);
591+
const scope = mockGetRawTestFile(
592+
flakyTestA.file,
593+
`# Owner(s): ["module: unknown"]\nimport blah;\nrest of file`
594+
);
620595

621596
const { labels, additionalErrMessage } =
622597
await disableFlakyTestBot.getTestOwnerLabels(flakyTestA);
623598
expect(labels).toEqual(["module: unknown", "module: windows", "triaged"]);
624599
expect(additionalErrMessage).toEqual(undefined);
625600

626-
if (!scope.isDone()) {
627-
console.error("pending mocks: %j", scope.pendingMocks());
628-
}
629-
scope.done();
601+
handleScope(scope);
630602
});
631603

632604
test("getTestOwnerLabels: ill-formatted file should return module: unknown", async () => {
633-
const scope = nock("https://raw.githubusercontent.com/")
634-
.get(`/pytorch/pytorch/main/test/${flakyTestA.file}`)
635-
.reply(
636-
200,
637-
Buffer.from("line1\nline2\nline3\nstill no owners\nline4\nlastline\n")
638-
);
605+
const scope = mockGetRawTestFile(
606+
flakyTestA.file,
607+
`line1\nline2\nline3\nstill no owners\nline4\nlastline\n`
608+
);
639609

640610
const { labels, additionalErrMessage } =
641611
await disableFlakyTestBot.getTestOwnerLabels(flakyTestA);
642612
expect(labels).toEqual(["module: windows", "triaged"]);
643613
expect(additionalErrMessage).toEqual(undefined);
644614

645-
if (!scope.isDone()) {
646-
console.error("pending mocks: %j", scope.pendingMocks());
647-
}
648-
scope.done();
615+
handleScope(scope);
649616
});
650617

651618
test("getTestOwnerLabels: retry getting file fails all times", async () => {
@@ -674,10 +641,7 @@ describe("Disable Flaky Test Bot Unit Tests", () => {
674641
"Error: Error retrieving file_a.py: 404, file_a: 404"
675642
);
676643

677-
if (!scope.isDone()) {
678-
console.error("pending mocks: %j", scope.pendingMocks());
679-
}
680-
scope.done();
644+
handleScope(scope);
681645
});
682646

683647
test("getTestOwnerLabels: retry getting file", async () => {
@@ -694,10 +658,7 @@ describe("Disable Flaky Test Bot Unit Tests", () => {
694658
expect(labels).toEqual(["module: fft", "module: windows", "triaged"]);
695659
expect(additionalErrMessage).toEqual(undefined);
696660

697-
if (!scope.isDone()) {
698-
console.error("pending mocks: %j", scope.pendingMocks());
699-
}
700-
scope.done();
661+
handleScope(scope);
701662
});
702663

703664
test("getTestOwnerLabels: fallback to invoking file when retrieving file", async () => {
@@ -720,22 +681,17 @@ describe("Disable Flaky Test Bot Unit Tests", () => {
720681
expect(labels).toEqual(["module: fft", "module: rocm", "triaged"]);
721682
expect(additionalErrMessage).toEqual(undefined);
722683

723-
if (!scope.isDone()) {
724-
console.error("pending mocks: %j", scope.pendingMocks());
725-
}
726-
scope.done();
684+
handleScope(scope);
727685
});
728686

729687
test("getTestOwnerLabels: give dynamo and inductor oncall: pt2 label", async () => {
730688
const test = { ...flakyTestA };
731689
test.jobNames = ["dynamo linux"];
732690

733-
let scope = nock("https://raw.githubusercontent.com/")
734-
.get(`/pytorch/pytorch/main/test/${test.file}`)
735-
.reply(
736-
200,
737-
Buffer.from(`# Owner(s): ["module: fft"]\nimport blah;\nrest of file`)
738-
);
691+
let scope = mockGetRawTestFile(
692+
test.file,
693+
`# Owner(s): ["module: fft"]\nimport blah;\nrest of file`
694+
);
739695

740696
let { labels, additionalErrMessage } =
741697
await disableFlakyTestBot.getTestOwnerLabels(test);
@@ -749,10 +705,7 @@ describe("Disable Flaky Test Bot Unit Tests", () => {
749705
const test = { ...flakyTestA };
750706
test.jobNames = ["inductor linux"];
751707

752-
let scope = nock("https://raw.githubusercontent.com/")
753-
.get(`/pytorch/pytorch/main/test/${test.file}`)
754-
.reply(200, Buffer.from(`import blah;\nrest of file`));
755-
708+
let scope = mockGetRawTestFile(test.file, `import blah;\nrest of file`);
756709
let { labels, additionalErrMessage } =
757710
await disableFlakyTestBot.getTestOwnerLabels(test);
758711
expect(additionalErrMessage).toEqual(undefined);
@@ -765,10 +718,7 @@ describe("Disable Flaky Test Bot Unit Tests", () => {
765718
const test = { ...flakyTestA };
766719
test.jobNames = ["rocm"];
767720

768-
let scope = nock("https://raw.githubusercontent.com/")
769-
.get(`/pytorch/pytorch/main/test/${test.file}`)
770-
.reply(200, Buffer.from(`import blah;\nrest of file`));
771-
721+
let scope = mockGetRawTestFile(test.file, `import blah;\nrest of file`);
772722
let { labels, additionalErrMessage } =
773723
await disableFlakyTestBot.getTestOwnerLabels(test);
774724
expect(additionalErrMessage).toEqual(undefined);
@@ -781,9 +731,7 @@ describe("Disable Flaky Test Bot Unit Tests", () => {
781731
const test = { ...flakyTestA };
782732
test.jobNames = ["rocm", "linux"];
783733

784-
let scope = nock("https://raw.githubusercontent.com/")
785-
.get(`/pytorch/pytorch/main/test/${test.file}`)
786-
.reply(200, Buffer.from(`import blah;\nrest of file`));
734+
let scope = mockGetRawTestFile(test.file, `import blah;\nrest of file`);
787735

788736
let { labels, additionalErrMessage } =
789737
await disableFlakyTestBot.getTestOwnerLabels(test);

torchci/test/utils.ts

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,25 @@ export function mockGetPR(repoFullName: string, prNumber: number, body: any) {
8989
.reply(200, body);
9090
}
9191

92+
export function mockCreateIssue(
93+
repoFullName: string,
94+
title: string,
95+
bodyContains: string[],
96+
labels: string[]
97+
) {
98+
return nock("https://api.github.com")
99+
.post(`/repos/${repoFullName}/issues`, (body) => {
100+
expect(body.title).toEqual(title);
101+
expect(body.labels.sort()).toEqual(labels.sort());
102+
const bodyString = JSON.stringify(body.body);
103+
for (const containedString of bodyContains) {
104+
expect(bodyString).toContain(containedString);
105+
}
106+
return true;
107+
})
108+
.reply(200, {});
109+
}
110+
92111
export function mockPostComment(
93112
repoFullName: string,
94113
prNumber: number,
@@ -131,18 +150,7 @@ export function mockHasApprovedWorkflowRun(repoFullName: string) {
131150
});
132151
}
133152

134-
export function genIssueData(
135-
nonDefaultInputs: {
136-
number?: number;
137-
title?: string;
138-
html_url?: string;
139-
state?: "open" | "closed";
140-
body?: string;
141-
updated_at?: string;
142-
author_association?: string;
143-
labels?: string[];
144-
} = {}
145-
): IssueData {
153+
export function genIssueData(nonDefaultInputs: Partial<IssueData>): IssueData {
146154
return {
147155
number: 1,
148156
title: "test issue",
@@ -156,7 +164,9 @@ export function genIssueData(
156164
};
157165
}
158166

159-
export function getDummyJob(nonDefaultInputs: any = {}): RecentWorkflowsData {
167+
export function getDummyJob(
168+
nonDefaultInputs: Partial<RecentWorkflowsData>
169+
): RecentWorkflowsData {
160170
// Use this function to create a dummy job with default values
161171
if (
162172
(nonDefaultInputs.conclusion == "failure" ||

0 commit comments

Comments
 (0)