Skip to content

Commit 166ddb1

Browse files
authored
Fix(cicd_bot): When enable_deploy_command: true, dont show message about required approvers (#4782)
1 parent d083888 commit 166ddb1

File tree

4 files changed

+91
-12
lines changed

4 files changed

+91
-12
lines changed

sqlmesh/integrations/github/cicd/command.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,9 @@ def _run_all(controller: GithubController) -> None:
276276
if has_required_approval and prod_plan_generated and controller.pr_targets_prod_branch:
277277
deployed_to_prod = _deploy_production(controller)
278278
elif is_auto_deploying_prod:
279-
if not has_required_approval:
279+
if controller.deploy_command_enabled and not has_required_approval:
280+
skip_reason = "Skipped Deploying to Production because a `/deploy` command has not been detected yet"
281+
elif controller.do_required_approval_check and not has_required_approval:
280282
skip_reason = (
281283
"Skipped Deploying to Production because a required approver has not approved"
282284
)

sqlmesh/integrations/github/cicd/controller.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1022,7 +1022,7 @@ def conclusion_handler(
10221022
conclusion_to_title = {
10231023
GithubCheckConclusion.SUCCESS: "Deployed to Prod",
10241024
GithubCheckConclusion.CANCELLED: "Cancelled deploying to prod",
1025-
GithubCheckConclusion.SKIPPED: skip_reason,
1025+
GithubCheckConclusion.SKIPPED: "Skipped deployment",
10261026
GithubCheckConclusion.FAILURE: "Failed to deploy to prod",
10271027
GithubCheckConclusion.ACTION_REQUIRED: "Failed due to error applying plan",
10281028
}
@@ -1031,7 +1031,7 @@ def conclusion_handler(
10311031
or f"Got an unexpected conclusion: {conclusion.value}"
10321032
)
10331033
if conclusion.is_skipped:
1034-
summary = title
1034+
summary = skip_reason
10351035
elif conclusion.is_failure:
10361036
captured_errors = self._console.consume_captured_errors()
10371037
summary = (

tests/integrations/github/cicd/test_github_commands.py

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1295,3 +1295,74 @@ def test_comment_command_deploy_prod_not_enabled(
12951295
with open(github_output_file, "r", encoding="utf-8") as f:
12961296
output = f.read()
12971297
assert output == ""
1298+
1299+
1300+
def test_comment_command_deploy_prod_no_deploy_detected_yet(
1301+
github_client,
1302+
make_controller,
1303+
make_mock_check_run,
1304+
make_mock_issue_comment,
1305+
tmp_path: pathlib.Path,
1306+
mocker: MockerFixture,
1307+
):
1308+
"""
1309+
Scenario:
1310+
- PR is not merged
1311+
- No requred approvers defined
1312+
- Tests passed
1313+
- PR Merge Method defined
1314+
- Deploy command enabled but not yet triggered
1315+
1316+
Outcome:
1317+
- "Prod Environment Synced" step should explain the reason why it was skipped is because /deploy has not yet been detected
1318+
"""
1319+
mock_repo = github_client.get_repo()
1320+
mock_repo.create_check_run = mocker.MagicMock(
1321+
side_effect=lambda **kwargs: make_mock_check_run(**kwargs)
1322+
)
1323+
1324+
created_comments = []
1325+
mock_issue = mock_repo.get_issue()
1326+
mock_issue.create_comment = mocker.MagicMock(
1327+
side_effect=lambda comment: make_mock_issue_comment(
1328+
comment=comment, created_comments=created_comments
1329+
)
1330+
)
1331+
mock_issue.get_comments = mocker.MagicMock(side_effect=lambda: created_comments)
1332+
1333+
mock_pull_request = mock_repo.get_pull()
1334+
mock_pull_request.get_reviews = mocker.MagicMock(lambda: [])
1335+
mock_pull_request.merged = False
1336+
mock_pull_request.merge = mocker.MagicMock()
1337+
1338+
controller = make_controller(
1339+
"tests/fixtures/github/pull_request_synchronized.json",
1340+
github_client,
1341+
bot_config=GithubCICDBotConfig(merge_method=MergeMethod.REBASE, enable_deploy_command=True),
1342+
)
1343+
controller._context._run_tests = mocker.MagicMock(
1344+
side_effect=lambda **kwargs: (TestResult(), "")
1345+
)
1346+
1347+
github_output_file = tmp_path / "github_output.txt"
1348+
1349+
with mock.patch.dict(os.environ, {"GITHUB_OUTPUT": str(github_output_file)}):
1350+
command._run_all(controller)
1351+
1352+
assert "SQLMesh - Prod Plan Preview" in controller._check_run_mapping
1353+
assert "SQLMesh - PR Environment Synced" in controller._check_run_mapping
1354+
assert "SQLMesh - Prod Environment Synced" in controller._check_run_mapping
1355+
assert "SQLMesh - Run Unit Tests" in controller._check_run_mapping
1356+
prod_checks_runs = controller._check_run_mapping["SQLMesh - Prod Environment Synced"].all_kwargs
1357+
assert len(prod_checks_runs) == 2
1358+
assert GithubCheckStatus(prod_checks_runs[0]["status"]).is_queued
1359+
assert GithubCheckStatus(prod_checks_runs[1]["status"]).is_completed
1360+
assert prod_checks_runs[1]["output"]["title"] == "Skipped deployment"
1361+
assert (
1362+
prod_checks_runs[1]["output"]["summary"]
1363+
== "Skipped Deploying to Production because a `/deploy` command has not been detected yet"
1364+
)
1365+
assert GithubCheckConclusion(prod_checks_runs[1]["conclusion"]).is_skipped
1366+
1367+
# required approvers are irrelevant because /deploy command is enabled
1368+
assert "SQLMesh - Has Required Approval" not in controller._check_run_mapping

tests/integrations/github/cicd/test_integration.py

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -691,9 +691,11 @@ def test_merge_pr_has_non_breaking_change_no_categorization(
691691
assert GithubCheckStatus(prod_checks_runs[0]["status"]).is_queued
692692
assert GithubCheckStatus(prod_checks_runs[1]["status"]).is_completed
693693
assert GithubCheckConclusion(prod_checks_runs[1]["conclusion"]).is_skipped
694-
skip_reason = "Skipped Deploying to Production because the PR environment was not updated"
695-
assert prod_checks_runs[1]["output"]["title"] == skip_reason
696-
assert prod_checks_runs[1]["output"]["summary"] == skip_reason
694+
assert prod_checks_runs[1]["output"]["title"] == "Skipped deployment"
695+
assert (
696+
prod_checks_runs[1]["output"]["summary"]
697+
== "Skipped Deploying to Production because the PR environment was not updated"
698+
)
697699

698700
assert "SQLMesh - Has Required Approval" in controller._check_run_mapping
699701
approval_checks_runs = controller._check_run_mapping[
@@ -1024,9 +1026,11 @@ def test_no_merge_since_no_deploy_signal(
10241026
assert GithubCheckStatus(prod_checks_runs[0]["status"]).is_queued
10251027
assert GithubCheckStatus(prod_checks_runs[1]["status"]).is_completed
10261028
assert GithubCheckConclusion(prod_checks_runs[1]["conclusion"]).is_skipped
1027-
skip_reason = "Skipped Deploying to Production because a required approver has not approved"
1028-
assert prod_checks_runs[1]["output"]["title"] == skip_reason
1029-
assert prod_checks_runs[1]["output"]["summary"] == skip_reason
1029+
assert prod_checks_runs[1]["output"]["title"] == "Skipped deployment"
1030+
assert (
1031+
prod_checks_runs[1]["output"]["summary"]
1032+
== "Skipped Deploying to Production because a required approver has not approved"
1033+
)
10301034

10311035
assert "SQLMesh - Has Required Approval" in controller._check_run_mapping
10321036
approval_checks_runs = controller._check_run_mapping[
@@ -1528,9 +1532,11 @@ def test_error_msg_when_applying_plan_with_bug(
15281532
assert GithubCheckStatus(prod_checks_runs[0]["status"]).is_queued
15291533
assert GithubCheckStatus(prod_checks_runs[1]["status"]).is_completed
15301534
assert GithubCheckConclusion(prod_checks_runs[1]["conclusion"]).is_skipped
1531-
skip_reason = "Skipped Deploying to Production because the PR environment was not updated"
1532-
assert prod_checks_runs[1]["output"]["title"] == skip_reason
1533-
assert prod_checks_runs[1]["output"]["summary"] == skip_reason
1535+
assert prod_checks_runs[1]["output"]["title"] == "Skipped deployment"
1536+
assert (
1537+
prod_checks_runs[1]["output"]["summary"]
1538+
== "Skipped Deploying to Production because the PR environment was not updated"
1539+
)
15341540

15351541
assert "SQLMesh - Has Required Approval" in controller._check_run_mapping
15361542
approval_checks_runs = controller._check_run_mapping[

0 commit comments

Comments
 (0)