Skip to content

Commit d82c63b

Browse files
HadhemiDDclaude
andauthored
[Dispatcher] Task Test Gatherer (DataDog#23938)
* Create dispatcher task test runner (squashed) * fix changelog * update with async client changes * Emit a failed BatchFinished when a test workflow times out Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * intial test gatherer * lint * changelog * Enrich failed checks and handle timed-out batches in the gatherer Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * lint * remove retry * download retry * Remove Jira reference from changelog * Anchor job-dir token matching to avoid substring collisions * update with test runner changes * Address Codex review on the test gatherer - Runner forwards workflow jobs: TaskTestRunner now populates BatchFinished.jobs from list_workflow_jobs, and the gatherer reads per-job status from message.jobs only (metadata-file fallback removed), so failed multi-job runs no longer report passing jobs as failed. - Disambiguate organized artifacts by platform and runner so jobs sharing integration+environment no longer overwrite each other's coverage/JUnit reports. - Merge the near-duplicate FailedCheck into WorkflowResult. - Use real coverage/JUnit samples as fixtures in the gatherer tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * update PR comment storage * TO DO * comments * lint * platform to strenum * updates * Address gatherer review: batch_id keying, clearer names, simpler artifact prefix - Key the gatherer registry by batch id (message.id) instead of run_id, so the batch identity is stable across retries (run_id changes on a re-run). - Rename job_dir -> job_artifacts_path for clarity. - Drop runner from the organized-artifact prefix; target-environment-platform matches BatchJob.artifact_name and still prevents cross-platform overwrites. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 2f942b5 commit d82c63b

15 files changed

Lines changed: 1471 additions & 45 deletions

ddev/changelog.d/23938.added

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Add TaskTestGatherer processor and a JUnit parsing utility for the CI dispatcher.

ddev/pyproject.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ dependencies = [
3232
"coverage",
3333
"datadog-api-client==2.20.0",
3434
"datadog-checks-dev[cli]>=39.0,<41",
35+
"defusedxml==0.7.1",
3536
"hatch>=1.13.0",
3637
"httpx",
3738
"jsonpointer",

ddev/src/ddev/cli/ci/tests/messages.py

Lines changed: 82 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,27 @@
55

66
import re
77
from dataclasses import dataclass, field
8-
from typing import TYPE_CHECKING, Literal
8+
from enum import StrEnum, auto
9+
from typing import TYPE_CHECKING
910

11+
from ddev.cli.ci.tests.status import Status
1012
from ddev.event_bus.orchestrator import BaseMessage
13+
from ddev.utils.junit import TestStatus
1114

1215
if TYPE_CHECKING:
1316
from pathlib import Path
1417

1518
from ddev.utils.github_async.models import WorkflowJob
19+
from ddev.utils.junit import JUnitReport, JUnitTestCase
20+
21+
22+
class Platform(StrEnum):
23+
"""Operating system a test job runs on."""
24+
25+
LINUX = auto()
26+
WINDOWS = auto()
27+
MACOS = auto()
28+
1629

1730
# Characters GitHub disallows in an artifact name (plus CR/LF).
1831
ARTIFACT_NAME_DISALLOWED = re.compile(r'["\:<>|*?\\/\r\n]')
@@ -29,7 +42,7 @@ class BatchJob:
2942
target: str
3043
runner: str
3144
environment: str
32-
platform: Literal["linux", "windows", "macos"]
45+
platform: Platform
3346
unit_tests: bool
3447
e2e_tests: bool
3548

@@ -45,11 +58,30 @@ def artifact_name(self) -> str:
4558

4659

4760
@dataclass
48-
class FailedCheck:
49-
"""A single failed test check within a workflow."""
61+
class JobResult:
62+
"""In-memory result of a single test job (one integration + environment + platform).
5063
51-
name: str
52-
url: str
64+
Carries the full parsed JUnit reports for the job, not just its failures, so the PR-comment
65+
layer can report passed/skipped counts as well.
66+
"""
67+
68+
integration: str
69+
environment: str
70+
platform: Platform
71+
status: Status
72+
failed_steps: list[str] = field(default_factory=list)
73+
reports: tuple[JUnitReport, ...] = ()
74+
75+
@property
76+
def failed_tests(self) -> list[JUnitTestCase]:
77+
"""Every failed/errored test case across this job's reports, flattened for rendering."""
78+
return [
79+
case
80+
for report in self.reports
81+
for suite in report.test_suites
82+
for case in suite.test_cases
83+
if case.status in (TestStatus.FAILED, TestStatus.ERROR)
84+
]
5385

5486

5587
@dataclass
@@ -105,13 +137,28 @@ def correlate(
105137

106138
@dataclass
107139
class WorkflowStatus:
108-
"""Status of a single GitHub Actions workflow run."""
140+
"""Status of a single GitHub Actions workflow run (one batch), with every job's result.
109141
142+
``batch_id`` is the human batch identifier (e.g. ``batch-01``) the comment renders; ``id`` is the
143+
numeric workflow run id and ``url`` links to the run.
144+
"""
145+
146+
batch_id: str
110147
url: str
111148
id: int
112-
success_count: int | None
113-
failed_count: int | None
114-
failed_checks: list[FailedCheck]
149+
success_count: int
150+
failed_count: int
151+
skipped_count: int
152+
results: list[JobResult]
153+
154+
@property
155+
def status(self) -> Status:
156+
"""Batch-level label: FAILURE if any job failed, else SUCCESS if any passed, else SKIPPED."""
157+
if self.failed_count > 0:
158+
return Status.FAILURE
159+
if self.success_count > 0:
160+
return Status.SUCCESS
161+
return Status.SKIPPED
115162

116163

117164
@dataclass
@@ -127,7 +174,7 @@ class TestBatch(BaseMessage):
127174
class BatchFinished(BaseMessage):
128175
"""Emitted when a GitHub Actions test workflow has completed."""
129176

130-
status: Literal["success", "failure", "skipped"]
177+
status: Status
131178
run_id: int
132179
workflow_url: str
133180
artifacts_path: str
@@ -137,7 +184,30 @@ class BatchFinished(BaseMessage):
137184

138185
@dataclass
139186
class UpdatePRComment(BaseMessage):
140-
"""Emitted to request a PR comment update."""
187+
"""Emitted per finished batch to request a PR comment update.
188+
189+
``revision`` is the gatherer's monotonic counter (one per consumed ``BatchFinished``); the
190+
PR-updater renders the latest and rejects stale revisions. ``done`` is ``True`` only on the
191+
revision that completes the final expected batch.
192+
"""
141193

194+
revision: int
142195
done: bool
143196
workflows: list[WorkflowStatus]
197+
198+
@property
199+
def passed(self) -> int:
200+
return sum(workflow.success_count for workflow in self.workflows)
201+
202+
@property
203+
def failed(self) -> int:
204+
return sum(workflow.failed_count for workflow in self.workflows)
205+
206+
@property
207+
def skipped(self) -> int:
208+
return sum(workflow.skipped_count for workflow in self.workflows)
209+
210+
@property
211+
def complete(self) -> int:
212+
"""Total jobs finished so far (passed + failed + skipped across all gathered batches)."""
213+
return sum(len(workflow.results) for workflow in self.workflows)
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
# (C) Datadog, Inc. 2026-present
2+
# All rights reserved
3+
# Licensed under a 3-clause BSD style license (see LICENSE)
4+
"""Internal status vocabulary for the ci/tests task pipeline.
5+
6+
GitHub's workflow-run/-job conclusions are a wide set of strings (see the models in
7+
``ddev.utils.github_async.models``). ``Status`` is the narrow, binary vocabulary the batch
8+
and PR-comment layers use internally, and ``conclusion_to_status`` is the single place that
9+
collapses a GitHub conclusion into it.
10+
"""
11+
12+
from __future__ import annotations
13+
14+
from enum import StrEnum, auto
15+
16+
from ddev.utils.github_async.models.workflow import WorkflowJobConclusion
17+
18+
19+
class Status(StrEnum):
20+
"""Binary outcome of a batch, job, or test as reported internally."""
21+
22+
SUCCESS = auto()
23+
FAILURE = auto()
24+
SKIPPED = auto()
25+
26+
27+
def conclusion_to_status(conclusion: str | None) -> Status:
28+
"""Map a GitHub Actions conclusion to the internal :class:`Status`.
29+
30+
Note: ``None`` maps to ``Status.FAILURE`` here while a check run reports ``"neutral"``
31+
for the same input. The asymmetry is intentional — status consumers want a binary
32+
outcome, the check UI prefers an explicit ``"neutral"`` badge.
33+
"""
34+
if conclusion == WorkflowJobConclusion.SUCCESS:
35+
return Status.SUCCESS
36+
if conclusion == WorkflowJobConclusion.SKIPPED:
37+
return Status.SKIPPED
38+
return Status.FAILURE
Lines changed: 185 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,185 @@
1+
# (C) Datadog, Inc. 2026-present
2+
# All rights reserved
3+
# Licensed under a 3-clause BSD style license (see LICENSE)
4+
from __future__ import annotations
5+
6+
import logging
7+
import shutil
8+
import threading
9+
from pathlib import Path
10+
from typing import TYPE_CHECKING
11+
12+
from ddev.cli.ci.tests.messages import (
13+
BatchFinished,
14+
BatchJob,
15+
BatchJobResult,
16+
JobResult,
17+
UpdatePRComment,
18+
WorkflowStatus,
19+
)
20+
from ddev.cli.ci.tests.status import Status, conclusion_to_status
21+
from ddev.event_bus.orchestrator import SyncProcessor
22+
from ddev.utils.github_async.models.workflow import WorkflowJobConclusion
23+
from ddev.utils.junit import parse_junit_dir
24+
25+
if TYPE_CHECKING:
26+
from ddev.utils.junit import JUnitReport
27+
28+
# Expected layout of the extracted ``test-result.zip`` tree (defined by ``test-batch.yaml``):
29+
# {artifacts_path}/
30+
# {artifact_name}/ one directory per job (its BatchJobResult.artifact_name_path)
31+
# coverage.xml Cobertura coverage report
32+
# test-{unit|e2e}-{env}.xml pytest JUnit report(s)
33+
# Each job's spec, workflow-job result, and artifact directory come pre-correlated on the message
34+
# (BatchFinished.batch_jobs). A timed-out batch fails every job; otherwise each job's status is its own
35+
# workflow-job conclusion, and a job with no correlated workflow job is a runner bug and raises.
36+
COVERAGE_GLOB = "coverage*.xml"
37+
JUNIT_GLOB = "test-*.xml"
38+
39+
40+
class TaskTestGatherer(SyncProcessor[BatchFinished]):
41+
"""
42+
Reads ``BatchFinished`` messages, analyzes the downloaded artifacts on disk, builds per-job
43+
``JobResult`` records, and organizes coverage/JUnit files for later publishing.
44+
45+
It keeps an in-memory registry of every job's full result across all batches and, on each finished
46+
batch, emits an ``UpdatePRComment`` carrying a monotonically increasing ``revision`` and the whole
47+
accumulated state. ``done`` is set once the final expected batch has been received. It does not post
48+
to GitHub — rendering the comment (and rejecting stale revisions) is a separate consumer's job.
49+
50+
This task makes no GitHub API calls — it works exclusively from the artifacts the runner
51+
already downloaded to ``BatchFinished.artifacts_path``.
52+
"""
53+
54+
def __init__(self, name: str, output_base_path: Path, expected_batches: int) -> None:
55+
super().__init__(name)
56+
self._output_base_path = output_base_path
57+
self._expected_batches = expected_batches
58+
self._received_batches = 0
59+
self._status_by_batch: dict[str, WorkflowStatus] = {}
60+
self._results_by_batch: dict[str, list[JobResult]] = {}
61+
self._lock = threading.Lock()
62+
self._logger = logging.getLogger(f"{__name__}.{name}")
63+
64+
def process_message(self, message: BatchFinished) -> None:
65+
if not message.batch_jobs:
66+
self._logger.warning("BatchFinished carried no jobs; nothing to gather", extra={"run_id": message.run_id})
67+
return
68+
69+
# Parse and organize artifacts outside the lock — these touch only this batch's own files.
70+
results = [self._job_result(batch_job_result, message) for batch_job_result in message.batch_jobs]
71+
status = self._build_workflow_status(message, results)
72+
73+
# Register the batch, bump the revision, and emit the update all under the lock so two batches
74+
# finishing at once cannot build a comment from half-updated shared state.
75+
with self._lock:
76+
# Keyed by batch id (stable across retries; run_id changes on a re-run). A duplicate is
77+
# ignored so it can't re-count the batch and inflate the revision — this check is
78+
# authoritative only inside the lock. (Retry-replace semantics come with the retry work.)
79+
if message.id in self._results_by_batch:
80+
self._logger.warning(
81+
"Duplicate BatchFinished ignored", extra={"batch_id": message.id, "run_id": message.run_id}
82+
)
83+
return
84+
self._results_by_batch[message.id] = results
85+
self._status_by_batch[message.id] = status
86+
self._received_batches += 1
87+
revision = self._received_batches
88+
done = revision >= self._expected_batches
89+
self.submit_message(self.build_update_message(message.id, revision, done))
90+
91+
self._logger.info(
92+
"Batch gathered, UpdatePRComment revision %s emitted (done=%s)",
93+
revision,
94+
done,
95+
extra={"run_id": message.run_id},
96+
)
97+
98+
def build_update_message(self, message_id: str, revision: int, done: bool) -> UpdatePRComment:
99+
"""Build an ``UpdatePRComment`` for *revision* from all accumulated results.
100+
101+
Must be called while holding ``self._lock`` when reading live shared state.
102+
"""
103+
return UpdatePRComment(
104+
id=message_id, revision=revision, done=done, workflows=list(self._status_by_batch.values())
105+
)
106+
107+
def _job_result(self, batch_job_result: BatchJobResult, message: BatchFinished) -> JobResult:
108+
"""Build a job's ``JobResult`` from its correlated workflow job and artifacts on disk."""
109+
batch_job = batch_job_result.job
110+
status, failed_steps = self._job_status(batch_job_result, message)
111+
112+
reports: tuple[JUnitReport, ...] = ()
113+
job_artifacts_path = Path(batch_job_result.artifact_name_path) if batch_job_result.artifact_name_path else None
114+
if job_artifacts_path is not None:
115+
reports = tuple(parse_junit_dir(job_artifacts_path))
116+
self._organize_artifacts(job_artifacts_path, batch_job)
117+
else:
118+
self._logger.warning(
119+
"No artifact directory found for job %s", batch_job.name, extra={"run_id": message.run_id}
120+
)
121+
122+
return JobResult(
123+
integration=batch_job.target,
124+
environment=batch_job.environment,
125+
platform=batch_job.platform,
126+
status=status,
127+
failed_steps=failed_steps,
128+
reports=reports,
129+
)
130+
131+
@staticmethod
132+
def _job_status(batch_job_result: BatchJobResult, message: BatchFinished) -> tuple[Status, list[str]]:
133+
"""Per-job (status, failed_steps). Deterministic: timed-out batches fail every job; otherwise the
134+
job's own workflow-job conclusion decides. A missing workflow job is unexpected and raises — the
135+
runner correlates every job before emitting, so a miss is a bug, not a state to paper over.
136+
137+
All steps concluding in failure are collected: a workflow can run on-failure steps, so more than
138+
one step may fail for a single job.
139+
"""
140+
if message.timed_out:
141+
return (Status.FAILURE, ["timed out"])
142+
143+
workflow_job = batch_job_result.workflow_job
144+
if workflow_job is None:
145+
raise ValueError(f"No workflow job correlated for {batch_job_result.job.name!r}")
146+
147+
failed_steps = [step.name for step in workflow_job.steps if step.conclusion == WorkflowJobConclusion.FAILURE]
148+
return (conclusion_to_status(workflow_job.conclusion), failed_steps)
149+
150+
def _organize_artifacts(self, job_artifacts_path: Path, batch_job: BatchJob) -> None:
151+
"""Copy coverage and JUnit files into the organized output tree with unique names.
152+
153+
The prefix is the job's target/environment/platform — the same fields as
154+
``BatchJob.artifact_name`` and the uniqueness key for a job within a batch.
155+
"""
156+
prefix = f"{batch_job.target}-{batch_job.environment}-{batch_job.platform}"
157+
158+
coverage_dir = self._output_base_path / "coverage"
159+
for index, coverage_file in enumerate(sorted(job_artifacts_path.rglob(COVERAGE_GLOB))):
160+
suffix = "" if index == 0 else f"-{index}"
161+
self._copy(coverage_file, coverage_dir / f"{prefix}{suffix}.xml")
162+
163+
test_results_dir = self._output_base_path / "test_results"
164+
for junit_file in sorted(job_artifacts_path.rglob(JUNIT_GLOB)):
165+
self._copy(junit_file, test_results_dir / f"{prefix}-{junit_file.stem}.xml")
166+
167+
def _copy(self, source: Path, destination: Path) -> None:
168+
destination.parent.mkdir(parents=True, exist_ok=True)
169+
shutil.copy2(source, destination)
170+
self._logger.debug("Organized artifact %s -> %s", source, destination)
171+
172+
@staticmethod
173+
def _build_workflow_status(message: BatchFinished, results: list[JobResult]) -> WorkflowStatus:
174+
success_count = sum(1 for result in results if result.status == Status.SUCCESS)
175+
failed_count = sum(1 for result in results if result.status == Status.FAILURE)
176+
skipped_count = sum(1 for result in results if result.status == Status.SKIPPED)
177+
return WorkflowStatus(
178+
batch_id=message.id,
179+
url=message.workflow_url,
180+
id=message.run_id,
181+
success_count=success_count,
182+
failed_count=failed_count,
183+
skipped_count=skipped_count,
184+
results=results,
185+
)

0 commit comments

Comments
 (0)