Skip to content

Commit e69d4ef

Browse files
committed
Define workflow names in validation; symmetric error shape; cover CancelledError
- Move WORKFLOW_LINUX/WORKFLOW_WINDOWS from dispatch.py to validation.py and have dispatch.py import them from validation. Inverts the previous arrow so the layer that runs first owns the constants; validation no longer pulls in dispatch's async/HTTP modules at import time. - Make fetch_target's OSError handler use `if/else` for symmetry with verify_workflows_present_on_ref. Behavior unchanged; the two helpers now read identically instead of relying on `app.abort`'s `NoReturn` to keep the trailing abort unreachable. - Add direct unit tests for extract_run_urls. The existing test_command tests only feed httpx.HTTPStatusError (an Exception), so the BaseException-but-not-Exception re-raise that lets CancelledError / KeyboardInterrupt propagate was untested in CI. New tests pin both the flow-control propagation contract and the both-failure message shape.
1 parent 8e0ec39 commit e69d4ef

4 files changed

Lines changed: 98 additions & 7 deletions

File tree

ddev/src/ddev/cli/release/test_agent/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ def _print_plan(
9797
plan on the same channel means piping the command into a file leaves stdout clean and
9898
keeps the whole pre-dispatch narrative coherent on stderr.
9999
"""
100-
from ddev.cli.release.test_agent.dispatch import WORKFLOW_LINUX, WORKFLOW_WINDOWS
100+
from ddev.cli.release.test_agent.validation import WORKFLOW_LINUX, WORKFLOW_WINDOWS
101101

102102
app.display_info('Dispatch plan')
103103
app.display_info(f' Workflows: {WORKFLOW_LINUX}, {WORKFLOW_WINDOWS}')

ddev/src/ddev/cli/release/test_agent/dispatch.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
import asyncio
99
from typing import TYPE_CHECKING
1010

11+
from ddev.cli.release.test_agent.validation import WORKFLOW_LINUX, WORKFLOW_WINDOWS
12+
1113
if TYPE_CHECKING:
1214
from collections.abc import Sequence
1315

@@ -16,9 +18,6 @@
1618

1719
DispatchOutcome = GitHubResponse[WorkflowDispatchResult] | BaseException
1820

19-
WORKFLOW_LINUX = 'test-agent.yml'
20-
WORKFLOW_WINDOWS = 'test-agent-windows.yml'
21-
2221
# Hard-coded: the two test workflows only live on DataDog/integrations-core. Forks and other
2322
# integrations repos (extras, marketplace) have nothing to dispatch even if the branch/tag exists,
2423
# so deferring either component to repo metadata would just hide misconfiguration. If we ever

ddev/src/ddev/cli/release/test_agent/validation.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,18 @@
1717
from dataclasses import dataclass
1818
from typing import TYPE_CHECKING
1919

20-
from ddev.cli.release.test_agent.dispatch import WORKFLOW_LINUX, WORKFLOW_WINDOWS
21-
2220
if TYPE_CHECKING:
2321
from ddev.cli.application import Application
2422

2523
BRANCH_PATTERN = r'^\d+\.\d+\.x$'
2624
TAG_PATTERN = r'^\d+\.\d+\.\d+(-rc\.\d+)?$'
2725

26+
# The two workflow filenames the command targets. Defined here (the layer that runs first)
27+
# rather than in `dispatch.py`, so the import graph flows validation -> dispatch and validation
28+
# does not pull in dispatch's async/HTTP machinery at module-load time.
29+
WORKFLOW_LINUX = 'test-agent.yml'
30+
WORKFLOW_WINDOWS = 'test-agent-windows.yml'
31+
2832
WORKFLOW_FILES = [
2933
f'.github/workflows/{WORKFLOW_LINUX}',
3034
f'.github/workflows/{WORKFLOW_WINDOWS}',
@@ -116,7 +120,8 @@ def fetch_target(app: Application, target: ReleaseTarget) -> None:
116120
msg = str(e).lower()
117121
if any(fragment in msg for fragment in GIT_FETCH_MISSING_FRAGMENTS):
118122
app.abort(f'{kind.capitalize()} `{target.name}` not found on origin.')
119-
app.abort(f'Failed to fetch {kind} `{target.name}` from origin: {e}')
123+
else:
124+
app.abort(f'Failed to fetch {kind} `{target.name}` from origin: {e}')
120125

121126

122127
def verify_workflows_present_on_ref(app: Application, target: ReleaseTarget) -> None:
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
# (C) Datadog, Inc. 2026-present
2+
# All rights reserved
3+
# Licensed under a 3-clause BSD style license (see LICENSE)
4+
"""Direct unit tests for `dispatch.extract_run_urls`.
5+
6+
The CLI-level tests in `test_command.py` only ever raise `httpx.HTTPStatusError` (an
7+
`Exception`) from the dispatch call, so they do not exercise the
8+
`BaseException`-but-not-`Exception` re-raise guard that lets `CancelledError` and
9+
`KeyboardInterrupt` propagate. These tests pin that contract on the helper directly.
10+
"""
11+
12+
from __future__ import annotations
13+
14+
import asyncio
15+
from typing import Any
16+
17+
import pytest
18+
19+
from ddev.cli.release.test_agent.dispatch import extract_run_urls
20+
from ddev.utils.github_async import GitHubResponse
21+
from ddev.utils.github_async.models import WorkflowDispatchResult
22+
23+
24+
def _ok(html_url: str) -> GitHubResponse[WorkflowDispatchResult]:
25+
return GitHubResponse(
26+
data=WorkflowDispatchResult(
27+
workflow_run_id=1,
28+
run_url=f'https://api.github.com/{html_url}',
29+
html_url=html_url,
30+
),
31+
headers={},
32+
)
33+
34+
35+
def test_extract_run_urls_returns_both_html_urls() -> None:
36+
results = [_ok('https://github.com/x/runs/1'), _ok('https://github.com/x/runs/2')]
37+
38+
assert extract_run_urls(results) == ('https://github.com/x/runs/1', 'https://github.com/x/runs/2')
39+
40+
41+
@pytest.mark.parametrize(
42+
'flow_control_exc',
43+
[
44+
pytest.param(asyncio.CancelledError('user pressed ctrl-c'), id='cancelled-error'),
45+
pytest.param(KeyboardInterrupt(), id='keyboard-interrupt'),
46+
],
47+
)
48+
@pytest.mark.parametrize(
49+
'position',
50+
[
51+
pytest.param('linux', id='linux-cancelled'),
52+
pytest.param('windows', id='windows-cancelled'),
53+
],
54+
)
55+
def test_extract_run_urls_reraises_flow_control_exceptions(flow_control_exc: BaseException, position: str) -> None:
56+
"""`CancelledError` / `KeyboardInterrupt` in either slot must propagate verbatim, not be wrapped."""
57+
other = _ok('https://github.com/x/runs/sibling')
58+
if position == 'linux':
59+
results: list[Any] = [flow_control_exc, other]
60+
else:
61+
results = [other, flow_control_exc]
62+
63+
with pytest.raises(type(flow_control_exc)):
64+
extract_run_urls(results)
65+
66+
67+
def test_extract_run_urls_wraps_regular_exception_failure_as_runtime_error() -> None:
68+
"""Sanity check that the `Exception` branch still produces a wrapped RuntimeError."""
69+
err = RuntimeError('forbidden')
70+
results = [err, _ok('https://github.com/x/runs/2')]
71+
72+
with pytest.raises(RuntimeError, match=r'Linux dispatch failed:.*forbidden.*runs/2'):
73+
extract_run_urls(results)
74+
75+
76+
def test_extract_run_urls_both_failures_includes_both_reprs() -> None:
77+
"""Both-fail must surface both error reprs in the RuntimeError message (not via __notes__)."""
78+
linux_err = RuntimeError('linux-side detail')
79+
windows_err = RuntimeError('windows-side detail')
80+
81+
with pytest.raises(RuntimeError) as excinfo:
82+
extract_run_urls([linux_err, windows_err])
83+
84+
message = str(excinfo.value)
85+
assert 'Both dispatches failed' in message
86+
assert 'linux-side detail' in message
87+
assert 'windows-side detail' in message

0 commit comments

Comments
 (0)