Skip to content
4 changes: 3 additions & 1 deletion ddev/src/ddev/cli/validate/all/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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,
Expand Down
39 changes: 37 additions & 2 deletions ddev/src/ddev/cli/validate/all/github.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
from ddev.cli.validate.all.orchestrator import ValidationConfig, ValidationResult

COMMENT_HEADING = "## Validation Report"
COMMENT_STATUS_SUCCESS = "<!-- ddev-validation-report:success -->"
COMMENT_STATUS_ACTION_REQUIRED = "<!-- ddev-validation-report:action-required -->"
VALIDATION_COMMENT_SUPPRESSION_LABEL = "ci/skip-validation-comments"


def parse_pr_number_from_event(event_path: str) -> int | None:
Expand Down Expand Up @@ -59,6 +62,30 @@ 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 event_path := os.environ.get("GITHUB_EVENT_PATH"):
return pr_has_label_from_event(event_path, VALIDATION_COMMENT_SUPPRESSION_LABEL)
Comment thread
nubtron marked this conversation as resolved.
return False


def get_workflow_run_url() -> str | None:
server = os.environ.get("GITHUB_SERVER_URL")
repo = os.environ.get("GITHUB_REPOSITORY")
Expand All @@ -75,8 +102,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:
Expand Down Expand Up @@ -136,6 +169,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] = {}
Expand All @@ -144,7 +178,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:
Expand Down
42 changes: 33 additions & 9 deletions ddev/src/ddev/cli/validate/all/orchestrator.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
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
Expand Down Expand Up @@ -183,6 +184,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,
Expand All @@ -199,6 +201,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(
Expand Down Expand Up @@ -236,14 +239,20 @@ 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 _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())
)

def _get_previous_validation_comments(self, pr_number: int) -> list[dict]:
comments = self._app.github.get_pull_request_comments(pr_number)
return [comment for comment in comments if comment.get("body", "").startswith(COMMENT_HEADING)]

def _delete_comments(self, comments: list[dict]) -> None:
for comment in comments:
self._app.github.delete_comment(comment["id"])
Comment thread
nubtron marked this conversation as resolved.
Outdated

def _publish_report(self, exception: Exception | None) -> None:
error_msg, extra_warning = self._build_error_and_warning(exception)
Expand All @@ -258,13 +267,15 @@ def _publish_report(self, exception: Exception | None) -> None:
)
write_step_summary(summary_body)

current_succeeded = self._current_run_succeeded(exception)
comment_body = format_pr_comment(
self._results,
VALIDATIONS,
self._target,
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})"
Expand All @@ -283,8 +294,21 @@ def _publish_report(self, exception: Exception | None) -> None:
previous_level = httpx_logger.level
httpx_logger.setLevel(logging.WARNING)
try:
self._app.logger.debug("Fetching previous validation comments on PR #%s...", self._pr_number)
previous_comments = self._get_previous_validation_comments(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 (
current_succeeded
and previous_comments
and all(is_successful_validation_comment(comment.get("body", "")) for comment in 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_previous_comments(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.")
Expand Down
25 changes: 25 additions & 0 deletions ddev/tests/cli/validate/all/test_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

from unittest.mock import patch

from ddev.cli.validate.all.github import VALIDATION_COMMENT_SUPPRESSION_LABEL
from ddev.cli.validate.all.orchestrator import VALIDATIONS

from .conftest import completed_process
Expand Down Expand Up @@ -117,3 +118,27 @@ def test_all_command_aborts_when_no_validations_configured(ddev):

assert result.exit_code != 0
assert NO_VALIDATIONS_ERROR in result.output


def test_all_command_passes_comment_suppression_label_state(ddev, tmp_path, monkeypatch):
event_file = tmp_path / "event.json"
event_file.write_text(f'{{"pull_request": {{"labels": [{{"name": "{VALIDATION_COMMENT_SUPPRESSION_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 True
64 changes: 61 additions & 3 deletions ddev/tests/cli/validate/all/test_github.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,17 @@
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,
write_step_summary,
)
from ddev.cli.validate.all.orchestrator import ValidationConfig, ValidationResult
Expand Down Expand Up @@ -127,6 +132,8 @@ def test_format_pr_comment_all_passed(helpers):
expected = helpers.dedent("""
## Validation Report

<!-- ddev-validation-report:success -->

All 2 validations passed.

<details>
Expand All @@ -138,7 +145,7 @@ def test_format_pr_comment_all_passed(helpers):
| `config` | Validate default configuration files against spec.yaml | ✅ |

</details>""")
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):
Expand All @@ -149,6 +156,8 @@ def test_format_pr_comment_one_failure_with_target(helpers):
expected = helpers.dedent("""
## Validation Report

<!-- ddev-validation-report:action-required -->

| Validation | Description | Status |
|---|---|---|
| `config` | Validate default configuration files against spec.yaml | ❌ |
Expand All @@ -173,6 +182,8 @@ def test_format_pr_comment_all_failures_no_details_section(helpers):
expected = helpers.dedent("""
## Validation Report

<!-- ddev-validation-report:action-required -->

| Validation | Description | Status |
|---|---|---|
| `config` | Validate default configuration files against spec.yaml | ❌ |
Expand All @@ -185,7 +196,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

Expand All @@ -197,6 +208,8 @@ def test_format_pr_comment_with_error_and_warning(helpers):
expected = helpers.dedent("""
## Validation Report

<!-- ddev-validation-report:action-required -->

> **Error:** Error running validations: boom

> **Warning:** Could not determine PR number
Expand Down Expand Up @@ -231,7 +244,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


Expand All @@ -253,6 +266,51 @@ 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


# --- format_step_summary ---


Expand Down
Loading
Loading