[CI] Check target branch freshness before starting CI (#56169)
Signed-off-by: Andreas Karatzas <akaratza@amd.com> Signed-off-by: Andreas Karatzas <Andreas.Karatzas@amd.com>
This commit is contained in:
@@ -22,8 +22,10 @@ from run_ci_command import (
|
||||
COMMAND_RUN_CI_ALL,
|
||||
COMMAND_RUN_CI_NIGHTLY,
|
||||
RETRY_STATES,
|
||||
RUN_CI_COMMAND_ENV,
|
||||
ApiError,
|
||||
BuildkiteClient,
|
||||
GitHubClient,
|
||||
HttpTransport,
|
||||
authorize,
|
||||
create_build_payload,
|
||||
@@ -85,6 +87,7 @@ class FakeGitHub:
|
||||
comments: list[str] | None = None,
|
||||
pulls_for_commit: list[dict[str, Any]] | None = None,
|
||||
prs: dict[int, dict[str, Any]] | None = None,
|
||||
behind: int = 0,
|
||||
) -> None:
|
||||
self.comments = comments or []
|
||||
self.permission = permission
|
||||
@@ -95,6 +98,12 @@ class FakeGitHub:
|
||||
self.reviews = reviews or []
|
||||
self.pulls_for_commit = pulls_for_commit or []
|
||||
self.prs = prs or {self.pr["number"]: self.pr}
|
||||
self.behind = behind
|
||||
self.lag_queries: list[tuple[str, str]] = []
|
||||
|
||||
def get_commits_behind_base(self, base_ref: str, head_sha: str) -> int:
|
||||
self.lag_queries.append((base_ref, head_sha))
|
||||
return self.behind
|
||||
|
||||
def get_pr(self, number: int) -> dict[str, Any]:
|
||||
try:
|
||||
@@ -322,6 +331,11 @@ class RunCiCommandTest(unittest.TestCase):
|
||||
for command in commands:
|
||||
with self.subTest(command=command):
|
||||
self.assertEqual(parse_command(command), command)
|
||||
override = f"{command} --allow-stale"
|
||||
if " run" in command:
|
||||
self.assertEqual(parse_command(override), override)
|
||||
else:
|
||||
self.assertIsNone(parse_command(override))
|
||||
|
||||
self.assertIsNone(parse_command("/ci run please"))
|
||||
self.assertIsNone(parse_command("/ci run all please"))
|
||||
@@ -331,6 +345,8 @@ class RunCiCommandTest(unittest.TestCase):
|
||||
self.assertIsNone(parse_command("/amd-ci retry "))
|
||||
self.assertIsNone(parse_command("/AMD-CI run"))
|
||||
self.assertIsNone(parse_command("/amdci run"))
|
||||
self.assertIsNone(parse_command("/ci run --allow-stale all"))
|
||||
self.assertIsNone(parse_command("/amd-ci run --allow-stale please"))
|
||||
|
||||
def test_commands_select_only_their_configured_pipeline(self) -> None:
|
||||
cases = (
|
||||
@@ -530,6 +546,177 @@ class RunCiCommandTest(unittest.TestCase):
|
||||
self.assertTrue(github.comments[0].startswith("✅ "))
|
||||
self.assertIn("Buildkite CI #123", github.comments[0])
|
||||
|
||||
def test_run_variants_require_current_base_without_changing_commit(self) -> None:
|
||||
for command in RUN_CI_COMMAND_ENV:
|
||||
if command.endswith(" --allow-stale"):
|
||||
continue
|
||||
for behind in (0, 1, 5):
|
||||
with self.subTest(command=command, behind=behind):
|
||||
github = FakeGitHub(
|
||||
behind=behind, pr=make_pr(base={"ref": "release/1.0"})
|
||||
)
|
||||
buildkite = FakeBuildkite([[], []])
|
||||
run(make_event(command), github, buildkite)
|
||||
|
||||
self.assertEqual(
|
||||
github.lag_queries,
|
||||
[("release/1.0", github.pr["head"]["sha"])],
|
||||
)
|
||||
if behind == 0:
|
||||
self.assertEqual(
|
||||
buildkite.created_builds[0]["commit"],
|
||||
github.pr["head"]["sha"],
|
||||
)
|
||||
self.assertIn(
|
||||
"](https://buildkite.example/builds/123)",
|
||||
github.comments[0],
|
||||
)
|
||||
else:
|
||||
self.assertEqual(buildkite.created_builds, [])
|
||||
self.assertTrue(github.comments[0].startswith("❌ "))
|
||||
self.assertIn(str(behind), github.comments[0])
|
||||
self.assertIn("upstream `release/1.0`", github.comments[0])
|
||||
self.assertIn("every commit", github.comments[0])
|
||||
self.assertIn(
|
||||
"No new CI build was started.", github.comments[0]
|
||||
)
|
||||
self.assertIn(command, github.comments[0])
|
||||
|
||||
def test_allow_stale_preserves_author_permissions_and_run_configuration(
|
||||
self,
|
||||
) -> None:
|
||||
cases = (
|
||||
(COMMAND_RUN_CI, "ci", {}),
|
||||
(COMMAND_RUN_CI_ALL, "ci", {"RUN_ALL": "1"}),
|
||||
(COMMAND_RUN_CI_NIGHTLY, "ci", {"RUN_ALL": "1", "NIGHTLY": "1"}),
|
||||
(COMMAND_RUN_AMD_CI, "amd-ci", {}),
|
||||
(COMMAND_RUN_AMD_CI_ALL, "amd-ci", {"RUN_ALL": "1"}),
|
||||
(
|
||||
COMMAND_RUN_AMD_CI_NIGHTLY,
|
||||
"amd-ci",
|
||||
{"RUN_ALL": "1", "NIGHTLY": "1"},
|
||||
),
|
||||
)
|
||||
for original_command, pipeline, expected_env in cases:
|
||||
command = f"{original_command} --allow-stale"
|
||||
for ready, behind in ((False, 100), (True, 0), (True, 100)):
|
||||
with self.subTest(command=command, ready=ready, behind=behind):
|
||||
github = FakeGitHub(
|
||||
permission="read",
|
||||
behind=behind,
|
||||
pr=make_pr(
|
||||
base={"ref": "feature/stack"},
|
||||
labels=[{"name": "ready"}] if ready else [],
|
||||
),
|
||||
)
|
||||
buildkite = FakeBuildkite([[], []])
|
||||
|
||||
run(make_event(command, "author"), github, buildkite)
|
||||
|
||||
self.assertEqual(pipeline_for_command(command), pipeline)
|
||||
if not ready:
|
||||
self.assertEqual(buildkite.created_builds, [])
|
||||
self.assertEqual(buildkite.list_calls, [])
|
||||
self.assertEqual(github.lag_queries, [])
|
||||
self.assertTrue(github.comments[0].startswith("❌ "))
|
||||
continue
|
||||
self.assertEqual(len(buildkite.created_builds), 1)
|
||||
payload = buildkite.created_builds[0]
|
||||
self.assertEqual(payload["commit"], github.pr["head"]["sha"])
|
||||
self.assertEqual(
|
||||
payload["pull_request_base_branch"], "feature/stack"
|
||||
)
|
||||
self.assertEqual(payload["message"], f"PR #42 {command} by @author")
|
||||
self.assertEqual(
|
||||
payload["env"],
|
||||
{
|
||||
"VLLM_CI_GITHUB_COMMENT_ID": "99",
|
||||
"VLLM_CI_TRIGGERED_BY": "author",
|
||||
**expected_env,
|
||||
},
|
||||
)
|
||||
self.assertEqual(
|
||||
github.lag_queries,
|
||||
[("feature/stack", github.pr["head"]["sha"])],
|
||||
)
|
||||
self.assertIn(
|
||||
"Buildkite AMD CI #123"
|
||||
if pipeline == "amd-ci"
|
||||
else "Buildkite CI #123",
|
||||
github.comments[0],
|
||||
)
|
||||
if behind:
|
||||
self.assertIn("100 commits behind", github.comments[0])
|
||||
self.assertIn("upstream `feature/stack`", github.comments[0])
|
||||
self.assertIn("Before merging", github.comments[0])
|
||||
self.assertIn("latest PR commit", github.comments[0])
|
||||
self.assertIn(original_command, github.comments[0])
|
||||
|
||||
def test_base_lag_query_qualifies_branch_refs_and_validates_count(self) -> None:
|
||||
for base_ref, encoded_ref in (
|
||||
("main", "main"),
|
||||
("release/1.0", "release%2F1.0"),
|
||||
("feature/stack", "feature%2Fstack"),
|
||||
):
|
||||
for count in (0, 5, None, -1, True, "5"):
|
||||
with self.subTest(base_ref=base_ref, count=count):
|
||||
transport = FakeTransport(
|
||||
{} if count is None else {"behind_by": count}
|
||||
)
|
||||
github = GitHubClient("token", "vllm-project/vllm", transport)
|
||||
if type(count) is int and count >= 0:
|
||||
self.assertEqual(
|
||||
github.get_commits_behind_base(base_ref, "head-sha"), count
|
||||
)
|
||||
else:
|
||||
with self.assertRaises(ApiError):
|
||||
github.get_commits_behind_base(base_ref, "head-sha")
|
||||
self.assertEqual(
|
||||
transport.calls[0]["url"],
|
||||
"https://api.github.com/repos/vllm-project/vllm/compare/"
|
||||
f"refs%2Fheads%2F{encoded_ref}...head-sha?per_page=1",
|
||||
)
|
||||
self.assertEqual(transport.calls[0]["method"], "GET")
|
||||
|
||||
def test_unknown_lag_or_changed_pr_prevents_runs(self) -> None:
|
||||
for command in (
|
||||
COMMAND_RUN_CI,
|
||||
COMMAND_RUN_AMD_CI,
|
||||
f"{COMMAND_RUN_CI} --allow-stale",
|
||||
f"{COMMAND_RUN_AMD_CI} --allow-stale",
|
||||
):
|
||||
for fault in ("api", "head", "closed", "base"):
|
||||
with self.subTest(command=command, fault=fault):
|
||||
github = FakeGitHub()
|
||||
buildkite = FakeBuildkite([[], []])
|
||||
|
||||
def check(
|
||||
base_ref: str,
|
||||
sha: str,
|
||||
fault: str = fault,
|
||||
github: FakeGitHub = github,
|
||||
) -> int:
|
||||
if fault == "api":
|
||||
raise ApiError(503, "Unavailable")
|
||||
changed = make_pr()
|
||||
if fault == "head":
|
||||
changed["head"]["sha"] = "another-head"
|
||||
elif fault == "closed":
|
||||
changed["state"] = "closed"
|
||||
else:
|
||||
changed["base"]["ref"] = "another-base"
|
||||
github.prs[42] = changed
|
||||
return 0
|
||||
|
||||
with patch.object(
|
||||
github, "get_commits_behind_base", side_effect=check
|
||||
):
|
||||
run(make_event(command), github, buildkite)
|
||||
self.assertEqual(buildkite.created_builds, [])
|
||||
self.assertEqual(buildkite.retry_calls, [])
|
||||
self.assertTrue(github.comments[0].startswith("❌ "))
|
||||
self.assertNotIn("Triggered", github.comments[0])
|
||||
|
||||
def test_amd_run_ignores_blocked_builds(self) -> None:
|
||||
for metadata in ({}, {"github-comment-id": "98"}):
|
||||
with self.subTest(metadata=metadata):
|
||||
@@ -627,6 +814,7 @@ class RunCiCommandTest(unittest.TestCase):
|
||||
run(make_event(COMMAND_RUN_CI, "author"), github, buildkite)
|
||||
|
||||
self.assertEqual(buildkite.list_calls, [])
|
||||
self.assertEqual(github.lag_queries, [])
|
||||
self.assertEqual(github.reactions, ["eyes"])
|
||||
self.assertTrue(github.comments[0].startswith("❌ "))
|
||||
self.assertIn("approve the PR", github.comments[0])
|
||||
@@ -848,6 +1036,7 @@ class RunCiCommandTest(unittest.TestCase):
|
||||
|
||||
def test_ci_retry_retries_failed_jobs_while_build_is_running(self) -> None:
|
||||
github = FakeGitHub(
|
||||
behind=100,
|
||||
permission="read",
|
||||
pr=make_pr(labels=[{"name": "ready"}]),
|
||||
)
|
||||
@@ -867,10 +1056,12 @@ class RunCiCommandTest(unittest.TestCase):
|
||||
run(make_event(COMMAND_RETRY_FAILED, "author"), github, buildkite)
|
||||
|
||||
self.assertEqual(buildkite.retry_calls, [(123, RETRY_STATES)])
|
||||
self.assertEqual(github.lag_queries, [])
|
||||
self.assertIn("Queued 3 failed job", github.comments[0])
|
||||
|
||||
def test_amd_ci_retry_retries_only_the_current_head_build(self) -> None:
|
||||
github = FakeGitHub(
|
||||
behind=100,
|
||||
permission="read",
|
||||
pr=make_pr(labels=[{"name": "ready"}]),
|
||||
)
|
||||
@@ -891,6 +1082,7 @@ class RunCiCommandTest(unittest.TestCase):
|
||||
run(make_event(COMMAND_RETRY_AMD_FAILED, "author"), github, buildkite)
|
||||
|
||||
self.assertEqual(buildkite.retry_calls, [(321, RETRY_STATES)])
|
||||
self.assertEqual(github.lag_queries, [])
|
||||
self.assertIn("Buildkite AMD CI #321", github.comments[0])
|
||||
|
||||
def test_amd_ci_retry_requires_a_build_for_the_current_head(self) -> None:
|
||||
@@ -912,6 +1104,7 @@ class RunCiCommandTest(unittest.TestCase):
|
||||
|
||||
def test_ci_retry_creates_filtered_build_for_new_head(self) -> None:
|
||||
github = FakeGitHub(
|
||||
behind=100,
|
||||
permission="read",
|
||||
pr=make_pr(labels=[{"name": "ready"}]),
|
||||
)
|
||||
@@ -953,6 +1146,7 @@ class RunCiCommandTest(unittest.TestCase):
|
||||
self.assertEqual(len(buildkite.created_builds), 1)
|
||||
payload = buildkite.created_builds[0]
|
||||
self.assertEqual(payload["commit"], "0123456789abcdef")
|
||||
self.assertEqual(github.lag_queries, [])
|
||||
self.assertEqual(payload["message"], "PR #42 /ci retry by @author")
|
||||
self.assertEqual(
|
||||
json.loads(payload["env"]["VLLM_CI_ONLY_STEP_KEYS"]),
|
||||
@@ -1104,6 +1298,7 @@ class RunCiCommandTest(unittest.TestCase):
|
||||
run(make_event(COMMAND_CANCEL_CI), github, buildkite)
|
||||
|
||||
self.assertEqual(buildkite.cancel_calls, [123, 124, 126])
|
||||
self.assertEqual(github.lag_queries, [])
|
||||
self.assertEqual(
|
||||
[request["branch"] for request in buildkite.list_requests],
|
||||
["contributor:feature", "feature"],
|
||||
@@ -1152,6 +1347,7 @@ class RunCiCommandTest(unittest.TestCase):
|
||||
run(make_event(COMMAND_CANCEL_AMD_CI), github, buildkite)
|
||||
|
||||
self.assertEqual(buildkite.cancel_calls, [322, 321])
|
||||
self.assertEqual(github.lag_queries, [])
|
||||
self.assertEqual(
|
||||
[request["branch"] for request in buildkite.list_requests],
|
||||
["contributor:feature", "feature"],
|
||||
|
||||
Reference in New Issue
Block a user