diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 6de5e5b9790eb..fb2a73bac57a3 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -5,6 +5,7 @@ on: branches: - master pull_request: + types: [opened, synchronize, reopened, labeled, unlabeled] merge_group: types: [checks_requested] diff --git a/ddev/src/ddev/cli/validate/all/__init__.py b/ddev/src/ddev/cli/validate/all/__init__.py index 216313ef57fe2..d0d98a033e628 100644 --- a/ddev/src/ddev/cli/validate/all/__init__.py +++ b/ddev/src/ddev/cli/validate/all/__init__.py @@ -55,7 +55,7 @@ def all( If TARGET is provided (e.g. 'changed'), per-integration validations are scoped to that target. Repo-wide validations always run without a target. """ - from ddev.cli.validate.all.github import get_pr_number, write_step_summary + from ddev.cli.validate.all.github import get_pr_number, should_suppress_validation_comments, write_step_summary from ddev.cli.validate.all.orchestrator import ValidationOrchestrator selected = _load_validations(app) @@ -69,12 +69,14 @@ def all( app.abort() pr_number = get_pr_number(app) + suppress_pr_comments = should_suppress_validation_comments() orchestrator = ValidationOrchestrator( app=app, target=target, validations=list(selected), fix=fix, pr_number=pr_number, + suppress_pr_comments=suppress_pr_comments, grace_period=grace_period, max_timeout=max_timeout, subprocess_timeout=subprocess_timeout, diff --git a/ddev/src/ddev/cli/validate/all/github.py b/ddev/src/ddev/cli/validate/all/github.py index 15f70aeade560..16019a89ac928 100644 --- a/ddev/src/ddev/cli/validate/all/github.py +++ b/ddev/src/ddev/cli/validate/all/github.py @@ -15,6 +15,10 @@ from ddev.cli.validate.all.orchestrator import ValidationConfig, ValidationResult COMMENT_HEADING = "## Validation Report" +COMMENT_STATUS_SUCCESS = "" +COMMENT_STATUS_ACTION_REQUIRED = "" +# Suppresses all validation PR comments, including failures, on the next validation run. +VALIDATION_COMMENT_SUPPRESSION_LABEL = "ci/skip-validation-comments" def parse_pr_number_from_event(event_path: str) -> int | None: @@ -59,6 +63,32 @@ def get_pr_number(app: Application) -> int | None: return None +def pr_has_label_from_event(event_path: str, label: str) -> bool: + """Return whether the GitHub Actions PR event payload contains a label.""" + try: + event = json.loads(Path(event_path).read_text()) + except (json.JSONDecodeError, OSError): + return False + + pr = event.get("pull_request") + if not isinstance(pr, dict): + return False + + labels = pr.get("labels", []) + if not isinstance(labels, list): + return False + + return any(isinstance(item, dict) and item.get("name") == label for item in labels) + + +def should_suppress_validation_comments() -> bool: + if os.environ.get("GITHUB_EVENT_NAME") != "pull_request": + return False + if event_path := os.environ.get("GITHUB_EVENT_PATH"): + return pr_has_label_from_event(event_path, VALIDATION_COMMENT_SUPPRESSION_LABEL) + return False + + def get_workflow_run_url() -> str | None: server = os.environ.get("GITHUB_SERVER_URL") repo = os.environ.get("GITHUB_REPOSITORY") @@ -75,8 +105,14 @@ def write_step_summary(content: str) -> None: f.write(content + "\n") -def _build_preamble(error: str | None, warning: str | None) -> list[str]: +def is_successful_validation_comment(body: str) -> bool: + return COMMENT_STATUS_SUCCESS in body + + +def _build_preamble(error: str | None, warning: str | None, status_marker: str | None = None) -> list[str]: parts: list[str] = [f"{COMMENT_HEADING}\n"] + if status_marker: + parts.append(f"{status_marker}\n") if error: parts.append(f"> **Error:** {error}\n") if warning: @@ -136,6 +172,7 @@ def format_pr_comment( *, error: str | None = None, warning: str | None = None, + successful: bool = False, ) -> str: """Format a PR comment with collapsible sections to reduce clutter.""" failures: dict[str, ValidationResult] = {} @@ -144,7 +181,8 @@ def format_pr_comment( (passed if result.success else failures)[name] = result incomplete = _build_incomplete_warning(expected_validations, results) - parts = _build_preamble(error, warning) + status_marker = COMMENT_STATUS_SUCCESS if successful else COMMENT_STATUS_ACTION_REQUIRED + parts = _build_preamble(error, warning, status_marker) parts.extend(incomplete) if failures: diff --git a/ddev/src/ddev/cli/validate/all/orchestrator.py b/ddev/src/ddev/cli/validate/all/orchestrator.py index 9a6c7e8c8ba04..888b39ade6a13 100644 --- a/ddev/src/ddev/cli/validate/all/orchestrator.py +++ b/ddev/src/ddev/cli/validate/all/orchestrator.py @@ -11,13 +11,14 @@ import time from concurrent.futures import ThreadPoolExecutor from dataclasses import dataclass, field -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any, TypedDict from ddev.cli.validate.all.github import ( COMMENT_HEADING, format_pr_comment, format_step_summary, get_workflow_run_url, + is_successful_validation_comment, write_step_summary, ) from ddev.event_bus.orchestrator import BaseMessage, EventBusOrchestrator, SyncProcessor @@ -26,6 +27,11 @@ from ddev.cli.application import Application +class GithubComment(TypedDict): + id: int + body: str + + @dataclass(frozen=True) class ValidationConfig: description: str = "" @@ -183,6 +189,7 @@ def __init__( validations: list[str] | None = None, fix: bool = False, pr_number: int | None = None, + suppress_pr_comments: bool = False, grace_period: float = 5, max_timeout: float = 600, subprocess_timeout: float = SUBPROCESS_TIMEOUT, @@ -199,6 +206,7 @@ def __init__( self._target = target self._fix = fix self._pr_number = pr_number + self._suppress_pr_comments = suppress_pr_comments self._results: dict[str, ValidationResult] = {} self.register_processor( @@ -236,28 +244,38 @@ def _build_error_and_warning(self, exception: Exception | None) -> tuple[str | N return error_msg, extra_warning - def _delete_previous_comments(self, pr_number: int) -> None: - try: - comments = self._app.github.get_pull_request_comments(pr_number) - for comment in comments: - if comment.get("body", "").startswith(COMMENT_HEADING): - self._app.github.delete_comment(comment["id"]) - except Exception as exc: - self._app.display_warning(f"Failed to clean up previous validation comments: {exc}") - - def _publish_report(self, exception: Exception | None) -> None: - error_msg, extra_warning = self._build_error_and_warning(exception) - - summary_body = format_step_summary( - self._results, - VALIDATIONS, - self._target, - self._validations, - error=error_msg, - warning=extra_warning, + def _current_run_succeeded(self, exception: Exception | None) -> bool: + return ( + exception is None + and len(self._results) == len(self._validations) + and all(result.success for result in self._results.values()) ) - write_step_summary(summary_body) + def _get_previous_validation_comments(self, pr_number: int) -> list[GithubComment]: + comments: list[dict[str, Any]] = self._app.github.get_pull_request_comments(pr_number) + return [ + {"id": comment["id"], "body": comment.get("body", "")} + for comment in comments + if comment.get("body", "").startswith(COMMENT_HEADING) + ] + + def _delete_comments(self, comments: list[GithubComment]) -> None: + for comment in comments: + try: + self._app.github.delete_comment(comment["id"]) + except Exception as exc: + self._app.display_warning( + f"Failed to delete previous validation comment {comment['id']}: {type(exc).__name__}: {exc}" + ) + + def _previous_success_already_reported( + self, current_succeeded: bool, previous_comments: list[GithubComment] + ) -> bool: + if not current_succeeded or not previous_comments: + return False + return all(is_successful_validation_comment(comment["body"]) for comment in previous_comments) + + def _build_pr_comment_body(self, error_msg: str | None, extra_warning: str | None, current_succeeded: bool) -> str: comment_body = format_pr_comment( self._results, VALIDATIONS, @@ -265,9 +283,54 @@ def _publish_report(self, exception: Exception | None) -> None: self._validations, error=error_msg, warning=extra_warning, + successful=current_succeeded, ) if run_url := get_workflow_run_url(): comment_body += f"\n\n[View full run]({run_url})" + return comment_body + + def _fetch_previous_validation_comments_or_empty(self, pr_number: int) -> list[GithubComment]: + self._app.logger.debug("Fetching previous validation comments on PR #%s...", pr_number) + try: + return self._get_previous_validation_comments(pr_number) + except Exception as exc: + self._app.display_warning(f"Failed to read previous validation comments: {type(exc).__name__}: {exc}") + return [] + + def _sync_pr_comment(self, comment_body: str, current_succeeded: bool) -> None: + if self._pr_number is None: + return + + previous_comments = self._fetch_previous_validation_comments_or_empty(self._pr_number) + if self._suppress_pr_comments: + self._app.logger.debug("Validation PR comments are suppressed for PR #%s.", self._pr_number) + self._delete_comments(previous_comments) + return + if self._previous_success_already_reported(current_succeeded, previous_comments): + self._app.logger.debug("Previous validation comments already reported success; skipping PR comment.") + return + + self._app.logger.debug("Deleting previous validation comments on PR #%s...", self._pr_number) + self._delete_comments(previous_comments) + self._app.logger.debug("Posting validation comment on PR #%s...", self._pr_number) + self._app.github.post_pull_request_comment(self._pr_number, comment_body) + self._app.logger.debug("Comment posted successfully.") + + def _publish_report(self, exception: Exception | None) -> None: + error_msg, extra_warning = self._build_error_and_warning(exception) + write_step_summary( + format_step_summary( + self._results, + VALIDATIONS, + self._target, + self._validations, + error=error_msg, + warning=extra_warning, + ) + ) + + current_succeeded = self._current_run_succeeded(exception) + comment_body = self._build_pr_comment_body(error_msg, extra_warning, current_succeeded) self._app.logger.debug("PR number: %s", self._pr_number) self._app.logger.debug("GitHub token configured: %s", bool(self._app.config.github.token)) @@ -283,11 +346,7 @@ def _publish_report(self, exception: Exception | None) -> None: previous_level = httpx_logger.level httpx_logger.setLevel(logging.WARNING) try: - self._app.logger.debug("Deleting previous validation comments on PR #%s...", self._pr_number) - self._delete_previous_comments(self._pr_number) - self._app.logger.debug("Posting validation comment on PR #%s...", self._pr_number) - self._app.github.post_pull_request_comment(self._pr_number, comment_body) - self._app.logger.debug("Comment posted successfully.") + self._sync_pr_comment(comment_body, current_succeeded) except Exception as exc: self._app.display_warning(f"Failed to post PR comment: {type(exc).__name__}: {exc}") write_step_summary(f"\n> Failed to post PR comment: {type(exc).__name__}: {exc}") diff --git a/ddev/tests/cli/validate/all/test_command.py b/ddev/tests/cli/validate/all/test_command.py index 5395b51c7daa0..c1e49849f9803 100644 --- a/ddev/tests/cli/validate/all/test_command.py +++ b/ddev/tests/cli/validate/all/test_command.py @@ -5,6 +5,9 @@ from unittest.mock import patch +import pytest + +from ddev.cli.validate.all.github import VALIDATION_COMMENT_SUPPRESSION_LABEL from ddev.cli.validate.all.orchestrator import VALIDATIONS from .conftest import completed_process @@ -117,3 +120,34 @@ def test_all_command_aborts_when_no_validations_configured(ddev): assert result.exit_code != 0 assert NO_VALIDATIONS_ERROR in result.output + + +@pytest.mark.parametrize( + "label, expected", + [ + pytest.param(VALIDATION_COMMENT_SUPPRESSION_LABEL, True, id="label-present"), + pytest.param("other-label", False, id="label-absent"), + ], +) +def test_all_command_passes_comment_suppression_label_state(ddev, tmp_path, monkeypatch, label, expected): + event_file = tmp_path / "event.json" + event_file.write_text(f'{{"pull_request": {{"labels": [{{"name": "{label}"}}]}}}}') + monkeypatch.setenv("GITHUB_EVENT_NAME", "pull_request") + monkeypatch.setenv("GITHUB_EVENT_PATH", str(event_file)) + captured: dict[str, object] = {} + + class FakeOrchestrator: + def __init__(self, **kwargs): + captured.update(kwargs) + + def run(self): + pass + + with ( + patch("ddev.cli.validate.all._load_validations", return_value={"config": VALIDATIONS["config"]}), + patch("ddev.cli.validate.all.orchestrator.ValidationOrchestrator", FakeOrchestrator), + ): + result = ddev("validate", "all", *FAST_ORCHESTRATOR_OPTS) + + assert result.exit_code == 0, result.output + assert captured["suppress_pr_comments"] is expected diff --git a/ddev/tests/cli/validate/all/test_github.py b/ddev/tests/cli/validate/all/test_github.py index e2701985f509b..ff25326624cc9 100644 --- a/ddev/tests/cli/validate/all/test_github.py +++ b/ddev/tests/cli/validate/all/test_github.py @@ -6,12 +6,18 @@ import pytest from ddev.cli.validate.all.github import ( + COMMENT_STATUS_ACTION_REQUIRED, + COMMENT_STATUS_SUCCESS, + VALIDATION_COMMENT_SUPPRESSION_LABEL, format_pr_comment, format_step_summary, get_pr_number, get_workflow_run_url, + is_successful_validation_comment, parse_pr_number_from_event, parse_pr_number_from_ref, + pr_has_label_from_event, + should_suppress_validation_comments, write_step_summary, ) from ddev.cli.validate.all.orchestrator import ValidationConfig, ValidationResult @@ -127,6 +133,8 @@ def test_format_pr_comment_all_passed(helpers): expected = helpers.dedent(""" ## Validation Report + + All 2 validations passed.
@@ -138,7 +146,7 @@ def test_format_pr_comment_all_passed(helpers): | `config` | Validate default configuration files against spec.yaml | ✅ |
""") - assert format_pr_comment(results, CONFIGS, "changed", list(results)) == expected + assert format_pr_comment(results, CONFIGS, "changed", list(results), successful=True) == expected def test_format_pr_comment_one_failure_with_target(helpers): @@ -149,6 +157,8 @@ def test_format_pr_comment_one_failure_with_target(helpers): expected = helpers.dedent(""" ## Validation Report + + | Validation | Description | Status | |---|---|---| | `config` | Validate default configuration files against spec.yaml | ❌ | @@ -173,6 +183,8 @@ def test_format_pr_comment_all_failures_no_details_section(helpers): expected = helpers.dedent(""" ## Validation Report + + | Validation | Description | Status | |---|---|---| | `config` | Validate default configuration files against spec.yaml | ❌ | @@ -185,7 +197,7 @@ def test_format_pr_comment_no_fix_command_when_all_pass(): results = { "config": ValidationResult(name="config", success=True, stdout="ok", stderr="", duration=1.0), } - comment = format_pr_comment(results, CONFIGS, None, list(results)) + comment = format_pr_comment(results, CONFIGS, None, list(results), successful=True) assert "All 1 validations passed." in comment assert "ddev validate all" not in comment @@ -197,6 +209,8 @@ def test_format_pr_comment_with_error_and_warning(helpers): expected = helpers.dedent(""" ## Validation Report + + > **Error:** Error running validations: boom > **Warning:** Could not determine PR number @@ -231,7 +245,7 @@ def test_format_pr_comment_missing_config_uses_empty_description(): results = { "unknown": ValidationResult(name="unknown", success=True, stdout="ok", stderr="", duration=1.0), } - comment = format_pr_comment(results, CONFIGS, None, list(results)) + comment = format_pr_comment(results, CONFIGS, None, list(results), successful=True) assert "| `unknown` | | ✅ |" in comment @@ -253,6 +267,68 @@ def test_format_step_summary_incomplete_validations_warns(): assert "2 validation(s) did not complete: `config`, `metadata`" in summary +def test_format_step_summary_does_not_include_comment_status(): + results = { + "config": ValidationResult(name="config", success=True, stdout="ok", stderr="", duration=1.0), + } + summary = format_step_summary(results, CONFIGS, None, ["config"]) + assert COMMENT_STATUS_SUCCESS not in summary + assert COMMENT_STATUS_ACTION_REQUIRED not in summary + + +@pytest.mark.parametrize( + "body, expected", + [ + pytest.param(f"## Validation Report\n\n{COMMENT_STATUS_SUCCESS}\n\nAll good", True, id="success-marker"), + pytest.param( + f"## Validation Report\n\n{COMMENT_STATUS_ACTION_REQUIRED}\n\nBad", False, id="action-required-marker" + ), + pytest.param("## Validation Report\n\nAll 2 validations passed.", False, id="legacy-success"), + pytest.param("unrelated comment", False, id="unrelated-comment"), + ], +) +def test_is_successful_validation_comment(body, expected): + assert is_successful_validation_comment(body) is expected + + +@pytest.mark.parametrize( + "content, expected", + [ + pytest.param( + f'{{"pull_request": {{"labels": [{{"name": "{VALIDATION_COMMENT_SUPPRESSION_LABEL}"}}]}}}}', + True, + id="label-present", + ), + pytest.param('{"pull_request": {"labels": [{"name": "other"}]}}', False, id="other-label"), + pytest.param('{"pull_request": {"labels": []}}', False, id="no-labels"), + pytest.param('{"pull_request": {}}', False, id="missing-labels"), + pytest.param('{"pull_request": "bad"}', False, id="pr-not-dict"), + pytest.param("{bad json}", False, id="malformed-json"), + ], +) +def test_pr_has_label_from_event(tmp_path, content, expected): + event_file = tmp_path / "event.json" + event_file.write_text(content) + assert pr_has_label_from_event(str(event_file), VALIDATION_COMMENT_SUPPRESSION_LABEL) is expected + + +@pytest.mark.parametrize( + "event_name, expected", + [ + pytest.param("pull_request", True, id="pull-request"), + pytest.param("pull_request_target", False, id="pull-request-target"), + pytest.param("pull_request_review", False, id="pull-request-review"), + ], +) +def test_should_suppress_validation_comments_only_for_pull_request_events(tmp_path, monkeypatch, event_name, expected): + event_file = tmp_path / "event.json" + event_file.write_text(f'{{"pull_request": {{"labels": [{{"name": "{VALIDATION_COMMENT_SUPPRESSION_LABEL}"}}]}}}}') + monkeypatch.setenv("GITHUB_EVENT_NAME", event_name) + monkeypatch.setenv("GITHUB_EVENT_PATH", str(event_file)) + + assert should_suppress_validation_comments() is expected + + # --- format_step_summary --- diff --git a/ddev/tests/cli/validate/all/test_orchestrator.py b/ddev/tests/cli/validate/all/test_orchestrator.py index bc379d89c943b..dc7a8d6795d5e 100644 --- a/ddev/tests/cli/validate/all/test_orchestrator.py +++ b/ddev/tests/cli/validate/all/test_orchestrator.py @@ -10,6 +10,7 @@ import pytest from ddev.cli.validate.all import _load_validations +from ddev.cli.validate.all.github import COMMENT_STATUS_ACTION_REQUIRED, COMMENT_STATUS_SUCCESS from ddev.cli.validate.all.orchestrator import ( VALIDATIONS, ValidationMessage, @@ -204,6 +205,7 @@ def test_on_finalize_writes_step_summary(mock_app, tmp_path, monkeypatch): def test_on_finalize_posts_pr_comment_on_failure(mock_app): mock_app.config.github.token = "fake-token" + mock_app.github.get_pull_request_comments.return_value = [] orch = ValidationOrchestrator(app=mock_app, validations=["config"], target=None, pr_number=42) orch._results = { @@ -219,12 +221,14 @@ def test_on_finalize_posts_pr_comment_on_failure(mock_app): assert "Validate default configuration files against spec.yaml" in body assert "| `config` |" in body assert "❌" in body + assert COMMENT_STATUS_ACTION_REQUIRED in body assert "[View full run](https://github.com/DataDog/integrations-core/actions/runs/12345)" in body def test_on_finalize_pr_comment_omits_run_link_when_env_missing(mock_app, monkeypatch): monkeypatch.delenv("GITHUB_RUN_ID") mock_app.config.github.token = "fake-token" + mock_app.github.get_pull_request_comments.return_value = [] orch = ValidationOrchestrator(app=mock_app, validations=["config"], target=None, pr_number=42) orch._results = { @@ -242,6 +246,7 @@ def test_on_finalize_step_summary_does_not_include_run_link(mock_app, tmp_path, monkeypatch.setenv("GITHUB_STEP_SUMMARY", str(summary_file)) mock_app.config.github.token = "fake-token" + mock_app.github.get_pull_request_comments.return_value = [] orch = ValidationOrchestrator(app=mock_app, validations=["config"], target=None, pr_number=42) orch._results = { "config": ValidationResult(name="config", success=False, stdout="err", stderr="", duration=1.0), @@ -255,6 +260,7 @@ def test_on_finalize_step_summary_does_not_include_run_link(mock_app, tmp_path, def test_on_finalize_posts_pr_comment_on_success(mock_app): mock_app.config.github.token = "fake-token" + mock_app.github.get_pull_request_comments.return_value = [] orch = ValidationOrchestrator(app=mock_app, validations=["config", "ci"], target=None, pr_number=42) orch._results = { @@ -269,6 +275,23 @@ def test_on_finalize_posts_pr_comment_on_success(mock_app): assert "
" in body assert "| `ci` |" in body assert "| `config` |" in body + assert COMMENT_STATUS_SUCCESS in body + + +def test_on_finalize_posts_pr_comment_when_fetching_previous_comments_fails(mock_app): + mock_app.config.github.token = "fake-token" + mock_app.github.get_pull_request_comments.side_effect = RuntimeError("network error") + + orch = ValidationOrchestrator(app=mock_app, validations=["config"], target=None, pr_number=42) + orch._results = { + "config": ValidationResult(name="config", success=True, stdout="ok", stderr="", duration=1.0), + } + asyncio.run(orch.on_finalize(exception=None)) + + mock_app.display_warning.assert_called() + warning_args = [str(c) for c in mock_app.display_warning.call_args_list] + assert any("Failed to read previous validation comments" in w for w in warning_args) + mock_app.github.post_pull_request_comment.assert_called_once() def test_on_finalize_deletes_previous_validation_comments(mock_app): @@ -288,6 +311,132 @@ def test_on_finalize_deletes_previous_validation_comments(mock_app): mock_app.github.post_pull_request_comment.assert_called_once() +def test_on_finalize_posts_pr_comment_when_deleting_previous_comment_fails(mock_app): + mock_app.config.github.token = "fake-token" + mock_app.github.get_pull_request_comments.return_value = [ + {"id": 100, "body": "## Validation Report\nold report"}, + ] + mock_app.github.delete_comment.side_effect = RuntimeError("not found") + + orch = ValidationOrchestrator(app=mock_app, validations=["config"], target=None, pr_number=42) + orch._results = { + "config": ValidationResult(name="config", success=True, stdout="ok", stderr="", duration=1.0), + } + asyncio.run(orch.on_finalize(exception=None)) + + mock_app.github.delete_comment.assert_called_once_with(100) + mock_app.github.post_pull_request_comment.assert_called_once() + warning_args = [str(c) for c in mock_app.display_warning.call_args_list] + assert any("Failed to delete previous validation comment 100" in w for w in warning_args) + + +def test_on_finalize_skips_pr_comment_on_repeated_success(mock_app): + mock_app.config.github.token = "fake-token" + mock_app.github.get_pull_request_comments.return_value = [ + {"id": 100, "body": f"## Validation Report\n\n{COMMENT_STATUS_SUCCESS}\n\nAll 1 validations passed."}, + ] + + orch = ValidationOrchestrator(app=mock_app, validations=["config"], target=None, pr_number=42) + orch._results = { + "config": ValidationResult(name="config", success=True, stdout="ok", stderr="", duration=1.0), + } + asyncio.run(orch.on_finalize(exception=None)) + + mock_app.github.get_pull_request_comments.assert_called_once_with(42) + mock_app.github.delete_comment.assert_not_called() + mock_app.github.post_pull_request_comment.assert_not_called() + + +def test_on_finalize_posts_recovery_comment_after_failure(mock_app): + mock_app.config.github.token = "fake-token" + mock_app.github.get_pull_request_comments.return_value = [ + {"id": 100, "body": f"## Validation Report\n\n{COMMENT_STATUS_ACTION_REQUIRED}\n\nold failure"}, + ] + + orch = ValidationOrchestrator(app=mock_app, validations=["config"], target=None, pr_number=42) + orch._results = { + "config": ValidationResult(name="config", success=True, stdout="ok", stderr="", duration=1.0), + } + asyncio.run(orch.on_finalize(exception=None)) + + mock_app.github.delete_comment.assert_called_once_with(100) + mock_app.github.post_pull_request_comment.assert_called_once() + body = mock_app.github.post_pull_request_comment.call_args[0][1] + assert COMMENT_STATUS_SUCCESS in body + + +def test_on_finalize_posts_failure_comment_after_success(mock_app): + mock_app.config.github.token = "fake-token" + mock_app.github.get_pull_request_comments.return_value = [ + {"id": 100, "body": f"## Validation Report\n\n{COMMENT_STATUS_SUCCESS}\n\nold success"}, + ] + + orch = ValidationOrchestrator(app=mock_app, validations=["config"], target=None, pr_number=42) + orch._results = { + "config": ValidationResult(name="config", success=False, stdout="err", stderr="", duration=1.0), + } + with pytest.raises(SystemExit): + asyncio.run(orch.on_finalize(exception=None)) + + mock_app.github.delete_comment.assert_called_once_with(100) + mock_app.github.post_pull_request_comment.assert_called_once() + body = mock_app.github.post_pull_request_comment.call_args[0][1] + assert COMMENT_STATUS_ACTION_REQUIRED in body + + +def test_on_finalize_posts_success_when_previous_comment_has_no_status(mock_app): + mock_app.config.github.token = "fake-token" + mock_app.github.get_pull_request_comments.return_value = [ + {"id": 100, "body": "## Validation Report\n\nAll 1 validations passed."}, + ] + + orch = ValidationOrchestrator(app=mock_app, validations=["config"], target=None, pr_number=42) + orch._results = { + "config": ValidationResult(name="config", success=True, stdout="ok", stderr="", duration=1.0), + } + asyncio.run(orch.on_finalize(exception=None)) + + mock_app.github.delete_comment.assert_called_once_with(100) + mock_app.github.post_pull_request_comment.assert_called_once() + body = mock_app.github.post_pull_request_comment.call_args[0][1] + assert COMMENT_STATUS_SUCCESS in body + + +def test_on_finalize_suppresses_pr_comments_and_deletes_previous_comments(mock_app): + mock_app.config.github.token = "fake-token" + mock_app.github.get_pull_request_comments.return_value = [ + {"id": 100, "body": f"## Validation Report\n\n{COMMENT_STATUS_ACTION_REQUIRED}\n\nold failure"}, + {"id": 200, "body": "unrelated comment"}, + ] + + orch = ValidationOrchestrator( + app=mock_app, validations=["config"], target=None, pr_number=42, suppress_pr_comments=True + ) + orch._results = { + "config": ValidationResult(name="config", success=True, stdout="ok", stderr="", duration=1.0), + } + asyncio.run(orch.on_finalize(exception=None)) + + mock_app.github.delete_comment.assert_called_once_with(100) + mock_app.github.post_pull_request_comment.assert_not_called() + + +def test_on_finalize_suppresses_pr_comments_with_no_previous_comments(mock_app): + mock_app.config.github.token = "fake-token" + mock_app.github.get_pull_request_comments.return_value = [] + + orch = ValidationOrchestrator( + app=mock_app, validations=["config"], target=None, pr_number=42, suppress_pr_comments=True + ) + orch._results = { + "config": ValidationResult(name="config", success=True, stdout="ok", stderr="", duration=1.0), + } + asyncio.run(orch.on_finalize(exception=None)) + + mock_app.github.delete_comment.assert_not_called() + mock_app.github.post_pull_request_comment.assert_not_called() + + def test_on_finalize_includes_pr_warning_in_summary(mock_app, tmp_path, monkeypatch): summary_file = tmp_path / "summary.md" monkeypatch.setenv("GITHUB_STEP_SUMMARY", str(summary_file)) @@ -306,6 +455,7 @@ def test_on_finalize_includes_pr_warning_in_summary(mock_app, tmp_path, monkeypa def test_on_finalize_handles_post_failure(mock_app): mock_app.config.github.token = "fake-token" + mock_app.github.get_pull_request_comments.return_value = [] mock_app.github.post_pull_request_comment.side_effect = RuntimeError("network error") orch = ValidationOrchestrator(app=mock_app, validations=["config"], target=None, pr_number=42)