Skip to content

Commit 73fbf31

Browse files
authored
[disablebot] Allow reading of disable issue for multiple tests (#6045)
The expected format of a disable issue for multiple tests is Title: DISABLED MULTIPLE <can be anything here> Body: additional info disable the following tests: ``` test_name (test_suite): platforms, mac test_name2 (test_suite): ``` additional info For example <img width="841" alt="image" src="https://github.com/user-attachments/assets/63200bc7-5321-4f80-a468-ef1bfdae3a53" /> --- Tested by running old version and new version and diffing the files. There was no change
1 parent 487906c commit 73fbf31

File tree

2 files changed

+180
-39
lines changed

2 files changed

+180
-39
lines changed

Diff for: .github/scripts/test_update_disabled_issues.py

+83-3
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@
22

33
from update_disabled_issues import (
44
condense_disable_jobs,
5-
condense_disable_tests,
65
filter_disable_issues,
76
get_disable_issues,
7+
get_disabled_tests,
88
OWNER,
99
REPO,
1010
UNSTABLE_PREFIX,
@@ -98,13 +98,13 @@ def test_filter_disable_issues(self, mock_get_disable_issues):
9898
[item["number"] for item in disabled_jobs], [32132, 42345, 94861]
9999
)
100100

101-
def test_condense_disable_tests(self, mock_get_disable_issues):
101+
def test_get_disable_tests(self, mock_get_disable_issues):
102102
mock_get_disable_issues.return_value = MOCK_DATA
103103

104104
disabled_issues = get_disable_issues("dummy token")
105105

106106
disabled_tests, _ = filter_disable_issues(disabled_issues)
107-
results = condense_disable_tests(disabled_tests)
107+
results = get_disabled_tests(disabled_tests)
108108

109109
self.assertDictEqual(
110110
{
@@ -125,6 +125,86 @@ def test_condense_disable_tests(self, mock_get_disable_issues):
125125
results,
126126
)
127127

128+
def test_get_disable_tests_aggregate_issue(self, mock_get_disable_issues):
129+
# Test that the function can read aggregate issues
130+
self.maxDiff = None
131+
mock_data = [
132+
{
133+
"url": "https://github.com/pytorch/pytorch/issues/32644",
134+
"number": 32644,
135+
"title": "DISABLED MULTIPLE dummy test",
136+
"body": "disable the following tests:\n```\ntest_quantized_nn (test_quantization.PostTrainingDynamicQuantTest): mac, win\ntest_zero_redundancy_optimizer (__main__.TestZeroRedundancyOptimizerDistributed)\n```",
137+
}
138+
]
139+
disabled_tests = get_disabled_tests(mock_data)
140+
self.assertDictEqual(
141+
{
142+
"test_quantized_nn (test_quantization.PostTrainingDynamicQuantTest)": (
143+
str(mock_data[0]["number"]),
144+
mock_data[0]["url"],
145+
["mac", "win"],
146+
),
147+
"test_zero_redundancy_optimizer (__main__.TestZeroRedundancyOptimizerDistributed)": (
148+
str(mock_data[0]["number"]),
149+
mock_data[0]["url"],
150+
[],
151+
),
152+
},
153+
disabled_tests,
154+
)
155+
156+
def test_get_disable_tests_merge_issues(self, mock_get_disable_issues):
157+
# Test that the function can merge multiple issues with the same test
158+
# name
159+
self.maxDiff = None
160+
mock_data = [
161+
{
162+
"url": "https://github.com/pytorch/pytorch/issues/32644",
163+
"number": 32644,
164+
"title": "DISABLED MULTIPLE dummy test",
165+
"body": "disable the following tests:\n```\ntest_2 (abc.ABC): mac, win\ntest_3 (DEF)\n```",
166+
},
167+
{
168+
"url": "https://github.com/pytorch/pytorch/issues/32645",
169+
"number": 32645,
170+
"title": "DISABLED MULTIPLE dummy test",
171+
"body": "disable the following tests:\n```\ntest_2 (abc.ABC): mac, win, linux\ntest_3 (DEF): mac\n```",
172+
},
173+
{
174+
"url": "https://github.com/pytorch/pytorch/issues/32646",
175+
"number": 32646,
176+
"title": "DISABLED test_1 (__main__.Test1)",
177+
"body": "platforms: linux",
178+
},
179+
{
180+
"url": "https://github.com/pytorch/pytorch/issues/32647",
181+
"number": 32647,
182+
"title": "DISABLED test_2 (abc.ABC)",
183+
"body": "platforms: dynamo",
184+
},
185+
]
186+
disabled_tests = get_disabled_tests(mock_data)
187+
self.assertDictEqual(
188+
{
189+
"test_2 (abc.ABC)": (
190+
str(mock_data[3]["number"]),
191+
mock_data[3]["url"],
192+
["dynamo", "linux", "mac", "win"],
193+
),
194+
"test_3 (DEF)": (
195+
str(mock_data[1]["number"]),
196+
mock_data[1]["url"],
197+
[],
198+
),
199+
"test_1 (__main__.Test1)": (
200+
str(mock_data[2]["number"]),
201+
mock_data[2]["url"],
202+
["linux"],
203+
),
204+
},
205+
disabled_tests,
206+
)
207+
128208
def test_condense_disable_jobs(self, mock_get_disable_issues):
129209
mock_get_disable_issues.return_value = MOCK_DATA
130210

Diff for: .github/scripts/update_disabled_issues.py

+97-36
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
DISABLED_PREFIX = "DISABLED"
2020
UNSTABLE_PREFIX = "UNSTABLE"
2121
DISABLED_TEST_ISSUE_TITLE = re.compile(r"DISABLED\s*test_.+\s*\(.+\)")
22+
DISABLED_TEST_MULTI_ISSUE_TITLE = re.compile(r"DISABLED MULTIPLE")
2223
JOB_NAME_MAXSPLIT = 2
2324

2425
OWNER = "pytorch"
@@ -122,14 +123,108 @@ def filter_disable_issues(
122123
if not title or not title.startswith(prefix):
123124
continue
124125

125-
if DISABLED_TEST_ISSUE_TITLE.match(title):
126+
if DISABLED_TEST_ISSUE_TITLE.match(
127+
title
128+
) or DISABLED_TEST_MULTI_ISSUE_TITLE.match(title):
126129
disable_test_issues.append(issue)
127130
else:
128131
disable_job_issues.append(issue)
129132

130133
return disable_test_issues, disable_job_issues
131134

132135

136+
def get_disabled_tests(issues: List[Dict[str, Any]]) -> Dict[str, Tuple]:
137+
def get_platforms_to_skip(body: str, prefix: str) -> List[str]:
138+
# Empty list = all platforms should skip the test
139+
platforms_to_skip = []
140+
if body is not None:
141+
for line in body.splitlines():
142+
line = line.lower()
143+
if line.startswith(prefix):
144+
platforms_to_skip.extend(
145+
[x.strip() for x in line[len(prefix) :].split(",") if x.strip()]
146+
)
147+
return platforms_to_skip
148+
149+
disabled_tests = {}
150+
151+
def update_disabled_tests(
152+
key: str, number: str, url: str, platforms_to_skip: List[str]
153+
):
154+
# merge the list of platforms to skip if the test is disabled by
155+
# multiple issues. This results in some urls being wrong
156+
if key not in disabled_tests:
157+
disabled_tests[key] = (number, url, platforms_to_skip)
158+
else:
159+
original_platforms = disabled_tests[key][2]
160+
if len(original_platforms) == 0 or len(platforms_to_skip) == 0:
161+
platforms = []
162+
else:
163+
platforms = sorted(set(original_platforms + platforms_to_skip))
164+
disabled_tests[key] = (
165+
number,
166+
url,
167+
platforms,
168+
)
169+
170+
test_name_regex = re.compile(r"(test_[a-zA-Z0-9-_\.]+)\s+\(([a-zA-Z0-9-_\.]+)\)")
171+
172+
def parse_test_name(s: str) -> Optional[str]:
173+
test_name_match = test_name_regex.match(s)
174+
if test_name_match:
175+
return f"{test_name_match.group(1)} ({test_name_match.group(2)})"
176+
return None
177+
178+
for issue in issues:
179+
try:
180+
url = issue["url"]
181+
number = url.split("/")[-1]
182+
title = issue["title"].strip()
183+
body = issue["body"]
184+
185+
test_name = parse_test_name(title[len("DISABLED") :].strip())
186+
if test_name is not None:
187+
update_disabled_tests(
188+
test_name, number, url, get_platforms_to_skip(body, "platforms:")
189+
)
190+
elif DISABLED_TEST_MULTI_ISSUE_TITLE.match(title):
191+
# This is a multi-test issue
192+
start = body.lower().find("disable the following tests:")
193+
# Format for disabling tests:
194+
# Title: DISABLED MULTIPLE anything
195+
# disable the following tests:
196+
# ```
197+
# test_name1 (test_suite1): mac, windows
198+
# test_name2 (test_suite2): mac, windows
199+
# ```
200+
for line in body[start:].splitlines()[2:]:
201+
if "```" in line:
202+
break
203+
split_by_colon = line.split(":")
204+
205+
test_name = parse_test_name(split_by_colon[0].strip())
206+
if test_name is None:
207+
continue
208+
update_disabled_tests(
209+
test_name,
210+
number,
211+
url,
212+
get_platforms_to_skip(
213+
split_by_colon[1].strip()
214+
if len(split_by_colon) > 1
215+
else "",
216+
"",
217+
),
218+
)
219+
else:
220+
print(f"Unknown disable issue type: {title}")
221+
except Exception as e:
222+
print(f"Failed to parse issue {issue['url']}: {e}")
223+
continue
224+
225+
return disabled_tests
226+
227+
133228
@lru_cache()
134229
def can_disable_jobs(owner: str, repo: str, username: str, token: str) -> bool:
135230
url = f"https://api.github.com/repos/{owner}/{repo}/collaborators/{username}/permission"
@@ -146,38 +241,6 @@ def can_disable_jobs(owner: str, repo: str, username: str, token: str) -> bool:
146241
return perm and perm.get("permission", "").lower() in PERMISSIONS_TO_DISABLE_JOBS
147242

148243

149-
def condense_disable_tests(
150-
disable_issues: List[Dict[str, Any]],
151-
) -> Dict[str, Tuple]:
152-
disabled_test_from_issues = {}
153-
for item in disable_issues:
154-
issue_url = item["url"]
155-
issue_number = issue_url.split("/")[-1]
156-
157-
title = item["title"]
158-
test_name = title[len(DISABLED_PREFIX) :].strip()
159-
160-
body = item["body"]
161-
platforms_to_skip = []
162-
key = "platforms:"
163-
# When the issue has no body, it is assumed that all platforms should skip the test
164-
if body is not None:
165-
for line in body.splitlines():
166-
line = line.lower()
167-
if line.startswith(key):
168-
platforms_to_skip.extend(
169-
[x.strip() for x in line[len(key) :].split(",") if x.strip()]
170-
)
171-
172-
disabled_test_from_issues[test_name] = (
173-
issue_number,
174-
issue_url,
175-
platforms_to_skip,
176-
)
177-
178-
return disabled_test_from_issues
179-
180-
181244
def condense_disable_jobs(
182245
disable_issues: List[Any],
183246
owner: str,
@@ -253,9 +316,7 @@ def main() -> None:
253316
disable_test_issues, disable_job_issues = filter_disable_issues(disable_issues)
254317
# Create the list of disabled tests taken into account the list of disabled issues
255318
# and those that are not flaky anymore
256-
dump_json(
257-
condense_disable_tests(disable_test_issues), "disabled-tests-condensed.json"
258-
)
319+
dump_json(get_disabled_tests(disable_test_issues), "disabled-tests-condensed.json")
259320
dump_json(
260321
condense_disable_jobs(disable_job_issues, args.owner, args.repo, token),
261322
"disabled-jobs.json",

0 commit comments

Comments
 (0)