diff --git a/sqlmesh/core/console.py b/sqlmesh/core/console.py index d9567ae484..8af837b08a 100644 --- a/sqlmesh/core/console.py +++ b/sqlmesh/core/console.py @@ -3641,7 +3641,10 @@ def show_linter_violations( msg = f"\nLinter {severity} for `{model._path}`:\n{violations_msg}\n" self._print(msg) - self._errors.append(msg) + if is_error: + self._errors.append(msg) + else: + self._warnings.append(msg) @property def captured_warnings(self) -> str: diff --git a/tests/integrations/github/cicd/test_github_controller.py b/tests/integrations/github/cicd/test_github_controller.py index a27f75f459..8242d697b6 100644 --- a/tests/integrations/github/cicd/test_github_controller.py +++ b/tests/integrations/github/cicd/test_github_controller.py @@ -15,6 +15,7 @@ from sqlmesh.core.model import SqlModel from sqlmesh.core.user import User, UserRole from sqlmesh.core.plan.definition import Plan +from sqlmesh.core.linter.rule import RuleViolation from sqlmesh.integrations.github.cicd.config import GithubCICDBotConfig, MergeMethod from sqlmesh.integrations.github.cicd.controller import ( BotCommand, @@ -29,6 +30,29 @@ pytestmark = pytest.mark.github + +def add_linter_violations(controller: GithubController): + class _MockModel: + _path = "tests/linter_test.sql" + + class _MockLinterRule: + name = "mock_linter_rule" + + controller._console.show_linter_violations( + [ + RuleViolation( + rule=_MockLinterRule(), violation_msg="Linter warning", violation_range=None + ) + ], + _MockModel(), + ) + controller._console.show_linter_violations( + [RuleViolation(rule=_MockLinterRule(), violation_msg="Linter error", violation_range=None)], + _MockModel(), + is_error=True, + ) + + github_controller_approvers_params = [ ( "2 approvers, 1 required", @@ -660,12 +684,18 @@ def test_get_plan_summary_includes_warnings_and_errors( controller._console.log_warning("Warning 1\nWith multiline") controller._console.log_warning("Warning 2") controller._console.log_error("Error 1") + add_linter_violations(controller) summary = controller.get_plan_summary(controller.prod_plan) - assert ("> [!WARNING]\n>\n> - Warning 1\n> With multiline\n>\n> - Warning 2\n\n") in summary - - assert ("> [!CAUTION]\n>\n> Error 1\n\n") in summary + assert ("> [!WARNING]\n>\n> - Warning 1\n> With multiline\n>\n> - Warning 2\n>\n>") in summary + assert ( + "> Linter warnings for `tests/linter_test.sql`:\n> - mock_linter_rule: Linter warning\n>" + ) in summary + assert ("> [!CAUTION]\n>\n> - Error 1\n>\n>") in summary + assert ( + "> Linter **errors** for `tests/linter_test.sql`:\n> - mock_linter_rule: Linter error\n>" + ) in summary def test_get_pr_environment_summary_includes_warnings_and_errors( @@ -679,24 +709,39 @@ def test_get_pr_environment_summary_includes_warnings_and_errors( controller._console.log_warning("Warning 1") controller._console.log_error("Error 1") + add_linter_violations(controller) # completed with no exception triggers a SUCCESS conclusion and only shows warnings success_summary = controller.get_pr_environment_summary( conclusion=GithubCheckConclusion.SUCCESS ) - assert "> [!WARNING]\n>\n> Warning 1\n" in success_summary - assert "> [!CAUTION]\n>\n> Error 1" not in success_summary + assert "> [!WARNING]\n>\n> - Warning 1\n" in success_summary + assert ( + "> Linter warnings for `tests/linter_test.sql`:\n> - mock_linter_rule: Linter warning\n" + in success_summary + ) + assert "Error 1" not in success_summary + assert "mock_linter_rule: Linter error" not in success_summary # since they got consumed in the previous call controller._console.log_warning("Warning 1") controller._console.log_error("Error 1") + add_linter_violations(controller) # completed with an exception triggers a FAILED conclusion and shows errors error_summary = controller.get_pr_environment_summary( conclusion=GithubCheckConclusion.FAILURE, exception=SQLMeshError("Something broke") ) - assert "> [!WARNING]\n>\n> Warning 1\n" in error_summary - assert "> [!CAUTION]\n>\n> Error 1" in error_summary + assert "> [!WARNING]\n>\n> - Warning 1\n>\n" in error_summary + assert ( + "> Linter warnings for `tests/linter_test.sql`:\n> - mock_linter_rule: Linter warning\n" + in error_summary + ) + assert "[!CAUTION]\n>
\n>\n> - Error 1\n>\n" in error_summary + assert ( + "> Linter **errors** for `tests/linter_test.sql`:\n> - mock_linter_rule: Linter error\n" + in error_summary + ) def test_pr_comment_deploy_indicator_includes_command_namespace(