DEVX-7: fix: make all warnings into errors across devx tools

This commit is contained in:
emil
2026-06-22 20:54:25 +00:00
parent 621b9936c8
commit 8411c92c95
20 changed files with 827 additions and 304 deletions
+1 -1
View File
@@ -1 +1 @@
DEVX-6 DEVX-7
+2 -1
View File
@@ -2,6 +2,7 @@
from __future__ import annotations from __future__ import annotations
import json
import logging import logging
import time import time
from typing import Any from typing import Any
@@ -21,7 +22,7 @@ def _parse_error(e: requests.HTTPError) -> tuple[int, str]:
try: try:
body: dict[str, Any] = response.json() if response is not None else {} body: dict[str, Any] = response.json() if response is not None else {}
message: str = body.get("message", str(e)) message: str = body.get("message", str(e))
except Exception: except (json.JSONDecodeError, ValueError, AttributeError):
message = str(e) message = str(e)
return status, message return status, message
+18 -14
View File
@@ -114,12 +114,11 @@ def validate_pr_title(pr_title: str, task_id: str) -> None:
def get_vikunja_task_title(task_id: str) -> str: def get_vikunja_task_title(task_id: str) -> str:
"""Fetch the Vikunja task title for the given DEVX-N identifier. """Fetch the Vikunja task title for the given DEVX-N identifier.
Returns empty string if VIKUNJA_TOKEN is not set (local dev without token). Raises ClickException if VIKUNJA_TOKEN is not set or the task is not found.
Raises ClickException if the token is set but the task is not found.
""" """
token = os.environ.get("VIKUNJA_TOKEN", "") token = os.environ.get("VIKUNJA_TOKEN", "")
if not token: if not token:
return "" raise click.ClickException(_("VIKUNJA_TOKEN is not set. This is required in CI to validate PR titles."))
client = VikunjaClient(VIKUNJA_API_URL, token) client = VikunjaClient(VIKUNJA_API_URL, token)
page = 1 page = 1
while True: while True:
@@ -145,14 +144,10 @@ def get_vikunja_task_title(task_id: str) -> str:
def validate_pr_title_matches_vikunja(pr_title: str, task_id: str) -> None: def validate_pr_title_matches_vikunja(pr_title: str, task_id: str) -> None:
"""Validate that PR title matches the Vikunja task title. """Validate that PR title matches the Vikunja task title.
Skips validation if VIKUNJA_TOKEN is not set (local dev). Raises ClickException if VIKUNJA_TOKEN is not set, the task is not found,
Raises ClickException if the task is not found or the title doesn't match. or the title doesn't match.
""" """
vikunja_title = get_vikunja_task_title(task_id) vikunja_title = get_vikunja_task_title(task_id)
if not vikunja_title:
# VIKUNJA_TOKEN not set — skip validation (local dev)
click.echo(_("Warning: VIKUNJA_TOKEN not set, skipping title match validation."))
return
expected = f"{task_id}: {vikunja_title}" expected = f"{task_id}: {vikunja_title}"
if pr_title != expected: if pr_title != expected:
raise click.ClickException( raise click.ClickException(
@@ -193,7 +188,16 @@ def main(branch: str, pr_title: str, repo: str, pr_number: str) -> None:
if not token: if not token:
raise click.ClickException(_("ERROR: REPO_TOKEN is not set.")) raise click.ClickException(_("ERROR: REPO_TOKEN is not set."))
owner, repo_name = repo.split("/") # Validate PR number is an integer
try:
pr_num = int(pr_number)
except ValueError:
raise click.ClickException(_("PR number must be an integer, got: {pr_number}", pr_number=pr_number)) from None
# Validate repo format
if "/" not in repo:
raise click.ClickException(_("Repo must be in 'owner/name' format, got: {repo}", repo=repo))
owner, repo_name = repo.split("/", 1)
client = GiteaClient(GITEA_API_URL, token, owner, repo_name) client = GiteaClient(GITEA_API_URL, token, owner, repo_name)
task_id = read_taskid(branch) task_id = read_taskid(branch)
@@ -210,14 +214,14 @@ def main(branch: str, pr_title: str, repo: str, pr_number: str) -> None:
validate_pr_title_matches_vikunja(pr_title, task_id) validate_pr_title_matches_vikunja(pr_title, task_id)
# Build merge title: DEVX-N: <conventional commit message> # Build merge title: DEVX-N: <conventional commit message>
commits = client.get_pr_commits(pr_number) commits = client.get_pr_commits(pr_num)
conv_msg = extract_conventional_msg(commits) conv_msg = extract_conventional_msg(commits)
if not conv_msg: if not conv_msg:
raise click.ClickException(_("Could not extract conventional commit message from PR commits.")) raise click.ClickException(_("Could not extract conventional commit message from PR commits."))
merge_title = f"{task_id}: {conv_msg}" merge_title = f"{task_id}: {conv_msg}"
try: try:
client.merge_pr(pr_number, merge_title) client.merge_pr(pr_num, merge_title)
except APIError as e: except APIError as e:
if e.status == 405 and "behind" in e.message.lower(): if e.status == 405 and "behind" in e.message.lower():
# Head branch is behind master — pull master and rebase, then retry # Head branch is behind master — pull master and rebase, then retry
@@ -229,7 +233,7 @@ def main(branch: str, pr_title: str, repo: str, pr_number: str) -> None:
run_cmd(["git", "rebase", "origin/master"]) run_cmd(["git", "rebase", "origin/master"])
run_cmd(["git", "push", "--force-with-lease", "origin", f"HEAD:{branch}"]) run_cmd(["git", "push", "--force-with-lease", "origin", f"HEAD:{branch}"])
click.echo(_("Rebased and pushed. Retrying merge...")) click.echo(_("Rebased and pushed. Retrying merge..."))
client.merge_pr(pr_number, merge_title) client.merge_pr(pr_num, merge_title)
except (APIError, Exception) as retry_err: except (APIError, Exception) as retry_err:
raise click.ClickException( raise click.ClickException(
_( _(
@@ -250,7 +254,7 @@ def main(branch: str, pr_title: str, repo: str, pr_number: str) -> None:
click.echo( click.echo(
_( _(
"Nice! PR #{pr_number} squash-merged with title: {merge_title}", "Nice! PR #{pr_number} squash-merged with title: {merge_title}",
pr_number=pr_number, pr_number=pr_num,
merge_title=merge_title, merge_title=merge_title,
) )
) )
+7 -16
View File
@@ -12,13 +12,13 @@ Checks performed (all fail with exit code 1 on error):
translations file. translations file.
- **Dead keys**: a key in a translations file is not used in any code. - **Dead keys**: a key in a translations file is not used in any code.
- **Missing languages**: a key exists but is missing one of the 5 supported - **Missing languages**: a key exists but is missing one of the 5 supported
languages (en, bg, de, ru, zh). Reported as a warning, not an error. languages (en, bg, de, ru, zh). This is an error — all supported languages
must have translations for every key.
Usage:: Usage::
python3 -m devx.ci.check_translations python3 -m devx.ci.check_translations
python3 -m devx.ci.check_translations --translations path/to/translations.json python3 -m devx.ci.check_translations --translations path/to/translations.json
python3 -m devx.ci.check_translations --strict # warnings are errors
""" """
from __future__ import annotations from __future__ import annotations
@@ -130,14 +130,15 @@ def check_translation_set(name: str, src_dir: Path, trans_file: Path) -> Transla
# Check for dead keys (in translations but not used in code) # Check for dead keys (in translations but not used in code)
result.dead_keys = result.defined_keys - result.used_keys result.dead_keys = result.defined_keys - result.used_keys
for key in sorted(result.dead_keys): for key in sorted(result.dead_keys):
result.warnings.append(f"Dead key in {name}: {key!r}") result.errors.append(f"Dead key in {name}: {key!r}")
# Check for missing languages # Check for missing languages — this is an error, not a warning.
# All supported languages must have translations for every key.
for key, langs in translations.items(): for key, langs in translations.items():
missing = [lang for lang in SUPPORTED_LANGS if lang not in langs] missing = [lang for lang in SUPPORTED_LANGS if lang not in langs]
if missing: if missing:
result.missing_langs[key] = missing result.missing_langs[key] = missing
result.warnings.append(f"Missing languages {missing} for key {key!r} in {name}") result.errors.append(f"Missing languages {missing} for key {key!r} in {name}")
return result return result
@@ -170,8 +171,7 @@ def print_result(result: TranslationCheckResult) -> None:
type=click.Path(exists=False, path_type=Path), type=click.Path(exists=False, path_type=Path),
help="Path to a translations JSON file to check (can be repeated). Defaults to src/devx/translations.json.", help="Path to a translations JSON file to check (can be repeated). Defaults to src/devx/translations.json.",
) )
@click.option("--strict", is_flag=True, default=False, help="Treat warnings as errors.") def main(translations: tuple[Path, ...]) -> None:
def main(translations: tuple[Path, ...], strict: bool) -> None:
"""Check translation files for gaps, dead keys, and missing languages.""" """Check translation files for gaps, dead keys, and missing languages."""
if not translations: if not translations:
# Default: check the devx package's own translations # Default: check the devx package's own translations
@@ -187,24 +187,15 @@ def main(translations: tuple[Path, ...], strict: bool) -> None:
results.append(check_translation_set(name, src_dir, trans_file)) results.append(check_translation_set(name, src_dir, trans_file))
has_errors = False has_errors = False
has_warnings = False
for result in results: for result in results:
print_result(result) print_result(result)
if result.errors: if result.errors:
has_errors = True has_errors = True
if result.warnings:
has_warnings = True
click.echo() click.echo()
if has_errors: if has_errors:
click.echo("FAIL: Translation check found errors.", err=True) click.echo("FAIL: Translation check found errors.", err=True)
sys.exit(1) sys.exit(1)
if strict and has_warnings:
click.echo("FAIL: Translation check found warnings (--strict mode).", err=True)
sys.exit(1)
if has_warnings:
click.echo("PASS with warnings: Translation check passed (warnings present).")
else:
click.echo("PASS: All translations are complete and up to date.") click.echo("PASS: All translations are complete and up to date.")
+13 -7
View File
@@ -39,7 +39,7 @@ def query_runners(api_url: str, token: str, owner: str, repo: str) -> int:
Returns the total count of active runners. If the API call fails Returns the total count of active runners. If the API call fails
(e.g., no admin access for instance-level runners), falls back to (e.g., no admin access for instance-level runners), falls back to
what we can see. what we can see. Fallbacks are logged to stderr for debugging.
""" """
headers = {"Authorization": f"token {token}"} headers = {"Authorization": f"token {token}"}
total = 0 total = 0
@@ -54,8 +54,10 @@ def query_runners(api_url: str, token: str, owner: str, repo: str) -> int:
if r.status_code == 200: if r.status_code == 200:
data = r.json() data = r.json()
total += data.get("total_count", 0) total += data.get("total_count", 0)
except (requests.RequestException, ValueError): else:
pass click.echo(f"Warning: repo-level runners query returned HTTP {r.status_code}", err=True)
except (requests.RequestException, ValueError) as e:
click.echo(f"Warning: repo-level runners query failed: {e}", err=True)
# 2. Organization-level runners # 2. Organization-level runners
try: try:
@@ -67,8 +69,10 @@ def query_runners(api_url: str, token: str, owner: str, repo: str) -> int:
if r.status_code == 200: if r.status_code == 200:
data = r.json() data = r.json()
total += data.get("total_count", 0) total += data.get("total_count", 0)
except (requests.RequestException, ValueError): else:
pass click.echo(f"Warning: org-level runners query returned HTTP {r.status_code}", err=True)
except (requests.RequestException, ValueError) as e:
click.echo(f"Warning: org-level runners query failed: {e}", err=True)
# 3. Instance-level runners (requires admin scope) # 3. Instance-level runners (requires admin scope)
try: try:
@@ -80,8 +84,10 @@ def query_runners(api_url: str, token: str, owner: str, repo: str) -> int:
if r.status_code == 200: if r.status_code == 200:
data = r.json() data = r.json()
total += data.get("total_count", 0) total += data.get("total_count", 0)
except (requests.RequestException, ValueError): elif r.status_code != 403: # 403 is expected without admin scope
pass click.echo(f"Warning: instance-level runners query returned HTTP {r.status_code}", err=True)
except (requests.RequestException, ValueError) as e:
click.echo(f"Warning: instance-level runners query failed: {e}", err=True)
return total return total
+12 -4
View File
@@ -15,7 +15,7 @@ Usage:
from __future__ import annotations from __future__ import annotations
import contextlib import logging
import os import os
import click import click
@@ -27,25 +27,33 @@ from devx.i18n import _
load_dotenv() load_dotenv()
logger = logging.getLogger("devx")
def _create_issue_via_tea(repo: str, title: str, body: str) -> int: def _create_issue_via_tea(repo: str, title: str, body: str) -> int:
"""Create issue via tea CLI. Returns issue index. """Create issue via tea CLI. Returns issue index.
Raises TeaCLIError if tea is not installed or the command fails. Raises TeaCLIError if tea is not installed or the command fails.
Label operations are best-effort — failures are logged but don't
prevent issue creation.
""" """
tea = TeaCLI(repo=repo) tea = TeaCLI(repo=repo)
# Check if "bug" label exists # Check if "bug" label exists (best-effort)
labels: list[str] = [] labels: list[str] = []
with contextlib.suppress(TeaCLIError): try:
existing_labels = tea.list_labels(repo) existing_labels = tea.list_labels(repo)
if any(label.get("name") == "bug" for label in existing_labels): if any(label.get("name") == "bug" for label in existing_labels):
labels = ["bug"] labels = ["bug"]
except TeaCLIError as e:
logger.warning("Could not fetch labels (best-effort): %s", e)
issue = tea.create_issue(repo, title=title, body=body, labels=labels if labels else None) issue = tea.create_issue(repo, title=title, body=body, labels=labels if labels else None)
if labels: if labels:
with contextlib.suppress(TeaCLIError): try:
tea.add_label(repo, issue["index"], labels) tea.add_label(repo, issue["index"], labels)
except TeaCLIError as e:
logger.warning("Could not add label to issue #%s (best-effort): %s", issue.get("index"), e)
return int(issue.get("index", 0)) return int(issue.get("index", 0))
+14 -13
View File
@@ -13,7 +13,7 @@ import click
from dotenv import load_dotenv # pyright: ignore[reportMissingImports,reportUnknownVariableType] from dotenv import load_dotenv # pyright: ignore[reportMissingImports,reportUnknownVariableType]
from devx.api_clients import VikunjaClient from devx.api_clients import VikunjaClient
from devx.config import DEFAULT_PER_PAGE, TASK_ID_RE, VIKUNJA_API_URL, VIKUNJA_PROJECT_ID from devx.config import DEFAULT_PER_PAGE, TASK_ID_RE, TASK_PREFIX, VIKUNJA_API_URL, VIKUNJA_PROJECT_ID
from devx.exceptions import APIError from devx.exceptions import APIError
from devx.i18n import _ from devx.i18n import _
@@ -151,14 +151,16 @@ def main(commit_msg: str | None, commit_sha: str, from_git: bool, git_sha: str)
) )
) )
return return
# Non-infrastructure commits without DEVX-N prefix — warn but don't fail # Non-infrastructure commits without DEVX-N prefix — this is a
click.echo( # convention violation. Fail the post-merge job so the issue is visible.
raise click.ClickException(
_( _(
"Warning: No task ID (DEVX-N) found in commit message: {msg}. Skipping Vikunja update.", "No task ID ({prefix}-N) found in commit message: {msg}. "
"Every non-infrastructure commit must have a task ID.",
prefix=TASK_PREFIX,
msg=first_line, msg=first_line,
) )
) )
return
client = VikunjaClient(VIKUNJA_API_URL, token) client = VikunjaClient(VIKUNJA_API_URL, token)
vikunja_task_id = resolve_task_id(client, task_id) vikunja_task_id = resolve_task_id(client, task_id)
@@ -170,19 +172,18 @@ def main(commit_msg: str | None, commit_sha: str, from_git: bool, git_sha: str)
client.post_comment(vikunja_task_id, html) client.post_comment(vikunja_task_id, html)
client.update_task(vikunja_task_id, done=True) client.update_task(vikunja_task_id, done=True)
except APIError as e: except APIError as e:
# Vikunja is a project management tool — if it's down, the merge # Vikunja API failures must be visible — the task was not updated
# still succeeded. Warn but don't fail the post-merge workflow. # and needs manual intervention. Failing the CI job makes this visible.
click.echo( raise click.ClickException(
_( _(
"Warning: Vikunja API error (HTTP {status}): {message}. " "Vikunja API error (HTTP {status}): {message}. "
"Task {task_id} was NOT updated. The merge succeeded — " "Task {task_id} was NOT updated. "
"please update the Vikunja task manually.", "The merge succeeded but the Vikunja task needs manual update.",
status=e.status, status=e.status,
message=e.message, message=e.message,
task_id=task_id, task_id=task_id,
) )
) ) from e
return
click.echo( click.echo(
_( _(
+13 -2
View File
@@ -86,7 +86,13 @@ def get_bumped_version() -> str:
if not version: if not version:
raise click.ClickException(_("git-cliff returned empty version.")) raise click.ClickException(_("git-cliff returned empty version."))
# git-cliff may return with or without 'v' prefix # git-cliff may return with or without 'v' prefix
return version.lstrip("v") version = version.lstrip("v")
# Validate semver format
if not re.match(r"^\d+\.\d+\.\d+$", version):
raise click.ClickException(
_("git-cliff returned invalid version format: {version}. Expected semver (e.g., 0.4.1).", version=version)
)
return version
def get_changelog(new_version: str) -> str: def get_changelog(new_version: str) -> str:
@@ -333,7 +339,12 @@ def main(dry_run: bool, skip_tests: bool) -> None:
# Generate changelog # Generate changelog
changelog = get_changelog(new_version) changelog = get_changelog(new_version)
if not changelog: if not changelog:
click.echo(_("Warning: git-cliff generated empty changelog.")) raise click.ClickException(
_(
"git-cliff generated empty changelog for v{version}. Check cliff.toml and commit history.",
version=new_version,
)
)
if dry_run: if dry_run:
click.echo(_("\n[dry-run] Changelog:\n{changelog}", changelog=changelog)) click.echo(_("\n[dry-run] Changelog:\n{changelog}", changelog=changelog))
+19 -8
View File
@@ -39,9 +39,20 @@ MAPPING_FILE = DOCS_DIR / "mapping.json"
def load_mapping() -> dict[str, str]: def load_mapping() -> dict[str, str]:
"""Load the file-to-wiki-page mapping from mapping.json.""" """Load the file-to-wiki-page mapping from mapping.json.
Validates that the mapping is a dict of string-to-string pairs.
"""
with open(MAPPING_FILE) as f: with open(MAPPING_FILE) as f:
return json.load(f) data = json.load(f)
if not isinstance(data, dict):
raise click.ClickException(
_("mapping.json must be a dict of file-path -> page-title, got {type}", type=type(data).__name__)
)
for k, v in data.items():
if not isinstance(k, str) or not isinstance(v, str):
raise click.ClickException(_("mapping.json keys and values must be strings, got {k}={v}", k=k, v=v))
return data
def read_doc_content(file_path: str) -> str: def read_doc_content(file_path: str) -> str:
@@ -239,14 +250,14 @@ def main(dry_run: bool, repo: str | None, verify: bool, strict: bool) -> None:
try: try:
content = read_doc_content(file_path) content = read_doc_content(file_path)
except FileNotFoundError: except FileNotFoundError:
click.echo(_("WARNING: File {file} not found — skipping.", file=file_path)) raise click.ClickException(
skipped += 1 _("Mapped file {file} not found. Update mapping.json or create the file.", file=file_path)
continue ) from None
if not content.strip(): if not content.strip():
click.echo(_("WARNING: File {file} is empty — skipping.", file=file_path)) raise click.ClickException(
skipped += 1 _("Mapped file {file} is empty. Update the content or remove from mapping.json.", file=file_path)
continue ) from None
result = sync_page(client, page_title, content, existing_pages, dry_run) result = sync_page(client, page_title, content, existing_pages, dry_run)
if result == "created": if result == "created":
+4
View File
@@ -174,6 +174,10 @@ def cli(
_write_github_env("SKIP", "true") _write_github_env("SKIP", "true")
return return
# Validate runner index is in range
if runner_index < 1:
raise click.ClickException(f"Runner index {runner_index} is out of range (must be >= 1)")
# Convert 1-based CLI index to 0-based internal index # Convert 1-based CLI index to 0-based internal index
zero_based = runner_index - 1 zero_based = runner_index - 1
assigned = pairs_for_runner(pairs, zero_based, max_runners) assigned = pairs_for_runner(pairs, zero_based, max_runners)
+5 -2
View File
@@ -159,8 +159,11 @@ def cli(pairs: tuple[str, ...]) -> None:
if failed_event.is_set(): if failed_event.is_set():
sys.exit(1) sys.exit(1)
scenario = pair.split("|")[0] parts = pair.split("|")
platform_name = pair.split("|")[1] if len(parts) < 2:
raise click.ClickException(f"Invalid pair format: {pair!r} (expected at least 2 pipe-delimited parts)")
scenario = parts[0]
platform_name = parts[1]
click.echo(_("Running: {scenario} on {platform}", scenario=scenario, platform=platform_name)) click.echo(_("Running: {scenario} on {platform}", scenario=scenario, platform=platform_name))
cmd = build_molecule_cmd(scenario) cmd = build_molecule_cmd(scenario)
+562 -178
View File
File diff suppressed because it is too large Load Diff
+48 -12
View File
@@ -77,8 +77,9 @@ class TestValidatePrTitle:
class TestValidatePrTitleMatchesVikunja: class TestValidatePrTitleMatchesVikunja:
@patch.dict("os.environ", {}, clear=True) @patch.dict("os.environ", {}, clear=True)
def test_skips_when_no_token(self) -> None: def test_raises_when_no_token(self) -> None:
# Should not raise — just warn """Should raise ClickException when VIKUNJA_TOKEN is not set."""
with pytest.raises(click.ClickException, match="VIKUNJA_TOKEN is not set"):
validate_pr_title_matches_vikunja("DEVX-19: test", "DEVX-19") validate_pr_title_matches_vikunja("DEVX-19: test", "DEVX-19")
@patch.dict("os.environ", {"VIKUNJA_TOKEN": "tok"}, clear=True) @patch.dict("os.environ", {"VIKUNJA_TOKEN": "tok"}, clear=True)
@@ -192,9 +193,12 @@ class TestRunCmd:
class TestMain: class TestMain:
@patch.dict("os.environ", {"REPO_TOKEN": "tok", "VIKUNJA_TOKEN": ""}, clear=True) @patch.dict("os.environ", {"REPO_TOKEN": "tok", "VIKUNJA_TOKEN": "tok"}, clear=True)
@patch("devx.ci.auto_merge.validate_pr_title_matches_vikunja")
@patch("devx.ci.auto_merge.GiteaClient") @patch("devx.ci.auto_merge.GiteaClient")
def test_full_merge_flow(self, mock_client_cls: MagicMock, tmp_path, monkeypatch) -> None: # type: ignore[no-untyped-def] def test_full_merge_flow(
self, mock_client_cls: MagicMock, _mock_validate: MagicMock, tmp_path, monkeypatch
) -> None: # type: ignore[no-untyped-def]
monkeypatch.chdir(tmp_path) monkeypatch.chdir(tmp_path)
(tmp_path / ".taskid").write_text("DEVX-19\n") (tmp_path / ".taskid").write_text("DEVX-19\n")
@@ -210,7 +214,7 @@ class TestMain:
["DEVX-19-fix-bug", "DEVX-19: Fix timeout", "owner/repo", "7"], ["DEVX-19-fix-bug", "DEVX-19: Fix timeout", "owner/repo", "7"],
) )
assert result.exit_code == 0, result.output assert result.exit_code == 0, result.output
mock_client.merge_pr.assert_called_once_with("7", "DEVX-19: fix: resolve timeout") mock_client.merge_pr.assert_called_once_with(7, "DEVX-19: fix: resolve timeout")
@patch.dict("os.environ", {"REPO_TOKEN": ""}, clear=True) @patch.dict("os.environ", {"REPO_TOKEN": ""}, clear=True)
def test_no_token_raises(self) -> None: def test_no_token_raises(self) -> None:
@@ -240,9 +244,12 @@ class TestMain:
assert result.exit_code != 0 assert result.exit_code != 0
assert "format" in result.output.lower() assert "format" in result.output.lower()
@patch.dict("os.environ", {"REPO_TOKEN": "tok"}, clear=True) @patch.dict("os.environ", {"REPO_TOKEN": "tok", "VIKUNJA_TOKEN": "tok"}, clear=True)
@patch("devx.ci.auto_merge.validate_pr_title_matches_vikunja")
@patch("devx.ci.auto_merge.GiteaClient") @patch("devx.ci.auto_merge.GiteaClient")
def test_merge_behind_master_rebases(self, mock_client_cls: MagicMock, tmp_path, monkeypatch) -> None: # type: ignore[no-untyped-def] def test_merge_behind_master_rebases(
self, mock_client_cls: MagicMock, _mock_validate: MagicMock, tmp_path, monkeypatch
) -> None: # type: ignore[no-untyped-def]
monkeypatch.chdir(tmp_path) monkeypatch.chdir(tmp_path)
(tmp_path / ".taskid").write_text("DEVX-19\n") (tmp_path / ".taskid").write_text("DEVX-19\n")
@@ -267,9 +274,12 @@ class TestMain:
# Should have fetched, rebased, and pushed # Should have fetched, rebased, and pushed
assert mock_run.call_count == 5 # config name, config email, fetch, rebase, push assert mock_run.call_count == 5 # config name, config email, fetch, rebase, push
@patch.dict("os.environ", {"REPO_TOKEN": "tok"}, clear=True) @patch.dict("os.environ", {"REPO_TOKEN": "tok", "VIKUNJA_TOKEN": "tok"}, clear=True)
@patch("devx.ci.auto_merge.validate_pr_title_matches_vikunja")
@patch("devx.ci.auto_merge.GiteaClient") @patch("devx.ci.auto_merge.GiteaClient")
def test_merge_failure_raises(self, mock_client_cls: MagicMock, tmp_path, monkeypatch) -> None: # type: ignore[no-untyped-def] def test_merge_failure_raises(
self, mock_client_cls: MagicMock, _mock_validate: MagicMock, tmp_path, monkeypatch
) -> None: # type: ignore[no-untyped-def]
monkeypatch.chdir(tmp_path) monkeypatch.chdir(tmp_path)
(tmp_path / ".taskid").write_text("DEVX-19\n") (tmp_path / ".taskid").write_text("DEVX-19\n")
@@ -288,9 +298,12 @@ class TestMain:
assert result.exit_code != 0 assert result.exit_code != 0
assert "Merge failed" in result.output assert "Merge failed" in result.output
@patch.dict("os.environ", {"REPO_TOKEN": "tok"}, clear=True) @patch.dict("os.environ", {"REPO_TOKEN": "tok", "VIKUNJA_TOKEN": "tok"}, clear=True)
@patch("devx.ci.auto_merge.validate_pr_title_matches_vikunja")
@patch("devx.ci.auto_merge.GiteaClient") @patch("devx.ci.auto_merge.GiteaClient")
def test_no_conventional_msg_raises(self, mock_client_cls: MagicMock, tmp_path, monkeypatch) -> None: # type: ignore[no-untyped-def] def test_no_conventional_msg_raises(
self, mock_client_cls: MagicMock, _mock_validate: MagicMock, tmp_path, monkeypatch
) -> None: # type: ignore[no-untyped-def]
"""When no conventional commit message is found in PR commits, raises.""" """When no conventional commit message is found in PR commits, raises."""
monkeypatch.chdir(tmp_path) monkeypatch.chdir(tmp_path)
(tmp_path / ".taskid").write_text("DEVX-19\n") (tmp_path / ".taskid").write_text("DEVX-19\n")
@@ -308,8 +321,31 @@ class TestMain:
assert "conventional commit" in result.output.lower() assert "conventional commit" in result.output.lower()
@patch.dict("os.environ", {"REPO_TOKEN": "tok"}, clear=True) @patch.dict("os.environ", {"REPO_TOKEN": "tok"}, clear=True)
def test_invalid_pr_number_raises(self, tmp_path, monkeypatch) -> None:
"""Non-integer PR number should raise."""
monkeypatch.chdir(tmp_path)
(tmp_path / ".taskid").write_text("DEVX-19\n")
runner = CliRunner()
result = runner.invoke(main, ["DEVX-19-fix", "DEVX-19: Test", "owner/repo", "not-a-number"])
assert result.exit_code != 0
assert "PR number must be an integer" in result.output
@patch.dict("os.environ", {"REPO_TOKEN": "tok"}, clear=True)
def test_invalid_repo_format_raises(self, tmp_path, monkeypatch) -> None:
"""Repo without owner/name should raise."""
monkeypatch.chdir(tmp_path)
(tmp_path / ".taskid").write_text("DEVX-19\n")
runner = CliRunner()
result = runner.invoke(main, ["DEVX-19-fix", "DEVX-19: Test", "invalidrepo", "7"])
assert result.exit_code != 0
assert "owner/name" in result.output
@patch.dict("os.environ", {"REPO_TOKEN": "tok", "VIKUNJA_TOKEN": "tok"}, clear=True)
@patch("devx.ci.auto_merge.validate_pr_title_matches_vikunja")
@patch("devx.ci.auto_merge.GiteaClient") @patch("devx.ci.auto_merge.GiteaClient")
def test_rebase_retry_failure_raises(self, mock_client_cls: MagicMock, tmp_path, monkeypatch) -> None: # type: ignore[no-untyped-def] def test_rebase_retry_failure_raises(
self, mock_client_cls: MagicMock, _mock_validate: MagicMock, tmp_path, monkeypatch
) -> None: # type: ignore[no-untyped-def]
"""When rebase retry also fails, raises with helpful message.""" """When rebase retry also fails, raises with helpful message."""
monkeypatch.chdir(tmp_path) monkeypatch.chdir(tmp_path)
(tmp_path / ".taskid").write_text("DEVX-19\n") (tmp_path / ".taskid").write_text("DEVX-19\n")
+14 -19
View File
@@ -67,7 +67,7 @@ class TestCheckTranslationSet:
trans_file.write_text(json.dumps({"Used": {"en": "Used"}, "Dead": {"en": "Dead"}})) trans_file.write_text(json.dumps({"Used": {"en": "Used"}, "Dead": {"en": "Dead"}}))
result = check_translations.check_translation_set("test", src_dir, trans_file) result = check_translations.check_translation_set("test", src_dir, trans_file)
assert any("Dead key" in w for w in result.warnings) assert any("Dead key" in e for e in result.errors)
def test_missing_language(self, tmp_path: Path) -> None: def test_missing_language(self, tmp_path: Path) -> None:
src_dir = tmp_path / "src" src_dir = tmp_path / "src"
@@ -77,7 +77,7 @@ class TestCheckTranslationSet:
trans_file.write_text(json.dumps({"Hello": {"en": "Hello"}})) trans_file.write_text(json.dumps({"Hello": {"en": "Hello"}}))
result = check_translations.check_translation_set("test", src_dir, trans_file) result = check_translations.check_translation_set("test", src_dir, trans_file)
assert any("Missing languages" in w for w in result.warnings) assert any("Missing languages" in e for e in result.errors)
def test_missing_translations_file(self, tmp_path: Path) -> None: def test_missing_translations_file(self, tmp_path: Path) -> None:
src_dir = tmp_path / "src" src_dir = tmp_path / "src"
@@ -108,23 +108,19 @@ class TestMain:
result = runner.invoke(check_translations.main, []) result = runner.invoke(check_translations.main, [])
assert result.exit_code == 0 assert result.exit_code == 0
def test_strict_fails_on_warnings(self, monkeypatch: pytest.MonkeyPatch) -> None: def test_errors_fail(self, monkeypatch: pytest.MonkeyPatch) -> None:
"""--strict should fail if there are missing language warnings.""" """Errors should cause exit code 1."""
warn_result = check_translations.TranslationCheckResult( error_result = check_translations.TranslationCheckResult(
name="devx", name="devx",
src_dir=Path("/tmp"), src_dir=Path("/tmp"),
trans_file=Path("/tmp/t.json"), trans_file=Path("/tmp/t.json"),
used_keys={"a"}, used_keys={"a"},
defined_keys={"a"}, defined_keys={"a"},
warnings=["Dead key: 'bar'"], errors=["Dead key: 'bar'"],
)
monkeypatch.setattr(
check_translations,
"check_translation_set",
lambda name, src, trans: warn_result,
) )
monkeypatch.setattr(check_translations, "check_translation_set", lambda name, src, trans: error_result)
runner = CliRunner() runner = CliRunner()
result = runner.invoke(check_translations.main, ["--strict"]) result = runner.invoke(check_translations.main, [])
assert result.exit_code == 1 assert result.exit_code == 1
def test_fails_on_errors(self, monkeypatch: pytest.MonkeyPatch) -> None: def test_fails_on_errors(self, monkeypatch: pytest.MonkeyPatch) -> None:
@@ -311,26 +307,25 @@ class TestCollectKeys:
assert "in_progress" in keys assert "in_progress" in keys
class TestMainWarnings: class TestMainCleanPass:
def test_passes_with_warnings(self, monkeypatch: pytest.MonkeyPatch) -> None: def test_passes_clean(self, monkeypatch: pytest.MonkeyPatch) -> None:
"""Should pass with exit code 0 and 'PASS with warnings' message.""" """Should pass with exit code 0 and 'PASS:' message when no errors."""
warn_result = check_translations.TranslationCheckResult( ok_result = check_translations.TranslationCheckResult(
name="devx", name="devx",
src_dir=Path("/tmp"), src_dir=Path("/tmp"),
trans_file=Path("/tmp/t.json"), trans_file=Path("/tmp/t.json"),
used_keys={"a"}, used_keys={"a"},
defined_keys={"a"}, defined_keys={"a"},
warnings=["Dead key: 'bar'"],
) )
monkeypatch.setattr( monkeypatch.setattr(
check_translations, check_translations,
"check_translation_set", "check_translation_set",
lambda name, src, trans: warn_result, lambda name, src, trans: ok_result,
) )
runner = CliRunner() runner = CliRunner()
result = runner.invoke(check_translations.main, []) result = runner.invoke(check_translations.main, [])
assert result.exit_code == 0 assert result.exit_code == 0
assert "PASS with warnings" in result.output assert "PASS:" in result.output
class TestI18nProjectTranslations: class TestI18nProjectTranslations:
+29
View File
@@ -120,6 +120,35 @@ class TestQueryRunners:
result = query_runners("https://api.example.com", "token", "owner", "repo") result = query_runners("https://api.example.com", "token", "owner", "repo")
assert result == 0 assert result == 0
@patch("devx.ci.discover_runners.requests.get")
def test_query_runners_403_no_warning(self, mock_get: MagicMock, capsys: pytest.CaptureFixture[str]) -> None:
"""403 on instance-level runners should not produce a warning (expected without admin scope)."""
responses = [
MagicMock(status_code=200, json=lambda: {"total_count": 2}),
MagicMock(status_code=200, json=lambda: {"total_count": 1}),
MagicMock(status_code=403, json=lambda: {"message": "forbidden"}),
]
mock_get.side_effect = responses
result = query_runners("https://api.example.com", "token", "owner", "repo")
assert result == 3
captured = capsys.readouterr()
assert "instance-level" not in captured.err
@patch("devx.ci.discover_runners.requests.get")
def test_instance_level_non_403_warns(self, mock_get: MagicMock, capsys: pytest.CaptureFixture[str]) -> None:
"""Non-200, non-403 status on instance-level runners should produce a warning."""
responses = [
MagicMock(status_code=200, json=lambda: {"total_count": 1}),
MagicMock(status_code=200, json=lambda: {"total_count": 1}),
MagicMock(status_code=500, json=lambda: {"message": "server error"}),
]
mock_get.side_effect = responses
result = query_runners("https://api.example.com", "token", "owner", "repo")
assert result == 2
captured = capsys.readouterr()
assert "instance-level" in captured.err
assert "500" in captured.err
class TestGetRunnerCount: class TestGetRunnerCount:
@patch("devx.ci.discover_runners.query_runners", return_value=5) @patch("devx.ci.discover_runners.query_runners", return_value=5)
+10
View File
@@ -240,6 +240,16 @@ class TestGithubEnv:
assert result.exit_code != 0 assert result.exit_code != 0
class TestRunnerIndexValidation:
def test_runner_index_zero_raises(self) -> None:
"""Runner index < 1 should raise."""
with patch("devx.molecule.distribute_molecule.discover_scenarios", return_value=["dummy"]):
runner = CliRunner()
result = runner.invoke(cli, ["--runner-index", "0", "--max-runners", "3"])
assert result.exit_code != 0
assert "out of range" in result.output
def test_main_module_block() -> None: def test_main_module_block() -> None:
import devx.molecule.distribute_molecule as dm import devx.molecule.distribute_molecule as dm
+9
View File
@@ -168,6 +168,15 @@ class TestCli:
assert result.exit_code == 0 assert result.exit_code == 0
assert "All molecule tests passed" in result.output assert "All molecule tests passed" in result.output
def test_invalid_pair_format_raises(self) -> None:
"""Pair with fewer than 2 parts should raise."""
from click.testing import CliRunner
runner = CliRunner()
result = runner.invoke(cli, ["invalid_no_pipe"])
assert result.exit_code != 0
assert "Invalid pair format" in result.output
def test_failure_exits_nonzero(self) -> None: def test_failure_exits_nonzero(self) -> None:
from click.testing import CliRunner from click.testing import CliRunner
+11 -14
View File
@@ -132,13 +132,12 @@ class TestMain:
assert "VIKUNJA_TOKEN" in result.output assert "VIKUNJA_TOKEN" in result.output
@patch.dict("os.environ", {"VIKUNJA_TOKEN": "tok"}) @patch.dict("os.environ", {"VIKUNJA_TOKEN": "tok"})
def test_no_task_id_non_release_warns(self) -> None: def test_no_task_id_non_release_fails(self) -> None:
"""Non-release commits without DEVX-N prefix should warn, not fail.""" """Non-release commits without DEVX-N prefix should fail."""
runner = CliRunner() runner = CliRunner()
result = runner.invoke(main, ["fix: resolve bug"]) result = runner.invoke(main, ["fix: resolve bug"])
assert result.exit_code == 0 assert result.exit_code != 0
assert "No task ID" in result.output assert "No task ID" in result.output
assert "Skipping" in result.output
@patch.dict("os.environ", {"VIKUNJA_TOKEN": "tok"}) @patch.dict("os.environ", {"VIKUNJA_TOKEN": "tok"})
def test_release_commit_without_task_id_skips(self) -> None: def test_release_commit_without_task_id_skips(self) -> None:
@@ -179,8 +178,8 @@ class TestMain:
@patch.dict("os.environ", {"VIKUNJA_TOKEN": "tok"}) @patch.dict("os.environ", {"VIKUNJA_TOKEN": "tok"})
@patch("devx.ci.post_merge.VikunjaClient") @patch("devx.ci.post_merge.VikunjaClient")
def test_post_comment_failure_warns(self, mock_client_cls: MagicMock) -> None: def test_post_comment_failure_fails(self, mock_client_cls: MagicMock) -> None:
"""Vikunja API errors should warn, not fail — the merge already succeeded.""" """Vikunja API errors should fail — the task was not updated."""
mock_client = MagicMock() mock_client = MagicMock()
mock_client.list_project_tasks.return_value = [ mock_client.list_project_tasks.return_value = [
{"id": 267, "identifier": "DEVX-20"}, {"id": 267, "identifier": "DEVX-20"},
@@ -189,14 +188,13 @@ class TestMain:
mock_client_cls.return_value = mock_client mock_client_cls.return_value = mock_client
runner = CliRunner() runner = CliRunner()
result = runner.invoke(main, ["DEVX-20: fix: bug"]) result = runner.invoke(main, ["DEVX-20: fix: bug"])
assert result.exit_code == 0 assert result.exit_code != 0
assert "Warning" in result.output assert "Vikunja API error" in result.output
assert "not updated" in result.output.lower()
@patch.dict("os.environ", {"VIKUNJA_TOKEN": "tok"}) @patch.dict("os.environ", {"VIKUNJA_TOKEN": "tok"})
@patch("devx.ci.post_merge.VikunjaClient") @patch("devx.ci.post_merge.VikunjaClient")
def test_mark_done_failure_warns(self, mock_client_cls: MagicMock) -> None: def test_mark_done_failure_fails(self, mock_client_cls: MagicMock) -> None:
"""Vikunja API errors should warn, not fail — the merge already succeeded.""" """Vikunja API errors should fail — the task was not updated."""
mock_client = MagicMock() mock_client = MagicMock()
mock_client.list_project_tasks.return_value = [ mock_client.list_project_tasks.return_value = [
{"id": 267, "identifier": "DEVX-20"}, {"id": 267, "identifier": "DEVX-20"},
@@ -206,9 +204,8 @@ class TestMain:
mock_client_cls.return_value = mock_client mock_client_cls.return_value = mock_client
runner = CliRunner() runner = CliRunner()
result = runner.invoke(main, ["DEVX-20: fix: bug"]) result = runner.invoke(main, ["DEVX-20: fix: bug"])
assert result.exit_code == 0 assert result.exit_code != 0
assert "Warning" in result.output assert "Vikunja API error" in result.output
assert "not updated" in result.output.lower()
class TestGetGitCommitMessage: class TestGetGitCommitMessage:
+11 -3
View File
@@ -84,6 +84,13 @@ class TestGetBumpedVersion:
with pytest.raises(click.ClickException): with pytest.raises(click.ClickException):
get_bumped_version() get_bumped_version()
@patch("devx.ci.release.run_cmd")
def test_invalid_version_format_raises(self, mock_run_cmd: MagicMock) -> None:
"""Non-semver version from git-cliff should raise."""
mock_run_cmd.return_value = MagicMock(returncode=0, stdout="not-a-version\n", stderr="")
with pytest.raises(click.ClickException, match="invalid version format"):
get_bumped_version()
class TestGetChangelog: class TestGetChangelog:
@patch("devx.ci.release.run_cmd") @patch("devx.ci.release.run_cmd")
@@ -384,7 +391,7 @@ class TestMain:
@patch("devx.ci.release.get_bumped_version", return_value="0.2.0") @patch("devx.ci.release.get_bumped_version", return_value="0.2.0")
@patch("devx.ci.release.has_unreleased_changes", return_value=True) @patch("devx.ci.release.has_unreleased_changes", return_value=True)
@patch("devx.ci.release.run_cmd") @patch("devx.ci.release.run_cmd")
def test_dry_run_empty_changelog( def test_dry_run_empty_changelog_fails(
self, self,
mock_run_cmd: MagicMock, mock_run_cmd: MagicMock,
mock_has: MagicMock, mock_has: MagicMock,
@@ -397,11 +404,12 @@ class TestMain:
mock_tag: MagicMock, mock_tag: MagicMock,
mock_user: MagicMock, mock_user: MagicMock,
) -> None: ) -> None:
"""Empty changelog should fail, not warn."""
mock_run_cmd.return_value = MagicMock(returncode=0, stdout="master\n", stderr="") mock_run_cmd.return_value = MagicMock(returncode=0, stdout="master\n", stderr="")
runner = CliRunner() runner = CliRunner()
result = runner.invoke(main, ["--dry-run"]) result = runner.invoke(main, ["--dry-run"])
assert result.exit_code == 0 assert result.exit_code != 0
assert "empty changelog" in result.output assert "empty changelog" in result.output.lower()
@patch.dict("os.environ", {}) @patch.dict("os.environ", {})
@patch("devx.ci.release.has_user_facing_changes", return_value=True) @patch("devx.ci.release.has_user_facing_changes", return_value=True)
+23 -8
View File
@@ -5,6 +5,7 @@ import json
from pathlib import Path from pathlib import Path
from unittest.mock import MagicMock, patch from unittest.mock import MagicMock, patch
import click
import pytest import pytest
from click.testing import CliRunner from click.testing import CliRunner
@@ -63,6 +64,22 @@ class TestLoadMapping:
with pytest.raises(FileNotFoundError): with pytest.raises(FileNotFoundError):
load_mapping() load_mapping()
def test_non_dict_mapping_raises(self, tmp_path: Path) -> None:
"""Non-dict mapping.json should raise."""
mapping_file = tmp_path / "mapping.json"
mapping_file.write_text('["not", "a", "dict"]')
with patch("devx.ci.sync_wiki.MAPPING_FILE", mapping_file):
with pytest.raises(click.ClickException, match="must be a dict"):
load_mapping()
def test_non_string_values_raise(self, tmp_path: Path) -> None:
"""Non-string values in mapping.json should raise."""
mapping_file = tmp_path / "mapping.json"
mapping_file.write_text('{"file.md": 123}')
with patch("devx.ci.sync_wiki.MAPPING_FILE", mapping_file):
with pytest.raises(click.ClickException, match="must be strings"):
load_mapping()
class TestReadDocContent: class TestReadDocContent:
def test_reads_file(self, tmp_path: Path) -> None: def test_reads_file(self, tmp_path: Path) -> None:
@@ -332,8 +349,8 @@ class TestMain:
@patch.dict("os.environ", {"REPO_TOKEN": "tok"}, clear=True) @patch.dict("os.environ", {"REPO_TOKEN": "tok"}, clear=True)
@patch("devx.ci.sync_wiki.GiteaClient") @patch("devx.ci.sync_wiki.GiteaClient")
def test_file_not_found_warning(self, mock_client_cls: MagicMock) -> None: def test_file_not_found_fails(self, mock_client_cls: MagicMock) -> None:
"""Test that missing doc files are skipped with a warning.""" """Test that missing doc files cause an error, not a warning."""
with patch("devx.ci.sync_wiki.MAPPING_FILE") as mock_mapping: with patch("devx.ci.sync_wiki.MAPPING_FILE") as mock_mapping:
mock_mapping.exists.return_value = True mock_mapping.exists.return_value = True
with patch("devx.ci.sync_wiki.load_mapping", return_value={"missing.md": "Missing"}): with patch("devx.ci.sync_wiki.load_mapping", return_value={"missing.md": "Missing"}):
@@ -341,14 +358,13 @@ class TestMain:
with patch("devx.ci.sync_wiki.list_wiki_pages", return_value={}): with patch("devx.ci.sync_wiki.list_wiki_pages", return_value={}):
runner = CliRunner() runner = CliRunner()
result = runner.invoke(main, ["--dry-run", "--repo", "owner/repo"]) result = runner.invoke(main, ["--dry-run", "--repo", "owner/repo"])
assert result.exit_code == 0 assert result.exit_code != 0
assert "not found" in result.output assert "not found" in result.output
assert "Skipped: 1" in result.output
@patch.dict("os.environ", {"REPO_TOKEN": "tok"}, clear=True) @patch.dict("os.environ", {"REPO_TOKEN": "tok"}, clear=True)
@patch("devx.ci.sync_wiki.GiteaClient") @patch("devx.ci.sync_wiki.GiteaClient")
def test_empty_doc_file_skipped(self, mock_client_cls: MagicMock) -> None: def test_empty_doc_file_fails(self, mock_client_cls: MagicMock) -> None:
"""Test that empty doc files are skipped with a warning.""" """Test that empty doc files cause an error, not a warning."""
with patch("devx.ci.sync_wiki.MAPPING_FILE") as mock_mapping: with patch("devx.ci.sync_wiki.MAPPING_FILE") as mock_mapping:
mock_mapping.exists.return_value = True mock_mapping.exists.return_value = True
with patch("devx.ci.sync_wiki.load_mapping", return_value={"empty.md": "Empty-Page"}): with patch("devx.ci.sync_wiki.load_mapping", return_value={"empty.md": "Empty-Page"}):
@@ -356,9 +372,8 @@ class TestMain:
with patch("devx.ci.sync_wiki.list_wiki_pages", return_value={}): with patch("devx.ci.sync_wiki.list_wiki_pages", return_value={}):
runner = CliRunner() runner = CliRunner()
result = runner.invoke(main, ["--dry-run", "--repo", "owner/repo"]) result = runner.invoke(main, ["--dry-run", "--repo", "owner/repo"])
assert result.exit_code == 0 assert result.exit_code != 0
assert "empty" in result.output.lower() assert "empty" in result.output.lower()
assert "Skipped: 1" in result.output
@patch.dict("os.environ", {"REPO_TOKEN": "tok"}, clear=True) @patch.dict("os.environ", {"REPO_TOKEN": "tok"}, clear=True)
@patch("devx.ci.sync_wiki.GiteaClient") @patch("devx.ci.sync_wiki.GiteaClient")