diff --git a/.ddev/config.toml b/.ddev/config.toml index 10b6a8771727a..c8d86469531ce 100644 --- a/.ddev/config.toml +++ b/.ddev/config.toml @@ -1,3 +1,29 @@ +# Validations to run with `ddev validate all`. +# Each entry must match a known validation name. +# Remove an entry to skip that validation for this repo. +validations = [ + "agent-reqs", + "ci", + "codeowners", + "config", + "dep", + "http", + "imports", + "integration-style", + "jmx-metrics", + "labeler", + "legacy-signature", + "license-headers", + "licenses", + "metadata", + "models", + "openmetrics", + "package", + "readmes", + "saved-views", + "version", +] + [overrides.display-name] datadog_checks_base = "Datadog Checks Base" datadog_checks_dev = "Datadog Checks Dev" diff --git a/.github/workflows/run-validations.yml b/.github/workflows/run-validations.yml index be524ac5c77f0..f22bf83de6273 100644 --- a/.github/workflows/run-validations.yml +++ b/.github/workflows/run-validations.yml @@ -6,106 +6,9 @@ on: repo: required: true type: string - ddev-version: required: false type: string - agent-reqs: - required: false - default: false - type: boolean - ci: - required: false - default: false - type: boolean - codeowners: - required: false - default: false - type: boolean - config: - required: false - default: false - type: boolean - dashboards: - required: false - default: false - type: boolean - dep: - required: false - default: false - type: boolean - eula: - required: false - default: false - type: boolean - http: - required: false - default: false - type: boolean - imports: - required: false - default: false - type: boolean - integration-style: - required: false - default: false - type: boolean - jmx-metrics: - required: false - default: false - type: boolean - labeler: - required: false - default: false - type: boolean - legacy-signature: - required: false - default: false - type: boolean - license-headers: - required: false - default: false - type: boolean - licenses: - required: false - default: false - type: boolean - metadata: - required: false - default: false - type: boolean - models: - required: false - default: false - type: boolean - openmetrics: - required: false - default: false - type: boolean - package: - required: false - default: false - type: boolean - readmes: - required: false - default: false - type: boolean - saved-views: - required: false - default: false - type: boolean - service-checks: - required: false - default: false - type: boolean - typos: - required: false - default: false - type: boolean - version: - required: false - default: false - type: boolean defaults: run: @@ -117,6 +20,9 @@ jobs: # Avoid blocking merge queues by validations that can break with false positives if: github.event_name != 'merge_group' runs-on: ubuntu-22.04 + permissions: + id-token: write + contents: read env: PYTHON_VERSION: "3.13" @@ -140,7 +46,6 @@ jobs: - name: Install ddev from PyPI if: inputs.repo != 'core' - # When ddev version is empty we install the latest released ddev. run: pip install ddev${{ inputs.ddev-version }} - name: Configure ddev @@ -151,112 +56,17 @@ jobs: ddev config set orgs.ci.dd_url "https://app.datadoghq.com" ddev config set org ci - - name: Validate Agent integration version requirements - if: ${{ !cancelled() && inputs.agent-reqs }} - run: ddev validate agent-reqs $TARGET - - - name: Validate CI configuration - if: ${{ !cancelled() && inputs.ci }} - run: ddev validate ci - - - name: Validate default configuration files - if: ${{ !cancelled() && inputs.config }} - run: ddev validate config $TARGET - - - name: Validate dashboards - if: ${{ !cancelled() && inputs.dashboards }} - run: ddev validate dashboards $TARGET - - - name: Validate dependencies - if: ${{ !cancelled() && inputs.dep }} - run: ddev validate dep - - - name: Validate EULA files - if: ${{ !cancelled() && inputs.eula }} - run: ddev validate eula $TARGET - - - name: Validate usage of HTTP(S) utilities - if: ${{ !cancelled() && inputs.http }} - run: ddev validate http $TARGET - - - name: Validate proper modern importing of core packages - if: ${{ !cancelled() && inputs.imports }} - run: ddev validate imports $TARGET - - - name: Validate coding conventions - if: ${{ !cancelled() && inputs.integration-style }} - run: ddev validate integration-style $TARGET - - - name: Validate default JMX metric files - if: ${{ !cancelled() && inputs.jmx-metrics }} - run: ddev validate jmx-metrics $TARGET - - - name: Validate signatures of check classes - if: ${{ !cancelled() && inputs.legacy-signature }} - run: ddev validate legacy-signature $TARGET - - - name: Validate all shipped files have license headers - if: ${{ !cancelled() && inputs.license-headers }} - run: ddev validate license-headers $TARGET - - - name: Validate metric metadata - if: ${{ !cancelled() && inputs.metadata }} - run: ddev validate metadata $TARGET - - - name: Validate configuration models - if: ${{ !cancelled() && inputs.models }} - run: ddev validate models $TARGET - - - name: Validate openmetrics metric limit - if: ${{ !cancelled() && inputs.openmetrics }} - run: ddev validate openmetrics $TARGET - - - name: Validate Python project metadata - if: ${{ !cancelled() && inputs.package }} - run: ddev validate package $TARGET - - - name: Validate README files - if: ${{ !cancelled() && inputs.readmes }} - run: ddev validate readmes $TARGET - - - name: Validate saved views - if: ${{ !cancelled() && inputs.saved-views }} - run: ddev validate saved-views $TARGET - - - name: Validate service check metadata - if: ${{ !cancelled() && inputs.service-checks }} - run: ddev validate service-checks $TARGET - - - name: Validate spelling - if: ${{ !cancelled() && inputs.typos }} - run: ddev validate typos $TARGET - - - name: Validate labeler configuration - if: ${{ !cancelled() && inputs.labeler }} - run: ddev validate labeler - - - name: Validate target version - if: ${{ !cancelled() && inputs.version }} - run: ddev validate version $TARGET + - name: Get GitHub token for PR comments + if: github.event_name == 'pull_request' + uses: DataDog/dd-octo-sts-action@96a25462dbcb10ebf0bfd6e2ccc917d2ab235b9a # v1.0.4 + id: octo-sts + continue-on-error: true + with: + scope: DataDog/integrations-core + policy: self.validate.pull-request - # Every validation below here is sorted by increasing runtime rather than alphabetically - - name: Validate third-party license metadata - if: ${{ !cancelled() && inputs.licenses }} + - name: Run all validations env: DD_GITHUB_USER: "${{ github.actor }}" - DD_GITHUB_TOKEN: "${{ secrets.GITHUB_TOKEN }}" - run: ddev validate licenses - - - name: Validate the CODEOWNERS file - if: ${{ !cancelled() && inputs.codeowners }} - run: ddev validate codeowners - - - name: Comment PR on failure - if: ${{ failure() && github.event_name == 'pull_request' && github.event.pull_request.merged != true }} - uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8.0.0 - continue-on-error: true - with: - github-token: ${{ secrets.GITHUB_TOKEN }} - script: | - const { issue: { number: issue_number }, repo: { owner, repo } } = context; - github.rest.issues.createComment({ issue_number, owner, repo, body: "The `validations` job has failed; please review the `Files changed` tab for possible suggestions to resolve." }); + DD_GITHUB_TOKEN: "${{ steps.octo-sts.outputs.token || secrets.GITHUB_TOKEN }}" + run: ddev validate all "$TARGET" diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 97a79a51c81a5..6de5e5b9790eb 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -12,32 +12,14 @@ concurrency: group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.sha }} cancel-in-progress: true +permissions: + id-token: write + contents: read + jobs: run: name: Run Validations uses: ./.github/workflows/run-validations.yml with: repo: core - - # Validations - agent-reqs: true - ci: true - codeowners: true - config: true - dep: true - http: true - imports: true - integration-style: true - jmx-metrics: true - labeler: true - legacy-signature: true - license-headers: true - licenses: true - metadata: true - models: true - openmetrics: true - package: true - readmes: true - saved-views: true - version: true secrets: inherit diff --git a/ddev/changelog.d/23249.added b/ddev/changelog.d/23249.added new file mode 100644 index 0000000000000..ad6a7b266975f --- /dev/null +++ b/ddev/changelog.d/23249.added @@ -0,0 +1 @@ +Add parallel validation orchestrator with ddev validate all command \ No newline at end of file diff --git a/ddev/src/ddev/cli/application.py b/ddev/src/ddev/cli/application.py index a871658a1eec4..767a21b916f46 100644 --- a/ddev/src/ddev/cli/application.py +++ b/ddev/src/ddev/cli/application.py @@ -3,6 +3,7 @@ # Licensed under a 3-clause BSD style license (see LICENSE) from __future__ import annotations +import logging import os from functools import cached_property from typing import cast @@ -16,6 +17,23 @@ from ddev.utils.platform import Platform +class AppLoggingHandler(logging.Handler): + """Routes Python logging through the Application display methods.""" + + def __init__(self, app: Application): + super().__init__() + self._app = app + + def emit(self, record: logging.LogRecord) -> None: + msg = self.format(record) + if record.levelno >= logging.ERROR: + self._app.display_error(msg) + elif record.levelno >= logging.WARNING: + self._app.display_warning(msg) + else: + self._app.display_info(msg) + + class Application(Terminal): def __init__(self, exit_func, *args, **kwargs): super().__init__(*args, **kwargs) @@ -47,6 +65,14 @@ def data_dir(self) -> Path: return Path(os.getenv(ConfigEnvVars.DATA) or user_data_dir('ddev', appauthor=False)).expand() + @cached_property + def logger(self) -> logging.Logger: + logger = logging.getLogger("ddev.app") + if not any(isinstance(h, AppLoggingHandler) for h in logger.handlers): + logger.addHandler(AppLoggingHandler(self)) + logger.setLevel(logging.WARNING) + return logger + @property def github(self) -> GitHubManager: return self.__github diff --git a/ddev/src/ddev/cli/validate/__init__.py b/ddev/src/ddev/cli/validate/__init__.py index 5865a9ba66c7b..a711e7d149f35 100644 --- a/ddev/src/ddev/cli/validate/__init__.py +++ b/ddev/src/ddev/cli/validate/__init__.py @@ -4,7 +4,6 @@ import click from datadog_checks.dev.tooling.commands.validate.agent_reqs import agent_reqs from datadog_checks.dev.tooling.commands.validate.agent_signature import legacy_signature -from datadog_checks.dev.tooling.commands.validate.all_validations import all from datadog_checks.dev.tooling.commands.validate.codeowners import codeowners from datadog_checks.dev.tooling.commands.validate.config import config from datadog_checks.dev.tooling.commands.validate.dashboards import dashboards @@ -21,6 +20,7 @@ from datadog_checks.dev.tooling.commands.validate.service_checks import service_checks from datadog_checks.dev.tooling.commands.validate.typos import typos +from ddev.cli.validate.all import all from ddev.cli.validate.ci import ci from ddev.cli.validate.http import http from ddev.cli.validate.labeler import labeler diff --git a/ddev/src/ddev/cli/validate/all/__init__.py b/ddev/src/ddev/cli/validate/all/__init__.py new file mode 100644 index 0000000000000..216313ef57fe2 --- /dev/null +++ b/ddev/src/ddev/cli/validate/all/__init__.py @@ -0,0 +1,82 @@ +# (C) Datadog, Inc. 2026-present +# All rights reserved +# Licensed under a 3-clause BSD style license (see LICENSE) +from __future__ import annotations + +from typing import TYPE_CHECKING + +import click + +if TYPE_CHECKING: + from ddev.cli.application import Application + from ddev.cli.validate.all.orchestrator import ValidationConfig + + +def _load_validations(app: Application) -> dict[str, ValidationConfig]: + """Read the selected validations from `.ddev/config.toml`. + + If the `/validations` key is absent, all known validations are returned. + Unknown names are warned about and skipped. + """ + from ddev.cli.validate.all.orchestrator import VALIDATIONS + + selected: list[str] | None = app.repo.config.get('/validations', None) + if selected is None: + return VALIDATIONS + + result: dict[str, ValidationConfig] = {} + for name in selected: + if name in VALIDATIONS: + result[name] = VALIDATIONS[name] + else: + app.display_warning(f"Unknown validation in .ddev/config.toml: {name!r}") + return result + + +@click.command(short_help="Run all validations in parallel") +@click.argument("target", required=False) +@click.option("--fix", is_flag=True, help="Attempt to auto-fix issues (passes --sync/--fix to each validation).") +@click.option("--grace-period", type=float, default=5, help="Seconds to wait for stragglers after first completion.") +@click.option("--max-timeout", type=float, default=600, help="Maximum total seconds before the orchestrator stops.") +@click.option( + "--subprocess-timeout", type=float, default=580, help="Timeout in seconds for each validation subprocess." +) +@click.pass_obj +def all( + app: Application, + target: str | None, + fix: bool, + grace_period: float, + max_timeout: float, + subprocess_timeout: float, +) -> None: + """Run all validations in parallel. + + 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.orchestrator import ValidationOrchestrator + + selected = _load_validations(app) + if not selected: + msg = ( + "No validations are configured to run for this repository.\n" + "Add entries to the `validations` list in `.ddev/config.toml` or remove the validation workflow." + ) + app.display_error(msg) + write_step_summary(f"## Validation Report\n\n> **Error:** {msg}") + app.abort() + + pr_number = get_pr_number(app) + orchestrator = ValidationOrchestrator( + app=app, + target=target, + validations=list(selected), + fix=fix, + pr_number=pr_number, + grace_period=grace_period, + max_timeout=max_timeout, + subprocess_timeout=subprocess_timeout, + ) + orchestrator.run() diff --git a/ddev/src/ddev/cli/validate/all/github.py b/ddev/src/ddev/cli/validate/all/github.py new file mode 100644 index 0000000000000..b025bf84feeba --- /dev/null +++ b/ddev/src/ddev/cli/validate/all/github.py @@ -0,0 +1,103 @@ +# (C) Datadog, Inc. 2026-present +# All rights reserved +# Licensed under a 3-clause BSD style license (see LICENSE) +from __future__ import annotations + +import contextlib +import json +import os +from typing import TYPE_CHECKING + +from ddev.utils.fs import Path + +if TYPE_CHECKING: + from ddev.cli.application import Application + from ddev.cli.validate.all.orchestrator import ValidationResult + +COMMENT_HEADING = "## Validation Report" + + +def parse_pr_number_from_event(event_path: str) -> int | None: + """Extract the PR number from the GitHub Actions event JSON file.""" + try: + event = json.loads(Path(event_path).read_text()) + except (json.JSONDecodeError, OSError): + return None + + pr = event.get("pull_request") + if isinstance(pr, dict): + number = pr.get("number") + if isinstance(number, int): + return number + + return None + + +def parse_pr_number_from_ref(ref: str) -> int | None: + """Extract the PR number from a GITHUB_REF like refs/pull/123/merge.""" + if ref.startswith("refs/pull/") and ref.endswith("/merge"): + with contextlib.suppress(IndexError, ValueError): + return int(ref.split("/")[2]) + return None + + +def get_pr_number(app: Application) -> int | None: + if os.environ.get("GITHUB_EVENT_NAME") != "pull_request": + return None + + if event_path := os.environ.get("GITHUB_EVENT_PATH"): + if pr_number := parse_pr_number_from_event(event_path): + return pr_number + app.display_warning(f"Failed to extract PR number from event file: {event_path}") + return None + + ref = os.environ.get("GITHUB_REF", "") + if pr_number := parse_pr_number_from_ref(ref): + return pr_number + + app.display_warning("Running in pull_request context but could not determine PR number") + return None + + +def get_workflow_run_url() -> str | None: + server = os.environ.get("GITHUB_SERVER_URL") + repo = os.environ.get("GITHUB_REPOSITORY") + run_id = os.environ.get("GITHUB_RUN_ID") + if server and repo and run_id: + return f"{server}/{repo}/actions/runs/{run_id}" + return None + + +def write_step_summary(content: str) -> None: + if summary_path := os.environ.get("GITHUB_STEP_SUMMARY"): + with contextlib.suppress(OSError): + with open(summary_path, "a", encoding="utf-8") as f: + f.write(content + "\n") + + +def format_pr_comment( + results: dict[str, ValidationResult], + target: str | None, + *, + error: str | None = None, + warning: str | None = None, +) -> str: + failures = {n for n, r in results.items() if not r.success} + + parts = [f"{COMMENT_HEADING}\n"] + if error: + parts.append(f"> **Error:** {error}\n") + if warning: + parts.append(f"> **Warning:** {warning}\n") + + parts.extend(("| Validation | Status |", "|---|---|")) + for name in sorted(results): + status = "❌" if name in failures else "✅" + parts.append(f"| `{name}` | {status} |") + + if failures: + fix_target = f" {target}" if target else "" + fix_all_cmd = f"ddev validate all{fix_target} --fix" + parts.append(f"\nRun `{fix_all_cmd}` to attempt to auto-fix supported validations.") + + return "\n".join(parts) diff --git a/ddev/src/ddev/cli/validate/all/orchestrator.py b/ddev/src/ddev/cli/validate/all/orchestrator.py new file mode 100644 index 0000000000000..20c570abb350f --- /dev/null +++ b/ddev/src/ddev/cli/validate/all/orchestrator.py @@ -0,0 +1,254 @@ +# (C) Datadog, Inc. 2026-present +# All rights reserved +# Licensed under a 3-clause BSD style license (see LICENSE) +from __future__ import annotations + +import logging +import os +import subprocess +import sys +import threading +import time +from concurrent.futures import ThreadPoolExecutor +from dataclasses import dataclass, field +from typing import TYPE_CHECKING + +from ddev.cli.validate.all.github import ( + COMMENT_HEADING, + format_pr_comment, + get_workflow_run_url, + write_step_summary, +) +from ddev.event_bus.orchestrator import BaseMessage, EventBusOrchestrator, SyncProcessor + +if TYPE_CHECKING: + from ddev.cli.application import Application + + +@dataclass(frozen=True) +class ValidationConfig: + repo_wide: bool = False + fix_flag: str | None = None + + +# This is a subset of the available validations. Some validations still live +# in datadog-checks-dev and are not yet migrated to ddev. Once all validations +# are migrated, we can improve the command structure so each validation +# auto-registers itself instead of maintaining this manually. +VALIDATIONS: dict[str, ValidationConfig] = { + "agent-reqs": ValidationConfig(), + "ci": ValidationConfig(repo_wide=True, fix_flag="--sync"), + "codeowners": ValidationConfig(repo_wide=True), + "config": ValidationConfig(fix_flag="--sync"), + "dep": ValidationConfig(repo_wide=True), + "http": ValidationConfig(), + "imports": ValidationConfig(), + "integration-style": ValidationConfig(), + "jmx-metrics": ValidationConfig(), + "labeler": ValidationConfig(repo_wide=True, fix_flag="--sync"), + "legacy-signature": ValidationConfig(), + "license-headers": ValidationConfig(fix_flag="--fix"), + "licenses": ValidationConfig(repo_wide=True, fix_flag="--sync"), + "metadata": ValidationConfig(fix_flag="--sync"), + "models": ValidationConfig(fix_flag="--sync"), + "openmetrics": ValidationConfig(), + "package": ValidationConfig(), + "readmes": ValidationConfig(), + "saved-views": ValidationConfig(), + "version": ValidationConfig(), +} + + +SUBPROCESS_TIMEOUT = 580 + + +@dataclass +class ValidationMessage(BaseMessage): + args: list[str] = field(default_factory=list) + + +@dataclass +class ValidationResult: + name: str + success: bool + stdout: str + stderr: str + duration: float + + +class ValidationProcessor(SyncProcessor[ValidationMessage]): + def __init__(self, app: Application, results: dict[str, ValidationResult], subprocess_timeout: float): + super().__init__("validation-processor") + self._app = app + self._results = results + self._subprocess_timeout = subprocess_timeout + self._lock = threading.Lock() + + def process_message(self, message: ValidationMessage) -> None: + start = time.monotonic() + try: + proc = subprocess.run( + [sys.executable, "-m", "ddev", "validate", message.id, *message.args], + capture_output=True, + text=True, + timeout=self._subprocess_timeout, + ) + result = ValidationResult( + name=message.id, + success=proc.returncode == 0, + stdout=proc.stdout, + stderr=proc.stderr, + duration=time.monotonic() - start, + ) + except subprocess.TimeoutExpired: + result = ValidationResult( + name=message.id, + success=False, + stdout="", + stderr=f"Validation timed out after {self._subprocess_timeout}s", + duration=time.monotonic() - start, + ) + + with self._lock: + self._results[message.id] = result + + if result.success: + self._app.display_success(f" ok {result.name} ({result.duration:.1f}s)") + else: + self._app.display_error(f" FAIL {result.name} ({result.duration:.1f}s)") + + +class ValidationOrchestrator(EventBusOrchestrator): + def __init__( + self, + app: Application, + target: str | None, + validations: list[str] | None = None, + fix: bool = False, + pr_number: int | None = None, + grace_period: float = 5, + max_timeout: float = 600, + subprocess_timeout: float = SUBPROCESS_TIMEOUT, + ): + validations = validations if validations is not None else list(VALIDATIONS) + super().__init__( + logger=app.logger, + max_timeout=max_timeout, + grace_period=grace_period, + executor=ThreadPoolExecutor(max_workers=len(validations)), + ) + self._app = app + self._validations = validations + self._target = target + self._fix = fix + self._pr_number = pr_number + self._results: dict[str, ValidationResult] = {} + + self.register_processor( + ValidationProcessor(app, self._results, subprocess_timeout), + [ValidationMessage], + ) + + async def on_initialize(self) -> None: + for name in self._validations: + config = VALIDATIONS.get(name, ValidationConfig()) + args: list[str] = [] + if not config.repo_wide and self._target: + args.append(self._target) + if self._fix and config.fix_flag: + args.append(config.fix_flag) + self.submit_message(ValidationMessage(id=name, args=args)) + + async def on_message_received(self, message: BaseMessage) -> None: + self._app.display_info(f"Starting: {message.id}") + + async def on_finalize(self, exception: Exception | None) -> None: + if exception is not None: + self._app.display_error(f"Error running validations: {exception}") + + self._post_pr_comment(exception) + self._print_console_output() + + def _build_report_body(self, exception: Exception | None) -> str: + error_msg = f"Error running validations: {exception}" if exception else None + + if self._pr_number is None and os.environ.get("GITHUB_EVENT_NAME") == "pull_request": + extra_warning = "Running in pull_request context but could not determine PR number to post a comment." + else: + extra_warning = None + + body = format_pr_comment(self._results, self._target, error=error_msg, warning=extra_warning) + if run_url := get_workflow_run_url(): + body += f"\n\n[View full run]({run_url})" + return body + + 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 _post_pr_comment(self, exception: Exception | None) -> None: + body = self._build_report_body(exception) + write_step_summary(body) + + self._app.logger.debug("PR number: %s", self._pr_number) + self._app.logger.debug("GitHub token configured: %s", bool(self._app.config.github.token)) + + if self._pr_number is None: + self._app.logger.debug("No PR number — skipping PR comment.") + return + if not self._app.config.github.token: + self._app.logger.debug("No GitHub token — skipping PR comment.") + return + + httpx_logger = logging.getLogger("httpx") + 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, body) + self._app.logger.debug("Comment posted successfully.") + except Exception as exc: + self._app.display_warning(f"Failed to post PR comment: {exc}") + write_step_summary(f"\n> Failed to post PR comment: {exc}") + finally: + httpx_logger.setLevel(previous_level) + + def _print_console_output(self) -> None: + from rich.rule import Rule + + failures = {name: r for name, r in self._results.items() if not r.success} + passed = len(self._results) - len(failures) + incomplete = len(self._validations) - len(self._results) + + self._app.display_info("") + + if failures: + for name, result in sorted(failures.items()): + self._app.console.print(Rule(title=name, style="red")) + output = "\n".join(filter(None, [result.stdout, result.stderr])) + if output: + self._app.console.print(output.rstrip()) + config = VALIDATIONS.get(name, ValidationConfig()) + if config.fix_flag: + fix_cmd = f"ddev validate {name} {config.fix_flag}" + self._app.display_info(f"Fix: {fix_cmd}") + self._app.console.print() + + if incomplete: + self._app.display_warning(f"{incomplete} validation(s) did not complete") + if failures or incomplete: + self._app.display_error(f"{len(failures)} failed, {passed} passed") + fix_all_cmd = "ddev validate all --fix" + if self._target: + fix_all_cmd = f"ddev validate all {self._target} --fix" + self._app.display_info(f"\nRun `{fix_all_cmd}` to attempt to auto-fix supported validations.") + self._app.abort() + else: + self._app.display_success(f"All {passed} validations passed") diff --git a/ddev/src/ddev/utils/github.py b/ddev/src/ddev/utils/github.py index 4d1eb2876c058..2e31fdc8a59e9 100644 --- a/ddev/src/ddev/utils/github.py +++ b/ddev/src/ddev/utils/github.py @@ -75,6 +75,9 @@ class GitHubManager: # https://docs.github.com/en/rest/commits/commits?apiVersion=2022-11-28#list-commits-on-a-repository COMMIT_API = 'https://api.github.com/repos/{repo_id}/commits/{sha}' + # https://docs.github.com/en/rest/issues/comments?apiVersion=2022-11-28#create-an-issue-comment + ISSUE_COMMENTS_API = 'https://api.github.com/repos/{repo_id}/issues/{issue_number}/comments' + def __init__(self, repo: Repository, *, user: str, token: str, status: BorrowedStatus): self.__repo = repo self.__auth = (user, token) @@ -147,6 +150,24 @@ def get_changed_files_by_commit_sha(self, sha: str) -> list[str] | None: return None return [file_data['filename'] for file_data in response.json().get('files', [])] + def get_pull_request_comments(self, pr_number: int) -> list[dict]: + response = self.__api_get( + self.ISSUE_COMMENTS_API.format(repo_id=self.repo_id, issue_number=pr_number), + ) + return response.json() + + def delete_comment(self, comment_id: int) -> None: + self.__api_call( + 'delete', + f'https://api.github.com/repos/{self.repo_id}/issues/comments/{comment_id}', + ) + + def post_pull_request_comment(self, pr_number: int, body: str) -> None: + self.__api_post( + self.ISSUE_COMMENTS_API.format(repo_id=self.repo_id, issue_number=pr_number), + content=json.dumps({'body': body}), + ) + def create_label(self, name, color): self.__api_post( self.LABELS_API.format(repo_id=self.repo_id), content=json.dumps({'name': name, 'color': color}) diff --git a/ddev/tests/cli/validate/all/__init__.py b/ddev/tests/cli/validate/all/__init__.py new file mode 100644 index 0000000000000..75c6647cb9233 --- /dev/null +++ b/ddev/tests/cli/validate/all/__init__.py @@ -0,0 +1,3 @@ +# (C) Datadog, Inc. 2026-present +# All rights reserved +# Licensed under a 3-clause BSD style license (see LICENSE) diff --git a/ddev/tests/cli/validate/all/conftest.py b/ddev/tests/cli/validate/all/conftest.py new file mode 100644 index 0000000000000..8cec00b4981f7 --- /dev/null +++ b/ddev/tests/cli/validate/all/conftest.py @@ -0,0 +1,35 @@ +# (C) Datadog, Inc. 2026-present +# All rights reserved +# Licensed under a 3-clause BSD style license (see LICENSE) +from __future__ import annotations + +import subprocess +from unittest.mock import MagicMock + +import pytest + + +@pytest.fixture(autouse=True) +def _clean_github_env(monkeypatch): + """Provide a consistent GitHub Actions environment for all tests.""" + for key in ("GITHUB_EVENT_NAME", "GITHUB_EVENT_PATH", "GITHUB_REF", "GITHUB_STEP_SUMMARY"): + monkeypatch.delenv(key, raising=False) + monkeypatch.setenv("GITHUB_SERVER_URL", "https://github.com") + monkeypatch.setenv("GITHUB_REPOSITORY", "DataDog/integrations-core") + monkeypatch.setenv("GITHUB_RUN_ID", "12345") + + +def completed_process(returncode=0, stdout="", stderr=""): + proc = MagicMock(spec=subprocess.CompletedProcess) + proc.returncode = returncode + proc.stdout = stdout + proc.stderr = stderr + return proc + + +@pytest.fixture +def mock_app(): + """Mock Application that records display calls and raises SystemExit on abort.""" + app = MagicMock() + app.abort.side_effect = SystemExit(1) + return app diff --git a/ddev/tests/cli/validate/all/test_command.py b/ddev/tests/cli/validate/all/test_command.py new file mode 100644 index 0000000000000..847de9d1eef95 --- /dev/null +++ b/ddev/tests/cli/validate/all/test_command.py @@ -0,0 +1,110 @@ +# (C) Datadog, Inc. 2026-present +# All rights reserved +# Licensed under a 3-clause BSD style license (see LICENSE) +from __future__ import annotations + +from unittest.mock import patch + +from ddev.cli.validate.all.orchestrator import VALIDATIONS + +from .conftest import completed_process + +NO_VALIDATIONS_ERROR = "No validations are configured to run" + +ALL_NAMES = list(VALIDATIONS) +REPO_WIDE_NAMES = {name for name, cfg in VALIDATIONS.items() if cfg.repo_wide} +FAST_ORCHESTRATOR_OPTS = ("--grace-period", "0", "--max-timeout", "10", "--subprocess-timeout", "10") + + +def test_all_command_passes_when_all_validations_succeed(ddev): + invoked: list[str] = [] + + def fake_run(cmd, **kwargs): + invoked.append(cmd[-1]) + return completed_process(returncode=0, stdout="ok") + + with patch("subprocess.run", side_effect=fake_run): + result = ddev("validate", "all", *FAST_ORCHESTRATOR_OPTS) + + assert result.exit_code == 0, result.output + assert set(invoked) == set(ALL_NAMES) + + +def test_all_command_aborts_on_failure_with_details(ddev): + def fake_run(cmd, **kwargs): + if "config" in cmd: + return completed_process(returncode=1, stdout="invalid config found") + return completed_process(returncode=0, stdout="ok") + + with patch("subprocess.run", side_effect=fake_run): + result = ddev("validate", "all", *FAST_ORCHESTRATOR_OPTS) + + assert result.exit_code != 0 + assert "1 failed" in result.output + assert "── config" in result.output + assert "invalid config found" in result.output + assert "Fix: ddev validate config --sync" in result.output + assert "Run `ddev validate all --fix` to attempt to auto-fix supported validations." in result.output + + +def test_all_command_shows_both_stdout_and_stderr_on_failure(ddev): + def fake_run(cmd, **kwargs): + if "config" in cmd: + return completed_process(returncode=1, stdout="stdout output", stderr="stderr output") + return completed_process(returncode=0, stdout="ok") + + with patch("subprocess.run", side_effect=fake_run): + result = ddev("validate", "all", *FAST_ORCHESTRATOR_OPTS) + + assert result.exit_code != 0 + assert "stdout output" in result.output + assert "stderr output" in result.output + + +def test_all_command_passes_target_to_per_integration_validations(ddev): + captured: dict[str, list[str]] = {} + + def fake_run(cmd, **kwargs): + captured[cmd[-1] if cmd[-1] != "changed" else cmd[-2]] = list(cmd) + return completed_process(returncode=0, stdout="ok") + + with patch("subprocess.run", side_effect=fake_run): + result = ddev("validate", "all", "changed", *FAST_ORCHESTRATOR_OPTS) + + assert result.exit_code == 0, result.output + for name in ALL_NAMES: + assert name in captured + if name not in REPO_WIDE_NAMES: + assert "changed" in captured[name] + else: + assert "changed" not in captured[name] + + +def test_all_command_fix_passes_correct_flags(ddev): + captured: dict[str, list[str]] = {} + + def fake_run(cmd, **kwargs): + name = cmd[4] + captured[name] = list(cmd) + return completed_process(returncode=0, stdout="ok") + + with patch("subprocess.run", side_effect=fake_run): + result = ddev("validate", "all", "--fix", *FAST_ORCHESTRATOR_OPTS) + + assert result.exit_code == 0, result.output + for name, config in VALIDATIONS.items(): + assert name in captured + if config.fix_flag: + assert config.fix_flag in captured[name], f"{name} should have {config.fix_flag}" + else: + assert "--fix" not in captured[name] and "--sync" not in captured[name], ( + f"{name} should not have a fix flag" + ) + + +def test_all_command_aborts_when_no_validations_configured(ddev): + with patch("ddev.cli.validate.all._load_validations", return_value={}): + result = ddev("validate", "all", *FAST_ORCHESTRATOR_OPTS) + + assert result.exit_code != 0 + assert NO_VALIDATIONS_ERROR in result.output diff --git a/ddev/tests/cli/validate/all/test_github.py b/ddev/tests/cli/validate/all/test_github.py new file mode 100644 index 0000000000000..b3d55fee64ae4 --- /dev/null +++ b/ddev/tests/cli/validate/all/test_github.py @@ -0,0 +1,237 @@ +# (C) Datadog, Inc. 2026-present +# All rights reserved +# Licensed under a 3-clause BSD style license (see LICENSE) +from __future__ import annotations + +import pytest + +from ddev.cli.validate.all.github import ( + format_pr_comment, + get_pr_number, + get_workflow_run_url, + parse_pr_number_from_event, + parse_pr_number_from_ref, + write_step_summary, +) +from ddev.cli.validate.all.orchestrator import ValidationResult + +# --- parse_pr_number_from_event --- + + +@pytest.mark.parametrize( + "content, expected", + [ + pytest.param('{"pull_request": {"number": 42}}', 42, id="happy-path"), + pytest.param("{bad json}", None, id="malformed-json"), + pytest.param("{}", None, id="missing-pr-key"), + pytest.param('{"pull_request": "bad"}', None, id="pr-not-dict"), + pytest.param('{"pull_request": {}}', None, id="missing-number"), + pytest.param('{"pull_request": {"number": "x"}}', None, id="number-not-int"), + ], +) +def test_parse_pr_number_from_event(tmp_path, content, expected): + event_file = tmp_path / "event.json" + event_file.write_text(content) + assert parse_pr_number_from_event(str(event_file)) == expected + + +def test_parse_pr_number_from_event_file_not_found(): + assert parse_pr_number_from_event("/nonexistent/event.json") is None + + +# --- parse_pr_number_from_ref --- + + +@pytest.mark.parametrize( + "ref, expected", + [ + pytest.param("refs/pull/99/merge", 99, id="valid-ref"), + pytest.param("refs/heads/main", None, id="branch-ref"), + pytest.param("refs/pull/notanumber/merge", None, id="non-numeric-ref"), + pytest.param("refs/pull//merge", None, id="empty-number"), + pytest.param("", None, id="empty-string"), + ], +) +def test_parse_pr_number_from_ref(ref, expected): + assert parse_pr_number_from_ref(ref) == expected + + +# --- get_pr_number --- + + +@pytest.mark.parametrize( + "event_name", + [ + pytest.param("push", id="push-event"), + pytest.param("merge_group", id="merge-group-event"), + ], +) +def test_get_pr_number_non_pr_events(mock_app, monkeypatch, event_name): + monkeypatch.setenv("GITHUB_EVENT_NAME", event_name) + + assert get_pr_number(mock_app) is None + mock_app.display_warning.assert_not_called() + + +def test_get_pr_number_no_event_name(mock_app): + assert get_pr_number(mock_app) is None + mock_app.display_warning.assert_not_called() + + +def test_get_pr_number_from_event_file(mock_app, tmp_path, monkeypatch): + monkeypatch.setenv("GITHUB_EVENT_NAME", "pull_request") + event_file = tmp_path / "event.json" + event_file.write_text('{"pull_request": {"number": 42}}') + monkeypatch.setenv("GITHUB_EVENT_PATH", str(event_file)) + + assert get_pr_number(mock_app) == 42 + + +def test_get_pr_number_from_ref_fallback(mock_app, monkeypatch): + monkeypatch.setenv("GITHUB_EVENT_NAME", "pull_request") + monkeypatch.setenv("GITHUB_REF", "refs/pull/99/merge") + + assert get_pr_number(mock_app) == 99 + + +def test_get_pr_number_warns_on_event_file_failure(mock_app, tmp_path, monkeypatch): + monkeypatch.setenv("GITHUB_EVENT_NAME", "pull_request") + monkeypatch.setenv("GITHUB_EVENT_PATH", str(tmp_path / "missing.json")) + + assert get_pr_number(mock_app) is None + mock_app.display_warning.assert_called() + + +def test_get_pr_number_warns_when_no_source_available(mock_app, monkeypatch): + monkeypatch.setenv("GITHUB_EVENT_NAME", "pull_request") + + assert get_pr_number(mock_app) is None + mock_app.display_warning.assert_called() + + +# --- format_pr_comment --- + + +def test_format_pr_comment_all_passed(helpers): + results = { + "config": ValidationResult(name="config", success=True, stdout="ok", stderr="", duration=1.0), + "ci": ValidationResult(name="ci", success=True, stdout="ok", stderr="", duration=2.0), + } + expected = helpers.dedent(""" + ## Validation Report + + | Validation | Status | + |---|---| + | `ci` | ✅ | + | `config` | ✅ |""") + assert format_pr_comment(results, target="changed") == expected + + +def test_format_pr_comment_one_failure_with_target(helpers): + results = { + "config": ValidationResult(name="config", success=False, stdout="error output", stderr="", duration=1.0), + "ci": ValidationResult(name="ci", success=True, stdout="ok", stderr="", duration=2.0), + } + expected = helpers.dedent(""" + ## Validation Report + + | Validation | Status | + |---|---| + | `ci` | ✅ | + | `config` | ❌ | + + Run `ddev validate all changed --fix` to attempt to auto-fix supported validations.""") + assert format_pr_comment(results, target="changed") == expected + + +def test_format_pr_comment_no_target(helpers): + results = { + "config": ValidationResult(name="config", success=False, stdout="fail", stderr="", duration=1.0), + } + expected = helpers.dedent(""" + ## Validation Report + + | Validation | Status | + |---|---| + | `config` | ❌ | + + Run `ddev validate all --fix` to attempt to auto-fix supported validations.""") + assert format_pr_comment(results, target=None) == expected + + +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, target=None) + assert "ddev validate all" not in comment + + +def test_format_pr_comment_with_error_and_warning(helpers): + results = { + "config": ValidationResult(name="config", success=False, stdout="bad", stderr="", duration=1.0), + } + expected = helpers.dedent(""" + ## Validation Report + + > **Error:** Error running validations: boom + + > **Warning:** Could not determine PR number + + | Validation | Status | + |---|---| + | `config` | ❌ | + + Run `ddev validate all --fix` to attempt to auto-fix supported validations.""") + assert ( + format_pr_comment( + results, target=None, error="Error running validations: boom", warning="Could not determine PR number" + ) + == expected + ) + + +def test_format_pr_comment_does_not_include_output(): + results = { + "config": ValidationResult(name="config", success=False, stdout="secret error output", stderr="", duration=1.0), + } + comment = format_pr_comment(results, target=None) + assert "secret error output" not in comment + + +# --- get_workflow_run_url --- + + +def test_get_workflow_run_url_returns_url(): + assert get_workflow_run_url() == "https://github.com/DataDog/integrations-core/actions/runs/12345" + + +def test_get_workflow_run_url_returns_none_when_env_missing(monkeypatch): + monkeypatch.delenv("GITHUB_RUN_ID") + assert get_workflow_run_url() is None + + +# --- write_step_summary --- + + +def test_write_step_summary_writes_to_file(tmp_path, monkeypatch): + summary_file = tmp_path / "summary.md" + monkeypatch.setenv("GITHUB_STEP_SUMMARY", str(summary_file)) + + write_step_summary("## Report\nAll good") + assert summary_file.read_text() == "## Report\nAll good\n" + + +def test_write_step_summary_appends(tmp_path, monkeypatch): + summary_file = tmp_path / "summary.md" + summary_file.write_text("existing\n") + monkeypatch.setenv("GITHUB_STEP_SUMMARY", str(summary_file)) + + write_step_summary("new content") + assert "existing\n" in summary_file.read_text() + assert "new content\n" in summary_file.read_text() + + +def test_write_step_summary_noop_without_env(monkeypatch): + monkeypatch.delenv("GITHUB_STEP_SUMMARY", raising=False) + write_step_summary("should not error") diff --git a/ddev/tests/cli/validate/all/test_orchestrator.py b/ddev/tests/cli/validate/all/test_orchestrator.py new file mode 100644 index 0000000000000..59c0366412b08 --- /dev/null +++ b/ddev/tests/cli/validate/all/test_orchestrator.py @@ -0,0 +1,341 @@ +# (C) Datadog, Inc. 2026-present +# All rights reserved +# Licensed under a 3-clause BSD style license (see LICENSE) +from __future__ import annotations + +import asyncio +import subprocess +from unittest.mock import patch + +import pytest + +from ddev.cli.validate.all import _load_validations +from ddev.cli.validate.all.orchestrator import ( + VALIDATIONS, + ValidationMessage, + ValidationOrchestrator, + ValidationProcessor, + ValidationResult, +) +from ddev.event_bus.orchestrator import BaseMessage + +from .conftest import completed_process + +ALL_NAMES = list(VALIDATIONS) +REPO_WIDE_NAMES = {name for name, cfg in VALIDATIONS.items() if cfg.repo_wide} + + +# --- ValidationProcessor --- + + +@pytest.mark.parametrize( + "returncode, stdout, stderr, expected_success", + [ + pytest.param(0, "all good", "", True, id="success"), + pytest.param(1, "", "error details", False, id="failure"), + ], +) +def test_processor_captures_result(mock_app, returncode, stdout, stderr, expected_success): + results: dict[str, ValidationResult] = {} + processor = ValidationProcessor(mock_app, results, subprocess_timeout=10) + with patch("subprocess.run", return_value=completed_process(returncode=returncode, stdout=stdout, stderr=stderr)): + processor.process_message(ValidationMessage(id="config", args=[])) + + assert results["config"].success is expected_success + assert results["config"].stdout == stdout + assert results["config"].stderr == stderr + + +def test_processor_passes_args_in_subprocess_command(mock_app): + results: dict[str, ValidationResult] = {} + processor = ValidationProcessor(mock_app, results, subprocess_timeout=10) + captured: list[list[str]] = [] + + def fake_run(cmd, **kwargs): + captured.append(list(cmd)) + return completed_process() + + with patch("subprocess.run", side_effect=fake_run): + processor.process_message(ValidationMessage(id="config", args=["changed"])) + + assert len(captured) == 1 + assert "config" in captured[0] + assert "changed" in captured[0] + + +@pytest.mark.parametrize( + "returncode, display_method, expected_text", + [ + pytest.param(0, "display_success", "ok config", id="pass"), + pytest.param(1, "display_error", "FAIL config", id="fail"), + ], +) +def test_processor_displays_result(mock_app, returncode, display_method, expected_text): + results: dict[str, ValidationResult] = {} + processor = ValidationProcessor(mock_app, results, subprocess_timeout=10) + + with patch("subprocess.run", return_value=completed_process(returncode=returncode, stdout="ok", stderr="bad")): + processor.process_message(ValidationMessage(id="config", args=[])) + + display_mock = getattr(mock_app, display_method) + display_mock.assert_called_once() + assert expected_text in str(display_mock.call_args) + + +def test_processor_handles_subprocess_timeout(mock_app): + results: dict[str, ValidationResult] = {} + processor = ValidationProcessor(mock_app, results, subprocess_timeout=10) + + with patch("subprocess.run", side_effect=subprocess.TimeoutExpired(cmd=["ddev"], timeout=580)): + processor.process_message(ValidationMessage(id="config", args=[])) + + assert results["config"].success is False + assert "timed out" in results["config"].stderr + + +# --- ValidationOrchestrator.on_initialize --- + + +class CapturingOrchestrator(ValidationOrchestrator): + """Orchestrator that captures submitted messages instead of dispatching them.""" + + def __init__(self, **kwargs): + super().__init__(**kwargs) + self.submitted: list[ValidationMessage] = [] + + def submit_message(self, message: BaseMessage) -> None: + assert isinstance(message, ValidationMessage) + self.submitted.append(message) + + +@pytest.mark.parametrize( + "validation, target, expect_args", + [ + ("config", "changed", ["changed"]), + ("metadata", "changed", ["changed"]), + *[pytest.param(name, "changed", [], id=f"repo-wide-{name}") for name in sorted(REPO_WIDE_NAMES)], + ("config", None, []), + ("ci", None, []), + ], +) +def test_on_initialize_message_args(mock_app, validation: str, target: str | None, expect_args: list[str]): + orch = CapturingOrchestrator(app=mock_app, validations=[validation], target=target) + asyncio.run(orch.on_initialize()) + + assert len(orch.submitted) == 1 + assert orch.submitted[0].id == validation + assert orch.submitted[0].args == expect_args + + +def test_on_initialize_submits_one_message_per_validation(mock_app): + orch = CapturingOrchestrator(app=mock_app, validations=ALL_NAMES, target=None) + asyncio.run(orch.on_initialize()) + + assert len(orch.submitted) == len(ALL_NAMES) + assert {m.id for m in orch.submitted} == set(ALL_NAMES) + + +# --- ValidationOrchestrator exit code logic --- + + +def test_all_pass_abort_not_called(mock_app): + with patch("subprocess.run", return_value=completed_process(returncode=0)): + orch = ValidationOrchestrator(app=mock_app, validations=["config", "ci"], target=None, grace_period=0) + orch.run() + + mock_app.abort.assert_not_called() + + +def test_any_failure_calls_abort(mock_app): + def fake_run(cmd, **kwargs): + if "config" in cmd: + return completed_process(returncode=1, stderr="bad config") + return completed_process(returncode=0) + + with patch("subprocess.run", side_effect=fake_run): + orch = ValidationOrchestrator(app=mock_app, validations=["config", "ci"], target=None, grace_period=0) + with pytest.raises(SystemExit): + orch.run() + + mock_app.abort.assert_called_once() + + +def test_on_finalize_warns_on_incomplete_validations(mock_app): + orch = ValidationOrchestrator(app=mock_app, validations=["config", "ci", "metadata"], target=None) + orch._results = { + "config": ValidationResult(name="config", success=True, stdout="ok", stderr="", duration=1.0), + } + with pytest.raises(SystemExit): + 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("2 validation(s) did not complete" in w for w in warning_args) + + +def test_on_finalize_logs_exception(mock_app): + orch = ValidationOrchestrator(app=mock_app, validations=["config"], target=None) + orch._results = { + "config": ValidationResult(name="config", success=True, stdout="ok", stderr="", duration=1.0), + } + asyncio.run(orch.on_finalize(exception=RuntimeError("boom"))) + error_args = [str(c) for c in mock_app.display_error.call_args_list] + assert any("boom" in e for e in error_args) + + +# --- on_finalize PR comment + step summary --- + + +def test_on_finalize_writes_step_summary(mock_app, tmp_path, monkeypatch): + summary_file = tmp_path / "summary.md" + monkeypatch.setenv("GITHUB_STEP_SUMMARY", str(summary_file)) + + mock_app.config.github.token = "" + orch = ValidationOrchestrator(app=mock_app, validations=["config"], target=None) + orch._results = { + "config": ValidationResult(name="config", success=True, stdout="ok", stderr="", duration=1.0), + } + asyncio.run(orch.on_finalize(exception=None)) + + assert summary_file.exists() + assert "Validation Report" in summary_file.read_text() + + +def test_on_finalize_posts_pr_comment_on_failure(mock_app): + mock_app.config.github.token = "fake-token" + + 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.post_pull_request_comment.assert_called_once() + body = mock_app.github.post_pull_request_comment.call_args[0][1] + assert "Validation Report" in body + assert "| Validation | Status |" in body + assert "| `config` | ❌ |" in body + assert "[View full run](https://github.com/DataDog/integrations-core/actions/runs/12345)" in body + + +def test_on_finalize_posts_pr_comment_on_success(mock_app): + mock_app.config.github.token = "fake-token" + + orch = ValidationOrchestrator(app=mock_app, validations=["config", "ci"], target=None, pr_number=42) + orch._results = { + "config": ValidationResult(name="config", success=True, stdout="ok", stderr="", duration=1.0), + "ci": ValidationResult(name="ci", success=True, stdout="ok", stderr="", duration=2.0), + } + asyncio.run(orch.on_finalize(exception=None)) + + mock_app.github.post_pull_request_comment.assert_called_once() + body = mock_app.github.post_pull_request_comment.call_args[0][1] + assert "| Validation | Status |" in body + assert "| `ci` | ✅ |" in body + assert "| `config` | ✅ |" in body + + +def test_on_finalize_deletes_previous_validation_comments(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"}, + {"id": 200, "body": "unrelated comment"}, + ] + + 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() + + +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)) + monkeypatch.setenv("GITHUB_EVENT_NAME", "pull_request") + + mock_app.config.github.token = "" + orch = ValidationOrchestrator(app=mock_app, validations=["config"], target=None, pr_number=None) + orch._results = { + "config": ValidationResult(name="config", success=True, stdout="ok", stderr="", duration=1.0), + } + asyncio.run(orch.on_finalize(exception=None)) + + content = summary_file.read_text() + assert "could not determine PR number" in content + + +def test_on_finalize_handles_post_failure(mock_app): + mock_app.config.github.token = "fake-token" + mock_app.github.post_pull_request_comment.side_effect = RuntimeError("network error") + + 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.display_warning.assert_called() + warning_args = [str(c) for c in mock_app.display_warning.call_args_list] + assert any("network error" in w for w in warning_args) + + +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" + + 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)) + + body = mock_app.github.post_pull_request_comment.call_args[0][1] + assert "[View full run]" not in body + + +# --- load_validations --- + + +def test_load_validations_returns_all_when_config_absent(mock_app): + mock_app.repo.config.get.return_value = None + + result = _load_validations(mock_app) + + assert result is VALIDATIONS + mock_app.display_warning.assert_not_called() + + +def test_load_validations_filters_to_selected_names(mock_app): + mock_app.repo.config.get.return_value = ["ci", "config"] + + result = _load_validations(mock_app) + + assert set(result) == {"ci", "config"} + assert result["ci"] == VALIDATIONS["ci"] + assert result["config"] == VALIDATIONS["config"] + mock_app.display_warning.assert_not_called() + + +def test_load_validations_warns_on_unknown_name(mock_app): + mock_app.repo.config.get.return_value = ["ci", "nonexistent"] + + result = _load_validations(mock_app) + + assert set(result) == {"ci"} + mock_app.display_warning.assert_called_once() + assert "nonexistent" in str(mock_app.display_warning.call_args) + + +def test_load_validations_empty_list_returns_empty(mock_app): + mock_app.repo.config.get.return_value = [] + + result = _load_validations(mock_app) + + assert result == {} + mock_app.display_warning.assert_not_called() diff --git a/prefect/tests/conftest.py b/prefect/tests/conftest.py index 4565e548809b0..c4dbb1955f9c4 100644 --- a/prefect/tests/conftest.py +++ b/prefect/tests/conftest.py @@ -7,8 +7,9 @@ from typing import Callable import pytest +import requests -from datadog_checks.dev.conditions import CheckDockerLogs, CheckEndpoints +from datadog_checks.dev.conditions import CheckDockerLogs, CheckEndpoints, WaitFor from datadog_checks.dev.docker import docker_run, get_docker_hostname from datadog_checks.dev.utils import find_free_port from datadog_checks.prefect import PrefectCheck @@ -20,6 +21,23 @@ } +def _check_task_runs_available(prefect_url: str): + """Verify that the Prefect API has task runs and task-run events queryable.""" + resp = requests.post(f"{prefect_url}/task_runs/filter", json={}, timeout=1) + resp.raise_for_status() + task_runs = resp.json() + assert task_runs, "No task runs available in Prefect API yet" + + resp = requests.post( + f"{prefect_url}/events/filter", + json={"filter": {"event": {"prefix": ["prefect.task-run"]}}}, + timeout=1, + ) + resp.raise_for_status() + events = resp.json().get("events", []) + assert events, "No task-run events available in Prefect API yet" + + @pytest.fixture(scope='session') def dd_environment(instance: Callable[[str], dict[str, str | dict[str, list[str]] | None | bool | int]]): port = find_free_port(get_docker_hostname()) @@ -35,6 +53,7 @@ def dd_environment(instance: Callable[[str], dict[str, str | dict[str, list[str] COMPOSE_FILE_E2E, patterns=["Finished all tasks"], service="prefect-worker", attempts=120, wait=1 ), CheckDockerLogs(COMPOSE_FILE_E2E, patterns=["Retried"], service="prefect-worker", attempts=120, wait=1), + WaitFor(lambda: _check_task_runs_available(prefect_url), attempts=60, wait=2), ] with docker_run( diff --git a/prefect/tests/test_e2e.py b/prefect/tests/test_e2e.py index c24cde87ac76e..7d68b4b25089f 100644 --- a/prefect/tests/test_e2e.py +++ b/prefect/tests/test_e2e.py @@ -83,7 +83,7 @@ @pytest.mark.e2e def test_e2e_metrics(dd_agent_check): - aggregator = dd_agent_check() + aggregator = dd_agent_check(check_times=2, pause=20000) cross_check_metrics = ( 'flow_runs.retry_gaps_duration', diff --git a/renovate.json b/renovate.json index 1681df118680d..bce7e6fdd6995 100644 --- a/renovate.json +++ b/renovate.json @@ -25,6 +25,15 @@ ], "minimumReleaseAge": "7 days" }, + { + "matchManagers": [ + "github-actions" + ], + "matchDepNames": [ + "python" + ], + "enabled": false + }, { "matchManagers": [ "github-actions"