Skip to content

Commit 3b1fe16

Browse files
authored
disable issues: throw error if issue retrieval goes wrong (#7141)
Reason: I've noticed that sometimes a lot of stuff in the disable json gets removed even though the issues weren't closed -> CI runs the tests which should be disabled -> fails -> HUD is red. Looking at the vercel logs, I can see `Expected 577 issues with prefix "DISABLED", but found 0.`, so it's probably that we are somehow failing to fetch the issues. My solution is to thrown an error in the API call, which then gets caught by the python script and raises an runtime error, so in the case of an error, the json won't get updated (and the old one will be left) Additional things that could be done: don't update json if dramatic changes happen (ex count goes from 100 -> 0) Testing: Change hud url to localhost on python script, run the python script
1 parent d16ba34 commit 3b1fe16

3 files changed

Lines changed: 51 additions & 15 deletions

File tree

.github/scripts/update_disabled_issues.py

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import time
1010
from datetime import datetime, timezone
1111
from typing import Any, Dict
12+
from urllib.error import HTTPError
1213
from urllib.request import Request, urlopen
1314

1415
from gen_historical_disabled_issue_data import format_info
@@ -23,16 +24,22 @@ def dump_json(data: Dict[str, Any], filename: str):
2324

2425

2526
def main() -> None:
26-
with urlopen(
27-
Request(
28-
f"{HUD_URL}/api/flaky-tests/getDisabledTestsAndJobs",
29-
headers={"Authorization": os.environ["FLAKY_TEST_BOT_KEY"]},
30-
)
31-
) as result:
32-
if result.status != 200:
33-
raise RuntimeError(f"Failed to fetch data: {result.status} {result.reason}")
34-
35-
json_data = json.loads(result.read().decode("utf-8"))
27+
try:
28+
with urlopen(
29+
Request(
30+
f"{HUD_URL}/api/flaky-tests/getDisabledTestsAndJobs",
31+
headers={"Authorization": os.environ["FLAKY_TEST_BOT_KEY"]},
32+
)
33+
) as result:
34+
if result.status != 200:
35+
# Not sure if this is necessary but just in case
36+
raise RuntimeError(
37+
f"Failed to fetch data: {result.status} {result.reason}"
38+
)
39+
json_data = json.loads(result.read().decode("utf-8"))
40+
except HTTPError as e:
41+
error_body = e.read().decode("utf-8")
42+
raise RuntimeError(f"HTTPError: {e.code} {e.reason} - {error_body}")
3643

3744
dump_json(json_data["disabledTests"], "disabled-tests-condensed.json")
3845
dump_json(json_data["disabledJobs"], "disabled-jobs.json")

torchci/pages/api/flaky-tests/getDisabledTestsAndJobs.ts

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,14 @@ export default async function handler(
5555
) {
5656
const authorization = req.headers.authorization;
5757
if (authorization === process.env.FLAKY_TEST_BOT_KEY) {
58-
const octokit = await getOctokit(PYTORCH, PYTORCH);
59-
res.status(200).json(await getDisabledTestsAndJobs(octokit));
58+
try {
59+
const octokit = await getOctokit(PYTORCH, PYTORCH);
60+
res.status(200).json(await getDisabledTestsAndJobs(octokit));
61+
} catch (err: any) {
62+
res
63+
.status(500)
64+
.json({ error: err instanceof Error ? err.message : String(err) });
65+
}
6066
} else {
6167
res.status(403).end();
6268
}
@@ -97,9 +103,9 @@ async function getIssues(octokit: Octokit, prefix: string) {
97103
} while (cursor);
98104

99105
if (issues.length !== totalCount) {
100-
console.warn(
101-
`Expected ${totalCount} issues with prefix "${prefix}", but found ${issues.length}.`
102-
);
106+
const errString = `Expected ${totalCount} issues with prefix "${prefix}", but found ${issues.length}.`;
107+
console.error(errString);
108+
throw new Error(errString);
103109
}
104110

105111
return issues.sort((a, b) => a.url.localeCompare(b.url));

torchci/test/flakyBotTests/getDisabledTestsAndJobs.test.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,29 @@ describe("Get disable/unstable job/test jsons", () => {
6464
handleScope(scope);
6565
});
6666

67+
test("Throw error if result from graphql is inconsistent", async () => {
68+
// Number of tests != node list => throw error
69+
const scope = nock("https://api.github.com")
70+
.post("/graphql", (body) => {
71+
return body.query.includes("search");
72+
})
73+
.reply(200, {
74+
data: {
75+
search: {
76+
issueCount: 15,
77+
pageInfo: { hasNextPage: false, endCursor: "" },
78+
nodes: [],
79+
},
80+
},
81+
});
82+
83+
await expect(
84+
getDisabledTestsAndJobs.getDisabledTestsAndJobs(octokit)
85+
).rejects.toThrow();
86+
87+
handleScope(scope);
88+
});
89+
6790
test("One test", async () => {
6891
const issue = genSingleIssueFor(flakyTestA, {});
6992
const scope = [

0 commit comments

Comments
 (0)