Skip to content
This repository was archived by the owner on Jan 22, 2022. It is now read-only.

Commit cbd388b

Browse files
janeyx99janeyx99
committed
clean everything up, fix issues, add tests
1 parent 59ceca3 commit cbd388b

File tree

3 files changed

+274
-102
lines changed

3 files changed

+274
-102
lines changed

‎src/verify-disable-test-issue.ts

Lines changed: 152 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,27 @@ import * as probot from 'probot';
22

33
const validationCommentStart = '<!-- validation-comment-start -->';
44
const validationCommentEnd = '<!-- validation-comment-end -->';
5-
const disabled_key = 'DISABLED'
6-
const supported_platforms = new Set(['mac', 'macos', 'win', 'windows', 'linux', 'rocm'])
7-
8-
async function getValidationComment(context: any, issue_number: string, owner: string, repo: string) {
5+
const disabledKey = 'DISABLED ';
6+
const supportedPlatforms = new Set([
7+
'asan',
8+
'linux',
9+
'mac',
10+
'macos',
11+
'rocm',
12+
'win',
13+
'windows'
14+
]);
15+
16+
async function getValidationComment(
17+
context,
18+
issueNumber: string,
19+
owner: string,
20+
repo: string
21+
): Promise<[number, string]> {
922
const commentsRes = await context.github.issues.listComments({
10-
owner: owner,
11-
repo: repo,
12-
issue_number: issue_number,
23+
owner,
24+
repo,
25+
issueNumber,
1326
per_page: 10
1427
});
1528
for (const comment of commentsRes.data) {
@@ -20,121 +33,159 @@ async function getValidationComment(context: any, issue_number: string, owner: s
2033
return [0, ''];
2134
}
2235

23-
function parseTitle(title: string) {
24-
const test_name = title.slice(disabled_key.length)
25-
const split = test_name.split(/\s+/)
26-
const test_case = split[0]
27-
let test_suite = ''
28-
if (split.length > 1 && split[1].startsWith('(__main__.') && split[1].endsWith(')')) {
29-
test_suite = split[1].slice('(__main__.'.length, split[1].length - 1)
30-
}
31-
return [test_case, test_suite]
32-
}
33-
34-
function parseBody(body: string) {
35-
const lines = body.split(/[\r\n]+/)
36-
let platforms_to_skip = new Set();
37-
let invalid_platforms = new Set();
38-
const key = 'platforms:'
36+
export function parseBody(body: string): [Set<string>, Set<string>] {
37+
const lines = body.split(/[\r\n]+/);
38+
const platformsToSkip = new Set<string>();
39+
const invalidPlatforms = new Set<string>();
40+
const key = 'platforms:';
3941
for (let line of lines) {
40-
line = line.toLowerCase()
42+
line = line.toLowerCase();
4143
if (line.startsWith(key)) {
42-
for (let platform of line.slice(key.length).split(/^\s+|\s*,\s*|\s+$/)) {
43-
if (supported_platforms.has(platform)) {
44-
platforms_to_skip.add(platform)
45-
} else {
46-
invalid_platforms.add(platform)
44+
for (const platform of line
45+
.slice(key.length)
46+
.split(/^\s+|\s*,\s*|\s+$/)) {
47+
if (supportedPlatforms.has(platform)) {
48+
platformsToSkip.add(platform);
49+
} else if (platform !== '') {
50+
invalidPlatforms.add(platform);
4751
}
4852
}
4953
}
5054
}
51-
return [platforms_to_skip, invalid_platforms]
55+
return [platformsToSkip, invalidPlatforms];
5256
}
5357

54-
function formValidationComment(test_info, platforms) {
55-
const test_case_name = test_info[0]
56-
const test_suite_name = test_info[1]
57-
const platforms_to_skip = Array.from(platforms[0]).sort()
58-
const platform_msg =
59-
platforms_to_skip.length === 0 ? 'none parsed, defaulting to ALL platforms' : platforms_to_skip.join(', ')
60-
const invalid_platforms = Array.from(platforms[1]).sort()
61-
62-
let body = '<details>Hello there! From the DISABLED prefix in this issue title, '
63-
body += 'it looks like you are attempting to disable a test in PyTorch CI. '
64-
body += 'The information I have parsed is below:\n'
65-
body += '<b>Test case name<b>: ' + test_case_name + '\n'
66-
body += '<b>Test suite name<b>: ' + test_suite_name + '\n'
67-
body += '<b>Platforms for which to skip the test<b>: ' + platform_msg + '\n\n'
68-
69-
if (!test_case_name || !test_suite_name) {
70-
body += '<b>ERROR!<b> As you can see above, I could not properly parse the test '
71-
body += 'information and determine which test to disable. Please modify the '
72-
body += 'title to be of the format: DISABLED test_case_name (__main__.TestSuiteName).\n'
58+
export function parseTitle(title: string): string {
59+
return title.slice(disabledKey.length).trim();
60+
}
61+
62+
function testNameIsExpected(testName: string): boolean {
63+
const split = testName.split(' ');
64+
if (split.length !== 2) {
65+
return false;
7366
}
7467

75-
if (invalid_platforms.length > 0) {
76-
body += '<b>WARNING!<b> In the parsing process, I received these invalid inputs as platforms for '
77-
body += 'which the test will be disabled: ' + invalid_platforms.join(', ') + '. These could '
78-
body += 'be typos or platforms we do not yet support test disabling. Please '
79-
body += 'verify the platform list above and modify your issue body if needed.\n'
68+
const testSuite = split[1].split('.');
69+
if (testSuite.length !== 2) {
70+
return false;
8071
}
72+
return true;
73+
}
8174

82-
if (test_case_name && test_suite_name) {
83-
body += 'Within ~15 minutes, test case ' + test_case_name + ' from test suite'
84-
body += test_suite_name + 'will be disabled in PyTorch CI for '
85-
body += (platforms_to_skip.length === 0 ? 'all platforms' : 'these platforms: ' + platforms_to_skip.join(', '))
86-
body += '.\n'
75+
export function formValidationComment(
76+
testName: string,
77+
platforms: [Set<string>, Set<string>]
78+
): string {
79+
const platformsToSkip = Array.from(platforms[0]).sort((a, b) =>
80+
a.localeCompare(b)
81+
);
82+
const platformMsg =
83+
platformsToSkip.length === 0
84+
? 'none parsed, defaulting to ALL platforms'
85+
: platformsToSkip.join(', ');
86+
const invalidPlatforms = Array.from(platforms[1]).sort((a, b) =>
87+
a.localeCompare(b)
88+
);
89+
90+
let body =
91+
'<details>Hello there! From the DISABLED prefix in this issue title, ';
92+
body += 'it looks like you are attempting to disable a test in PyTorch CI. ';
93+
body += 'The information I have parsed is below:\n';
94+
body += `<b>Test name<b>: ${testName}\n`;
95+
body += `<b>Platforms for which to skip the test<b>: ${platformMsg}\n\n`;
96+
97+
if (invalidPlatforms.length > 0) {
98+
body +=
99+
'<b>WARNING!<b> In the parsing process, I received these invalid inputs as platforms for ';
100+
body += `which the test will be disabled: ${invalidPlatforms.join(
101+
', '
102+
)}. These could `;
103+
body +=
104+
'be typos or platforms we do not yet support test disabling. Please ';
105+
body +=
106+
'verify the platform list above and modify your issue body if needed.\n';
87107
}
88108

89-
body += 'To modify the platforms list, please include a line in the issue body:\n\n'
90-
body += 'Platforms: case-insensitive, list, of, platforms\n\nWe currently support the following platforms: '
91-
body += Array.from(supported_platforms).sort().join(', ') + '.</details>'
109+
if (!testNameIsExpected(testName)) {
110+
body +=
111+
'<b>ERROR!<b> As you can see above, I could not properly parse the test ';
112+
body +=
113+
'information and determine which test to disable. Please modify the ';
114+
body +=
115+
'title to be of the format: DISABLED test_case_name (test.ClassName), ';
116+
body += 'for example, test_cuda_assert_async (__main__.TestCuda).\n';
117+
} else {
118+
body += `Within ~15 minutes, ${testName} will be disabled in PyTorch CI for `;
119+
body +=
120+
platformsToSkip.length === 0
121+
? 'all platforms'
122+
: `these platforms: ${platformsToSkip.join(', ')}`;
123+
body +=
124+
'. Please verify that your test name looks correct, e.g., test_cuda_assert_async (__main__.TestCuda).\n';
125+
}
92126

93-
return validationCommentStart + body + validationCommentEnd
127+
body +=
128+
'To modify the platforms list, please include a line in the issue body, like below. The default ';
129+
body +=
130+
'action will disable the test for all platforms if no platforms list is specified. \n\n';
131+
body +=
132+
'Platforms: case-insensitive, list, of, platforms\n\nWe currently support the following platforms: ';
133+
body += `${Array.from(supportedPlatforms)
134+
.sort((a, b) => a.localeCompare(b))
135+
.join(', ')}.</details>`;
136+
137+
return validationCommentStart + body + validationCommentEnd;
94138
}
95139

96140
function myBot(app: probot.Application): void {
97-
app.on(['issues.opened', 'issues.edited'], async context => {
98-
const state = context.payload['issue']['state']
99-
const title = context.payload['issue']['title']
100-
const owner = context.github.owner
101-
const repo = context.github.repo
102-
103-
if (state === 'closed' || !title.startsWith(disabled_key)) {
104-
return
105-
}
106-
107-
const body = context.payload['issue']['body']
108-
const number = context.payload['issue']['number']
109-
const existingValidationCommentData = await getValidationComment(context, number, owner, repo)
110-
const existingValidationCommentID = existingValidationCommentData[0]
111-
const existingValidationComment = existingValidationCommentData[1]
112-
113-
const test_info = parseTitle(title)
114-
const platforms = parseBody(body)
115-
const validationComment = formValidationComment(test_info, platforms)
141+
app.on(
142+
['issues.opened', 'issues.edited'],
143+
async (context: probot.Context) => {
144+
const state = context.payload['issue']['state'];
145+
const title = context.payload['issue']['title'];
146+
const owner = context.payload['repository']['owner']['login'];
147+
const repo = context.payload['repository']['name'];
148+
149+
if (state === 'closed' || !title.startsWith(disabledKey)) {
150+
return;
151+
}
116152

117-
if (existingValidationComment === validationComment) {
118-
return
119-
}
153+
const body = context.payload['issue']['body'];
154+
const number = context.payload['issue']['number'];
155+
const existingValidationCommentData = await getValidationComment(
156+
context,
157+
number,
158+
owner,
159+
repo
160+
);
161+
const existingValidationCommentID = existingValidationCommentData[0];
162+
const existingValidationComment = existingValidationCommentData[1];
163+
164+
const testName = parseTitle(title);
165+
const platforms = parseBody(body);
166+
const validationComment = formValidationComment(testName, platforms);
167+
168+
if (existingValidationComment === validationComment) {
169+
return;
170+
}
120171

121-
if (existingValidationCommentID === 0) {
122-
const res = await this.ctx.github.issues.createComment({
123-
validationComment,
124-
owner: owner,
125-
repo: repo,
126-
issue_number: number
127-
});
128-
const newCommentID = res.data.id;
129-
} else {
130-
await this.ctx.github.issues.updateComment({
131-
validationComment,
132-
owner: owner,
133-
repo: repo,
134-
comment_id: existingValidationCommentID
135-
});
172+
if (existingValidationCommentID === 0) {
173+
await context.github.issues.createComment({
174+
body: validationComment,
175+
owner,
176+
repo,
177+
issue_number: number
178+
});
179+
} else {
180+
await context.github.issues.updateComment({
181+
body: validationComment,
182+
owner,
183+
repo,
184+
comment_id: existingValidationCommentID
185+
});
186+
}
136187
}
137-
});
188+
);
138189
}
139190

140191
export default myBot;

0 commit comments

Comments
 (0)