diff --git a/.workflows-lib b/.workflows-lib index 477e9de95..48f12e5a9 160000 --- a/.workflows-lib +++ b/.workflows-lib @@ -1 +1 @@ -Subproject commit 477e9de95572844bc1ddcb393c5e176a494ea088 +Subproject commit 48f12e5a9f482dfd1f43079ef4b18cdfee6211b8 diff --git a/agents/codex-405.md b/agents/codex-405.md new file mode 100644 index 000000000..005f3eee3 --- /dev/null +++ b/agents/codex-405.md @@ -0,0 +1 @@ + diff --git a/autofix_report_enriched.json b/autofix_report_enriched.json index 4d3d5ad3b..51b615184 100644 --- a/autofix_report_enriched.json +++ b/autofix_report_enriched.json @@ -1 +1 @@ -{"changed": true, "classification": {"total": 0, "new": 0, "allowed": 0}, "timestamp": "2025-12-31T17:57:55Z", "files": ["tests/scripts/test_keepalive_metrics_dashboard.py"]} \ No newline at end of file +{"changed": true, "classification": {"total": 0, "new": 0, "allowed": 0}, "timestamp": "2025-12-31T19:30:16Z", "files": ["tests/scripts/test_ledger_validate.py"]} \ No newline at end of file diff --git a/scripts/ci_cosmetic_repair.py b/scripts/ci_cosmetic_repair.py index 800153ce6..6c496a272 100644 --- a/scripts/ci_cosmetic_repair.py +++ b/scripts/ci_cosmetic_repair.py @@ -307,7 +307,7 @@ def stage_and_commit( summary: str, branch_suffix: str | None, ) -> str: - branch_suffix = branch_suffix or datetime.utcnow().strftime("%Y%m%d%H%M%S") + branch_suffix = branch_suffix or datetime.now(UTC).strftime("%Y%m%d%H%M%S") branch = f"{BRANCH_PREFIX}-{branch_suffix}" _run(["git", "checkout", "-B", branch], cwd=root) _run(["git", "add", *{str(p.relative_to(root)) for p in paths}], cwd=root) diff --git a/scripts/classify_test_failures.py b/scripts/classify_test_failures.py index e25eebdba..9a603a347 100644 --- a/scripts/classify_test_failures.py +++ b/scripts/classify_test_failures.py @@ -139,7 +139,9 @@ def classify_reports(paths: Iterable[str | Path]) -> dict[str, object]: root = tree.getroot() testcases = list(root.iter("testcase")) for testcase in testcases: - failure_node = testcase.find("failure") or testcase.find("error") + failure_node = testcase.find("failure") + if failure_node is None: + failure_node = testcase.find("error") if failure_node is None: continue marker_set = _extract_markers(testcase) diff --git a/tests/scripts/test_classify_test_failures.py b/tests/scripts/test_classify_test_failures.py new file mode 100644 index 000000000..828add7d2 --- /dev/null +++ b/tests/scripts/test_classify_test_failures.py @@ -0,0 +1,177 @@ +import json +import xml.etree.ElementTree as ET +from pathlib import Path + +from scripts import classify_test_failures + + +def _write_junit(tmp_path: Path, name: str, body: str) -> Path: + path = tmp_path / name + path.write_text( + '\n\n' f"{body}\n\n", + encoding="utf-8", + ) + return path + + +def test_classify_reports_ignores_missing_file() -> None: + summary = classify_test_failures.classify_reports(["missing-report.xml"]) + + assert summary["total_failures"] == 0 + assert summary["has_failures"] is False + assert summary["only_cosmetic"] is False + assert summary["cosmetic"] == [] + assert summary["runtime"] == [] + assert summary["unknown"] == [] + + +def test_classify_reports_handles_parse_error(tmp_path: Path) -> None: + report = tmp_path / "bad-report.xml" + report.write_text("", encoding="utf-8") + + summary = classify_test_failures.classify_reports([report]) + + assert summary["total_failures"] == 1 + assert summary["has_failures"] is True + assert summary["unknown"][0]["failure_type"] == "error" + assert "Unable to parse JUnit XML" in summary["unknown"][0]["message"] + assert summary["unknown"][0]["id"].startswith(":") + + +def test_classify_reports_classifies_markers_and_dedupes(tmp_path: Path) -> None: + body = """ + + + + + trace + + + + + + trace + + + + + + stack + + + + + + + + + + + """ + report = _write_junit(tmp_path, "report.xml", body) + + summary = classify_test_failures.classify_reports([report]) + + assert summary["total_failures"] == 4 + assert summary["has_failures"] is True + assert len(summary["runtime"]) == 2 + assert len(summary["cosmetic"]) == 1 + assert len(summary["unknown"]) == 1 + assert summary["cosmetic"][0]["failure_type"] == "error" + assert summary["runtime"][0]["message"].startswith("boom:") + + +def test_failure_message_and_test_id_helpers() -> None: + failure_case = ET.fromstring( + '' + 'trace' + "" + ) + message, failure_type = classify_test_failures._failure_message(failure_case) + + assert failure_type == "failure" + assert message == "boom: trace" + assert classify_test_failures._test_id(failure_case, Path("report.xml")) == "pkg.mod::test_it" + + error_case = ET.fromstring('stack') + message, failure_type = classify_test_failures._failure_message(error_case) + + assert failure_type == "error" + assert message == "stack" + assert classify_test_failures._test_id(error_case, Path("report.xml")) == "test_err" + + unnamed_case = ET.fromstring("") + assert ( + classify_test_failures._test_id(unnamed_case, Path("report.xml")) == "report.xml::testcase" + ) + empty_case = ET.fromstring("") + assert classify_test_failures._failure_message(empty_case) == ("", "failure") + + +def test_extract_markers_skips_blank_values() -> None: + testcase = ET.fromstring( + "" + "" + '' + '' + '' + "" + "" + ) + + markers = classify_test_failures._extract_markers(testcase) + + assert markers == {"runtime"} + + +def test_classify_reports_skips_non_failures(tmp_path: Path) -> None: + report = _write_junit( + tmp_path, + "report.xml", + """ + + + stack + + """, + ) + + summary = classify_test_failures.classify_reports([report]) + + assert summary["total_failures"] == 1 + + +def test_main_writes_output_file(tmp_path: Path, capsys, monkeypatch) -> None: + report = _write_junit( + tmp_path, + "report.xml", + """ + + + + """, + ) + output = tmp_path / "summary.json" + + monkeypatch.chdir(tmp_path) + status = classify_test_failures.main([report.name, "--output", str(output)]) + + assert status == 0 + assert output.exists() + payload = json.loads(output.read_text(encoding="utf-8")) + assert payload["has_failures"] is True + assert payload["total_failures"] == 1 + assert "cosmetic" in payload + + captured = capsys.readouterr() + assert '"total_failures": 1' in captured.out + + +def test_main_with_missing_report_prints_empty_summary(tmp_path: Path, capsys, monkeypatch) -> None: + monkeypatch.chdir(tmp_path) + + status = classify_test_failures.main(["missing-report.xml"]) + + assert status == 0 + captured = capsys.readouterr() + assert '"total_failures": 0' in captured.out diff --git a/tests/scripts/test_coverage_history_append.py b/tests/scripts/test_coverage_history_append.py index d3e0fb255..9f5c4880b 100644 --- a/tests/scripts/test_coverage_history_append.py +++ b/tests/scripts/test_coverage_history_append.py @@ -26,6 +26,14 @@ def test_load_existing_skips_invalid_lines(tmp_path: Path) -> None: assert records == [{"run_id": 1}, {"run_id": 2}] +def test_load_existing_returns_empty_when_missing(tmp_path: Path) -> None: + history_path = tmp_path / "missing.ndjson" + + records = coverage_history_append.load_existing(history_path) + + assert records == [] + + def test_main_replaces_matching_run_id_and_sorts( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: @@ -55,6 +63,62 @@ def test_main_replaces_matching_run_id_and_sorts( assert records[1]["coverage"] == 75.0 +def test_main_sorts_by_run_id_when_no_run_number( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + history_path = tmp_path / "history.ndjson" + record_path = tmp_path / "record.json" + + _write_ndjson( + history_path, + [ + {"run_id": 3, "coverage": 50.0}, + {"run_id": 1, "coverage": 45.0}, + ], + ) + record_path.write_text( + json.dumps({"run_id": 2, "coverage": 60.0}), + encoding="utf-8", + ) + + monkeypatch.setenv("HISTORY_PATH", str(history_path)) + monkeypatch.setenv("RECORD_PATH", str(record_path)) + + exit_code = coverage_history_append.main() + + assert exit_code == 0 + records = _read_ndjson(history_path) + assert [record["run_id"] for record in records] == [1, 2, 3] + + +def test_main_appends_record_without_run_id( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + history_path = tmp_path / "history.ndjson" + record_path = tmp_path / "record.json" + + _write_ndjson( + history_path, + [ + {"run_number": 1, "coverage": 50.0}, + {"run_number": 2, "coverage": 55.0}, + ], + ) + record_path.write_text( + json.dumps({"run_number": 3, "coverage": 60.0}), + encoding="utf-8", + ) + + monkeypatch.setenv("HISTORY_PATH", str(history_path)) + monkeypatch.setenv("RECORD_PATH", str(record_path)) + + exit_code = coverage_history_append.main() + + assert exit_code == 0 + records = _read_ndjson(history_path) + assert len(records) == 3 + + def test_main_skips_missing_record(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: history_path = tmp_path / "history.ndjson" record_path = tmp_path / "missing.json" diff --git a/tests/scripts/test_fix_cosmetic_aggregate.py b/tests/scripts/test_fix_cosmetic_aggregate.py new file mode 100644 index 000000000..c5250bfb6 --- /dev/null +++ b/tests/scripts/test_fix_cosmetic_aggregate.py @@ -0,0 +1,10 @@ +from pathlib import Path + +from scripts import fix_cosmetic_aggregate + + +def test_main_returns_when_target_missing(tmp_path, monkeypatch) -> None: + monkeypatch.setattr(fix_cosmetic_aggregate, "ROOT", Path(tmp_path)) + monkeypatch.setattr(fix_cosmetic_aggregate, "TARGET", Path("missing.py")) + + assert fix_cosmetic_aggregate.main() == 0 diff --git a/tests/scripts/test_ledger_migrate_base.py b/tests/scripts/test_ledger_migrate_base.py index daeef218b..ddbd3a113 100644 --- a/tests/scripts/test_ledger_migrate_base.py +++ b/tests/scripts/test_ledger_migrate_base.py @@ -1,4 +1,5 @@ import textwrap +from pathlib import Path import pytest import yaml @@ -37,6 +38,46 @@ def fake_run(args): assert ledger_migrate_base.detect_default_branch() == "main" +def test_detect_default_branch_ignores_blank_head_branch(monkeypatch) -> None: + def fake_run(args): + if args == ["remote", "show", "origin"]: + return " HEAD branch:\n" + if args == ["symbolic-ref", "--quiet", "refs/remotes/origin/HEAD"]: + return "refs/heads/dev\n" + raise AssertionError(f"unexpected args: {args}") + + monkeypatch.setattr(ledger_migrate_base, "_run_git", fake_run) + assert ledger_migrate_base.detect_default_branch() == "dev" + + +def test_detect_default_branch_returns_raw_ref(monkeypatch) -> None: + def fake_run(args): + if args == ["remote", "show", "origin"]: + raise ledger_migrate_base.MigrationError("no remote") + if args == ["symbolic-ref", "--quiet", "refs/remotes/origin/HEAD"]: + return "feature\n" + raise AssertionError(f"unexpected args: {args}") + + monkeypatch.setattr(ledger_migrate_base, "_run_git", fake_run) + assert ledger_migrate_base.detect_default_branch() == "feature" + + +def test_detect_default_branch_falls_back_to_current_branch(monkeypatch) -> None: + def fake_run(args): + if args == ["remote", "show", "origin"]: + raise ledger_migrate_base.MigrationError("no remote") + if args == ["symbolic-ref", "--quiet", "refs/remotes/origin/HEAD"]: + raise ledger_migrate_base.MigrationError("no origin head") + if args == ["rev-parse", "--abbrev-ref", "origin/HEAD"]: + return "origin/HEAD\n" + if args == ["symbolic-ref", "--quiet", "HEAD"]: + return "release\n" + raise AssertionError(f"unexpected args: {args}") + + monkeypatch.setattr(ledger_migrate_base, "_run_git", fake_run) + assert ledger_migrate_base.detect_default_branch() == "release" + + def test_detect_default_branch_falls_back_to_head(monkeypatch) -> None: def fake_run(args): if args == ["remote", "show", "origin"]: @@ -76,6 +117,16 @@ def fake_run(args): ledger_migrate_base.detect_default_branch() +def test_run_git_returns_stdout(monkeypatch) -> None: + def fake_run(args, check, capture_output, text): + assert args[0] == "git" + return type("Result", (), {"stdout": "ok\n"})() + + monkeypatch.setattr(ledger_migrate_base.subprocess, "run", fake_run) + + assert ledger_migrate_base._run_git(["status"]) == "ok\n" + + def test_load_ledger_requires_mapping(tmp_path) -> None: ledger_path = tmp_path / "issue-1-ledger.yml" ledger_path.write_text("- item\n", encoding="utf-8") @@ -145,6 +196,12 @@ def fake_run(args): ledger_migrate_base.find_repo_root() +def test_find_repo_root_returns_path(monkeypatch) -> None: + monkeypatch.setattr(ledger_migrate_base, "_run_git", lambda args: "/tmp/repo\n") + + assert ledger_migrate_base.find_repo_root() == Path("/tmp/repo") + + def test_discover_ledgers_lists_agents(tmp_path) -> None: agents_dir = tmp_path / ".agents" agents_dir.mkdir() @@ -156,6 +213,10 @@ def test_discover_ledgers_lists_agents(tmp_path) -> None: assert ledger_migrate_base.discover_ledgers(tmp_path) == [first, second] +def test_discover_ledgers_returns_empty_when_missing(tmp_path) -> None: + assert ledger_migrate_base.discover_ledgers(tmp_path) == [] + + def test_main_reports_no_ledgers(monkeypatch, capsys, tmp_path) -> None: monkeypatch.setattr(ledger_migrate_base, "find_repo_root", lambda: tmp_path) monkeypatch.setattr(ledger_migrate_base, "detect_default_branch", lambda _=None: "main") @@ -237,3 +298,26 @@ def fail_root(): assert exit_code == 2 err = capsys.readouterr().err assert "::error::boom" in err + + +def test_main_reports_no_updates(monkeypatch, capsys, tmp_path) -> None: + agents_dir = tmp_path / ".agents" + agents_dir.mkdir() + ledger_path = agents_dir / "issue-33-ledger.yml" + ledger_path.write_text("base: main\n", encoding="utf-8") + + monkeypatch.setattr(ledger_migrate_base, "find_repo_root", lambda: tmp_path) + monkeypatch.setattr(ledger_migrate_base, "detect_default_branch", lambda _=None: "main") + monkeypatch.setattr( + ledger_migrate_base, + "migrate_ledger", + lambda path, default_branch, check: ledger_migrate_base.LedgerResult( + path=path, previous="main", updated="main", changed=False + ), + ) + + exit_code = ledger_migrate_base.main([]) + + assert exit_code == 0 + out = capsys.readouterr().out + assert "Ledgers already matched the default branch; no updates written." in out diff --git a/tests/scripts/test_ledger_validate.py b/tests/scripts/test_ledger_validate.py index 34909553d..cdac52454 100644 --- a/tests/scripts/test_ledger_validate.py +++ b/tests/scripts/test_ledger_validate.py @@ -161,6 +161,27 @@ def test_validate_timestamp_formats_are_checked(tmp_path: Path, monkeypatch) -> assert "tasks[1].finished_at is not a valid timestamp" in errors[0] +def test_validate_timestamp_rejects_non_iso_format(tmp_path: Path, monkeypatch) -> None: + ledger_validate = _load_module(monkeypatch, tmp_path) + + errors = ledger_validate._validate_timestamp( + "2024-01-01", + field="started_at", + path="tasks[0]", + ) + + assert errors == [ + "tasks[0].started_at must be an ISO-8601 UTC timestamp (YYYY-MM-DDTHH:MM:SSZ)" + ] + + +def test_ensure_type_allows_none(tmp_path: Path, monkeypatch) -> None: + ledger_validate = _load_module(monkeypatch, tmp_path) + + assert ledger_validate._ensure_type(None, str, allow_none=True) is True + assert ledger_validate._ensure_type(123, str) is False + + def test_commit_files_raises_for_unknown_commit(tmp_path: Path, monkeypatch) -> None: ledger_validate = _load_module(monkeypatch, tmp_path) @@ -174,6 +195,372 @@ def raise_called_process_error(*args, **kwargs): ledger_validate._commit_files("deadbeef") +def test_fetch_commit_succeeds_without_retry(tmp_path: Path, monkeypatch) -> None: + ledger_validate = _load_module(monkeypatch, tmp_path) + calls: list[list[str]] = [] + + def fake_check_call(args, stdout=None, stderr=None): + calls.append(args) + return 0 + + monkeypatch.setattr(ledger_validate.subprocess, "check_call", fake_check_call) + + assert ledger_validate._fetch_commit("abc1234") is True + assert calls == [ + [ + "git", + "fetch", + "--no-tags", + "--filter=blob:none", + "origin", + "abc1234", + ] + ] + + +def test_fetch_commit_retries_after_deepen(tmp_path: Path, monkeypatch) -> None: + ledger_validate = _load_module(monkeypatch, tmp_path) + calls: list[list[str]] = [] + + def fake_check_call(args, stdout=None, stderr=None): + calls.append(args) + if args[-1] == "abc1234" and len(calls) == 1: + raise subprocess.CalledProcessError(1, args) + return 0 + + monkeypatch.setattr(ledger_validate.subprocess, "check_call", fake_check_call) + + assert ledger_validate._fetch_commit("abc1234") is True + assert calls[0][-1] == "abc1234" + assert calls[-1][-1] == "abc1234" + + +def test_fetch_commit_continues_after_failed_retry(tmp_path: Path, monkeypatch) -> None: + ledger_validate = _load_module(monkeypatch, tmp_path) + calls: list[list[str]] = [] + + def fake_check_call(args, stdout=None, stderr=None): + calls.append(args) + if len(calls) == 1: + raise subprocess.CalledProcessError(1, args) + if args[-1] == "abc1234" and len(calls) == 3: + raise subprocess.CalledProcessError(1, args) + return 0 + + monkeypatch.setattr(ledger_validate.subprocess, "check_call", fake_check_call) + + assert ledger_validate._fetch_commit("abc1234") is True + assert calls[-1][-1] == "abc1234" + + +def test_commit_files_fetches_history(tmp_path: Path, monkeypatch) -> None: + ledger_validate = _load_module(monkeypatch, tmp_path) + calls = {"count": 0} + + def fake_check_output(args, text=True): + calls["count"] += 1 + if calls["count"] == 1: + raise subprocess.CalledProcessError(1, args) + return "first.txt\nsecond.txt\n" + + monkeypatch.setattr(ledger_validate.subprocess, "check_output", fake_check_output) + monkeypatch.setattr(ledger_validate, "_fetch_commit", lambda commit: True) + + assert ledger_validate._commit_files("abc1234") == ["first.txt", "second.txt"] + + +def test_commit_subject_fetches_history(tmp_path: Path, monkeypatch) -> None: + ledger_validate = _load_module(monkeypatch, tmp_path) + calls = {"count": 0} + + def fake_check_output(args, text=True): + calls["count"] += 1 + if calls["count"] == 1: + raise subprocess.CalledProcessError(1, args) + return "fix: subject" + + monkeypatch.setattr(ledger_validate.subprocess, "check_output", fake_check_output) + monkeypatch.setattr(ledger_validate, "_fetch_commit", lambda commit: True) + + assert ledger_validate._commit_subject("abc1234") == "fix: subject" + + +def test_commit_subject_raises_for_unknown_commit(tmp_path: Path, monkeypatch) -> None: + ledger_validate = _load_module(monkeypatch, tmp_path) + + def raise_called_process_error(*args, **kwargs): + raise subprocess.CalledProcessError(1, args[0]) + + monkeypatch.setattr(ledger_validate.subprocess, "check_output", raise_called_process_error) + monkeypatch.setattr(ledger_validate, "_fetch_commit", lambda commit: False) + + with pytest.raises(ledger_validate.LedgerError, match="unknown commit deadbeef"): + ledger_validate._commit_subject("deadbeef") + + +def test_validate_ledger_rejects_non_mapping_task(tmp_path: Path, monkeypatch) -> None: + ledger_validate = _load_module(monkeypatch, tmp_path) + ledger_path = tmp_path / "ledger.yml" + payload = { + "version": 1, + "issue": 1, + "base": "main", + "branch": "feature", + "tasks": ["not-a-mapping"], + } + ledger_path.write_text(yaml.safe_dump(payload), encoding="utf-8") + + errors = ledger_validate.validate_ledger(ledger_path) + + assert f"{ledger_path}: tasks[0] must be a mapping" in errors + + +def test_validate_task_commit_rules(tmp_path: Path, monkeypatch) -> None: + ledger_validate = _load_module(monkeypatch, tmp_path) + ledger_path = tmp_path / "ledger.yml" + + task = { + "id": "task-1", + "title": "Work", + "status": "done", + "notes": None, + "commit": None, + } + errors = ledger_validate._validate_task(task, index=0, seen_ids=set(), ledger_path=ledger_path) + + assert "tasks[0].commit is required when status is done" in errors + assert not any("notes must be a list" in error for error in errors) + + +def test_validate_task_invalid_fields(tmp_path: Path, monkeypatch) -> None: + ledger_validate = _load_module(monkeypatch, tmp_path) + ledger_path = tmp_path / "ledger.yml" + + task = { + "id": " ", + "title": "", + "status": "unknown", + "notes": ["ok", 1], + "commit": "bad", + } + + errors = ledger_validate._validate_task(task, index=0, seen_ids=set(), ledger_path=ledger_path) + + assert "tasks[0].id must be a non-empty string" in errors + assert "tasks[0].title must be a non-empty string" in errors + assert "tasks[0].status must be one of" in errors[2] + assert "tasks[0].notes must be a list of strings" in errors + assert "tasks[0].commit must be empty or a Git SHA" in errors + + +def test_validate_task_done_requires_valid_commit(tmp_path: Path, monkeypatch) -> None: + ledger_validate = _load_module(monkeypatch, tmp_path) + ledger_path = tmp_path / "ledger.yml" + + task = { + "id": "task-1", + "title": "Done", + "status": "done", + "commit": "nope", + } + + errors = ledger_validate._validate_task(task, index=0, seen_ids=set(), ledger_path=ledger_path) + + assert "tasks[0].commit must be a Git SHA (7-40 hex characters)" in errors + + +def test_validate_task_commit_has_no_files(tmp_path: Path, monkeypatch) -> None: + ledger_validate = _load_module(monkeypatch, tmp_path) + ledger_path = tmp_path / "ledger.yml" + + monkeypatch.setattr(ledger_validate, "_commit_files", lambda commit: []) + + task = { + "id": "task-1", + "title": "Ship it", + "status": "done", + "commit": "abcdef1", + } + + errors = ledger_validate._validate_task(task, index=0, seen_ids=set(), ledger_path=ledger_path) + + assert f"{ledger_path}: tasks[0].commit abcdef1 has no changed files" in errors + + +def test_validate_task_commit_type_and_duplicate_ids(tmp_path: Path, monkeypatch) -> None: + ledger_validate = _load_module(monkeypatch, tmp_path) + ledger_path = tmp_path / "ledger.yml" + seen_ids: set[str] = set() + + first = { + "id": "task-1", + "title": "First", + "status": "todo", + "commit": 123, + } + second = { + "id": "task-1", + "title": "Second", + "status": "todo", + } + + errors = ledger_validate._validate_task( + first, index=0, seen_ids=seen_ids, ledger_path=ledger_path + ) + errors += ledger_validate._validate_task( + second, index=1, seen_ids=seen_ids, ledger_path=ledger_path + ) + + assert "tasks[0].commit must be a string" in errors + assert "duplicate task id: task-1" in errors + + +def test_validate_task_handles_commit_errors(tmp_path: Path, monkeypatch) -> None: + ledger_validate = _load_module(monkeypatch, tmp_path) + ledger_validate.REPO_ROOT = tmp_path + ledger_path = tmp_path / "ledger.yml" + + def raise_commit_files(_commit): + raise ledger_validate.LedgerError("missing") + + monkeypatch.setattr(ledger_validate, "_commit_files", raise_commit_files) + + task = { + "id": "task-1", + "title": "Title", + "status": "done", + "commit": "abcdef1", + } + + errors = ledger_validate._validate_task(task, index=0, seen_ids=set(), ledger_path=ledger_path) + + assert f"{ledger_path}: tasks[0].commit abcdef1 not found in repository" in errors[0] + + +def test_validate_task_commit_subject_failure(tmp_path: Path, monkeypatch) -> None: + ledger_validate = _load_module(monkeypatch, tmp_path) + ledger_validate.REPO_ROOT = tmp_path + ledger_dir = tmp_path / ".agents" + ledger_dir.mkdir() + ledger_path = ledger_dir / "issue-1-ledger.yml" + + monkeypatch.setattr( + ledger_validate, "_commit_files", lambda commit: [".agents/issue-1-ledger.yml"] + ) + + def raise_subject(_commit): + raise ledger_validate.LedgerError("no subject") + + monkeypatch.setattr(ledger_validate, "_commit_subject", raise_subject) + + task = { + "id": "task-1", + "title": "Ship it", + "status": "done", + "commit": "abcdef1", + } + + errors = ledger_validate._validate_task(task, index=0, seen_ids=set(), ledger_path=ledger_path) + + assert any("not found in repository" in error for error in errors) + assert any("must include non-ledger changes" in error for error in errors) + + +def test_validate_task_allows_non_agents_files(tmp_path: Path, monkeypatch) -> None: + ledger_validate = _load_module(monkeypatch, tmp_path) + ledger_path = tmp_path / "ledger.yml" + + monkeypatch.setattr(ledger_validate, "_commit_files", lambda commit: ["src/app.py"]) + monkeypatch.setattr(ledger_validate, "_commit_subject", lambda commit: "feat: update") + + task = { + "id": "task-1", + "title": "Ship it", + "status": "done", + "commit": "abcdef1", + } + + errors = ledger_validate._validate_task(task, index=0, seen_ids=set(), ledger_path=ledger_path) + + assert errors == [] + + +def test_find_ledgers_respects_explicit_paths(tmp_path: Path, monkeypatch) -> None: + ledger_validate = _load_module(monkeypatch, tmp_path) + + ledgers = ledger_validate.find_ledgers([str(tmp_path / "one.yml"), str(tmp_path / "two.yml")]) + + assert ledgers == [tmp_path / "one.yml", tmp_path / "two.yml"] + + +def test_find_ledgers_returns_empty_when_missing(tmp_path: Path, monkeypatch) -> None: + ledger_validate = _load_module(monkeypatch, tmp_path) + + assert ledger_validate.find_ledgers([]) == [] + + +def test_main_reports_validated_ledgers(tmp_path: Path, monkeypatch, capsys) -> None: + ledger_validate = _load_module(monkeypatch, tmp_path) + ledger_path = tmp_path / "ledger.yml" + ledger_path.write_text( + yaml.safe_dump( + { + "version": 1, + "issue": 1, + "base": "main", + "branch": "feature", + "tasks": [ + {"id": "task-1", "title": "Ok", "status": "todo"}, + ], + } + ), + encoding="utf-8", + ) + + monkeypatch.setattr(ledger_validate, "find_ledgers", lambda paths: [ledger_path]) + + exit_code = ledger_validate.main([]) + + assert exit_code == 0 + assert f"Validated {ledger_path}" in capsys.readouterr().out + + +def test_main_reports_no_ledgers(tmp_path: Path, monkeypatch, capsys) -> None: + ledger_validate = _load_module(monkeypatch, tmp_path) + monkeypatch.setattr(ledger_validate, "find_ledgers", lambda paths: []) + + exit_code = ledger_validate.main([]) + + assert exit_code == 0 + assert "No ledger files found." in capsys.readouterr().out + + +def test_main_prints_errors_to_stderr(tmp_path: Path, monkeypatch, capsys) -> None: + ledger_validate = _load_module(monkeypatch, tmp_path) + ledger_path = tmp_path / "ledger.yml" + ledger_path.write_text( + yaml.safe_dump( + { + "version": 1, + "issue": "nope", + "base": "main", + "branch": "feature", + "tasks": [ + {"id": "task-1", "title": "Ok", "status": "todo"}, + ], + } + ), + encoding="utf-8", + ) + + monkeypatch.setattr(ledger_validate, "find_ledgers", lambda paths: [ledger_path]) + + exit_code = ledger_validate.main([]) + + assert exit_code == 1 + assert "issue must be an integer" in capsys.readouterr().err + + def test_main_json_output_includes_errors(tmp_path: Path, monkeypatch, capsys) -> None: ledger_validate = _load_module(monkeypatch, tmp_path) ledger_path = tmp_path / "ledger.yml" diff --git a/tests/scripts/test_mypy_return_autofix.py b/tests/scripts/test_mypy_return_autofix.py new file mode 100644 index 000000000..d0c037f82 --- /dev/null +++ b/tests/scripts/test_mypy_return_autofix.py @@ -0,0 +1,125 @@ +import ast +import textwrap +from pathlib import Path + +from scripts import mypy_return_autofix + + +def _expr(source: str) -> ast.AST: + return ast.parse(source).body[0].value + + +def test_is_str_like_variants() -> None: + assert mypy_return_autofix._is_str_like(_expr("'hello'"), set()) is True + assert mypy_return_autofix._is_str_like(_expr('f"{value}"'), set()) is True + assert mypy_return_autofix._is_str_like(_expr("name"), {"name"}) is True + assert mypy_return_autofix._is_str_like(_expr("str(123)"), set()) is True + assert mypy_return_autofix._is_str_like(_expr("'a'.upper()"), set()) is True + assert mypy_return_autofix._is_str_like(_expr("'-'.join(['a'])"), set()) is True + assert mypy_return_autofix._is_str_like(_expr("'hi {}'.format('x')"), set()) is True + assert mypy_return_autofix._is_str_like(_expr("missing"), set()) is False + + +def test_is_list_of_str_variants() -> None: + assert mypy_return_autofix._is_list_of_str(_expr("['a', 'b']"), set()) is True + assert mypy_return_autofix._is_list_of_str(_expr("[1, 2]"), set()) is False + assert mypy_return_autofix._is_list_of_str(_expr("names"), {"names"}) is True + + +def test_collect_string_vars_tracks_assignments() -> None: + module = ast.parse( + textwrap.dedent( + """\ + def sample(): + greeting = "hi" + alias = greeting + names = ["a", f"{greeting}"] + nums = [1] + first, second = ["x", "y"] + """ + ) + ) + func = module.body[0] + string_vars, list_vars = mypy_return_autofix._collect_string_vars(func.body) + + assert string_vars == {"greeting", "alias"} + assert list_vars == {"names", "nums"} + + +def test_process_function_updates_list_annotation() -> None: + source = textwrap.dedent( + """\ + def names() -> list[int]: + items = ["a"] + return items + """ + ) + module = ast.parse(source) + lines = source.splitlines() + func = module.body[0] + + changed = mypy_return_autofix._process_function(func, lines, set()) + + assert changed is True + assert "list[str]" in lines[0] + + +def test_process_function_no_annotation_no_change() -> None: + source = textwrap.dedent( + """\ + def value(): + return "hi" + """ + ) + module = ast.parse(source) + lines = source.splitlines() + func = module.body[0] + + changed = mypy_return_autofix._process_function(func, lines, set()) + + assert changed is False + + +def test_process_function_skips_bare_return() -> None: + source = textwrap.dedent( + """\ + def value() -> int: + return + """ + ) + module = ast.parse(source) + lines = source.splitlines() + func = module.body[0] + + changed = mypy_return_autofix._process_function(func, lines, set()) + + assert changed is False + + +def test_process_file_rewrites_annotation(tmp_path: Path) -> None: + path = tmp_path / "sample.py" + path.write_text( + textwrap.dedent( + """\ + def value() -> int: + return "hello" + """ + ), + encoding="utf-8", + ) + + assert mypy_return_autofix._process_file(path) is True + assert "-> str:" in path.read_text(encoding="utf-8") + + +def test_annotation_to_str_without_unparse(monkeypatch) -> None: + monkeypatch.delattr(ast, "unparse", raising=False) + + assert mypy_return_autofix._annotation_to_str(ast.Name(id="value")) == "" + + +def test_main_skips_missing_project_dirs(tmp_path: Path, monkeypatch) -> None: + monkeypatch.setattr(mypy_return_autofix, "ROOT", tmp_path) + monkeypatch.setattr(mypy_return_autofix, "PROJECT_DIRS", [Path("missing")]) + + assert mypy_return_autofix.main([]) == 0 diff --git a/tests/scripts/test_update_autofix_expectations.py b/tests/scripts/test_update_autofix_expectations.py new file mode 100644 index 000000000..b6a2c658e --- /dev/null +++ b/tests/scripts/test_update_autofix_expectations.py @@ -0,0 +1,63 @@ +from __future__ import annotations + +import types +from pathlib import Path + +from scripts import update_autofix_expectations + + +def test_update_constant_skips_missing_module_file(tmp_path: Path) -> None: + module = types.ModuleType("sample") + module.__file__ = str(tmp_path / "missing.py") + + def build() -> str: + return "value" + + module.build = build + target = update_autofix_expectations.AutofixTarget( + module="sample", + callable_name="build", + constant_name="EXPECTED", + ) + + assert update_autofix_expectations._update_constant(module, target) is False + + +def test_update_constant_no_matching_constant(tmp_path: Path) -> None: + module = types.ModuleType("sample") + module_path = tmp_path / "sample.py" + module_path.write_text("OTHER = 'value'\n", encoding="utf-8") + module.__file__ = str(module_path) + + def build() -> str: + return "new" + + module.build = build + target = update_autofix_expectations.AutofixTarget( + module="sample", + callable_name="build", + constant_name="EXPECTED", + ) + + assert update_autofix_expectations._update_constant(module, target) is False + assert module_path.read_text(encoding="utf-8") == "OTHER = 'value'\n" + + +def test_update_constant_rewrites_matching_constant(tmp_path: Path) -> None: + module = types.ModuleType("sample") + module_path = tmp_path / "sample.py" + module_path.write_text("EXPECTED = 'old'\nOTHER = 1\n", encoding="utf-8") + module.__file__ = str(module_path) + + def build() -> str: + return "new" + + module.build = build + target = update_autofix_expectations.AutofixTarget( + module="sample", + callable_name="build", + constant_name="EXPECTED", + ) + + assert update_autofix_expectations._update_constant(module, target) is True + assert "EXPECTED = 'new'" in module_path.read_text(encoding="utf-8") diff --git a/tests/scripts/test_workflow_health_check.py b/tests/scripts/test_workflow_health_check.py new file mode 100644 index 000000000..4f513a724 --- /dev/null +++ b/tests/scripts/test_workflow_health_check.py @@ -0,0 +1,173 @@ +"""Tests for workflow_health_check module (scripts coverage).""" + +from __future__ import annotations + +import json +from datetime import UTC, datetime +from pathlib import Path + +import pytest + +from scripts import workflow_health_check + + +def test_format_duration_branches() -> None: + """Exercise short, minute, and hour duration formats.""" + assert workflow_health_check.format_duration(45) == "45s" + assert workflow_health_check.format_duration(75) == "1m 15s" + assert workflow_health_check.format_duration(3725) == "1h 2m" + + +def test_get_recent_runs_skips_invalid_timestamp() -> None: + """Invalid timestamps should be ignored during recency filtering.""" + recent_time = datetime.now(UTC).isoformat() + runs = [ + {"verdict": "pass", "recorded_at": recent_time}, + {"verdict": "fail", "recorded_at": "not-a-timestamp"}, + {"verdict": "pass", "recorded_at": ""}, + ] + + recent = workflow_health_check.get_recent_runs(runs, days=7) + + assert recent == [{"verdict": "pass", "recorded_at": recent_time}] + + +def test_generate_report_writes_output(tmp_path: Path) -> None: + """Output file should be written when output_path is provided.""" + metrics_file = tmp_path / "metrics.ndjson" + metrics_file.write_text( + "\n".join( + [ + json.dumps( + { + "verdict": "pass", + "recorded_at": datetime.now(UTC).isoformat(), + } + ), + json.dumps( + { + "verdict": "fail", + "recorded_at": datetime.now(UTC).isoformat(), + "error": "timeout", + } + ), + ] + ) + + "\n" + ) + output_file = tmp_path / "report.json" + + report = workflow_health_check.generate_report(str(metrics_file), str(output_file)) + + assert output_file.exists() + saved = json.loads(output_file.read_text()) + assert saved["total_runs"] == 2 + assert saved == report + + +def test_load_workflow_runs_skips_missing_file(tmp_path: Path) -> None: + missing = tmp_path / "missing.ndjson" + + runs = workflow_health_check.load_workflow_runs(str(missing)) + + assert runs == [] + + +def test_load_workflow_runs_reads_nonempty_lines(tmp_path: Path) -> None: + metrics_file = tmp_path / "metrics.ndjson" + metrics_file.write_text( + "\n".join( + [ + json.dumps({"verdict": "pass"}), + "", + json.dumps({"status": "failure"}), + ] + ) + + "\n" + ) + + runs = workflow_health_check.load_workflow_runs(str(metrics_file)) + + assert runs == [{"verdict": "pass"}, {"status": "failure"}] + + +def test_calculate_success_rate_uses_verdict_or_status() -> None: + runs = [ + {"verdict": "pass"}, + {"status": "success"}, + {"verdict": "fail"}, + {"status": "failure"}, + ] + + assert workflow_health_check.calculate_success_rate(runs) == 50.0 + + +def test_calculate_success_rate_empty() -> None: + assert workflow_health_check.calculate_success_rate([]) == 0.0 + + +def test_analyze_failure_patterns_counts_reasons() -> None: + runs = [ + {"verdict": "pass"}, + {"verdict": "fail", "skip_reason": "flaky"}, + {"status": "failure", "error": "timeout"}, + {"status": "failure"}, + {"verdict": "fail", "skip_reason": "flaky"}, + ] + + patterns = workflow_health_check.analyze_failure_patterns(runs) + + assert patterns == {"flaky": 2, "timeout": 1, "unknown": 1} + + +def test_get_recent_runs_skips_older_runs() -> None: + old_time = datetime(2000, 1, 1, tzinfo=UTC).isoformat() + runs = [{"verdict": "pass", "recorded_at": old_time}] + + recent = workflow_health_check.get_recent_runs(runs, days=7) + + assert recent == [] + + +def test_main_successful_run_prints_summary( + tmp_path: Path, capsys, monkeypatch: pytest.MonkeyPatch +) -> None: + metrics_file = tmp_path / "metrics.ndjson" + metrics_file.write_text( + json.dumps( + { + "verdict": "pass", + "recorded_at": datetime.now(UTC).isoformat(), + } + ) + + "\n" + ) + monkeypatch.setenv("METRICS_PATH", str(metrics_file)) + monkeypatch.setenv("SUCCESS_THRESHOLD", "80") + + workflow_health_check.main() + + captured = capsys.readouterr() + assert "Workflow Health Report" in captured.out + assert "Overall success rate: 100.0%" in captured.out + + +def test_main_exits_below_threshold(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """Main should exit non-zero when recent success rate is below threshold.""" + metrics_file = tmp_path / "metrics.ndjson" + metrics_file.write_text( + json.dumps( + { + "verdict": "fail", + "recorded_at": datetime.now(UTC).isoformat(), + } + ) + + "\n" + ) + monkeypatch.setenv("METRICS_PATH", str(metrics_file)) + monkeypatch.setenv("SUCCESS_THRESHOLD", "50") + + with pytest.raises(SystemExit) as excinfo: + workflow_health_check.main() + + assert excinfo.value.code == 1 diff --git a/tests/test_workflow_validator.py b/tests/test_workflow_validator.py index 2f60baa41..2bf94999d 100644 --- a/tests/test_workflow_validator.py +++ b/tests/test_workflow_validator.py @@ -154,6 +154,20 @@ def test_detect_write_all(self) -> None: issues = check_permissions(workflow) assert len(issues) >= 1 + def test_detect_job_write_all(self) -> None: + """Test detection of job-level write-all permissions.""" + workflow = {"jobs": {"build": {"permissions": "write-all"}}} + + issues = check_permissions(workflow) + assert issues == ["Job build has write-all permissions"] + + def test_allows_contents_write(self) -> None: + """Test that contents: write does not raise a permission issue.""" + workflow = {"permissions": {"contents": "write"}, "jobs": {}} + + issues = check_permissions(workflow) + assert issues == [] + class TestValidateWorkflow: """Tests for validate_workflow function.""" @@ -202,6 +216,14 @@ def test_validate_bad_workflow(self, tmp_path: Path) -> None: assert len(results["missing_timeout"]) >= 1 assert len(results["permission_issues"]) >= 1 + def test_validate_invalid_yaml(self, tmp_path: Path) -> None: + """Test validation reports errors for invalid YAML.""" + workflow_file = tmp_path / "invalid.yml" + workflow_file.write_text("{{invalid yaml") + + results = validate_workflow(str(workflow_file)) + assert results["errors"] == [f"Failed to load workflow: {workflow_file}"] + class TestValidateAllWorkflows: """Tests for validate_all_workflows function.""" diff --git a/tests/workflows/test_autofix_pipeline_diverse.py b/tests/workflows/test_autofix_pipeline_diverse.py index c71233399..53a2daad3 100644 --- a/tests/workflows/test_autofix_pipeline_diverse.py +++ b/tests/workflows/test_autofix_pipeline_diverse.py @@ -125,6 +125,8 @@ def compute_expected_report_count() -> int: encoding="utf-8", ) + black_targets = (sample_module, automation_module, numpy_test, expectations_module) + commands = [ ( [ @@ -140,7 +142,6 @@ def compute_expected_report_count() -> int: ), ([sys.executable, "-m", "isort", str(sample_module)], (0,)), ([sys.executable, "-m", "docformatter", "-i", str(sample_module)], (0, 3)), - ([sys.executable, "-m", "black", str(repo_root)], (0,)), ( [ sys.executable, @@ -158,6 +159,9 @@ def compute_expected_report_count() -> int: for command, ok_codes in commands: _run(command, cwd=repo_root, ok_exit_codes=ok_codes) + for target in black_targets: + _run([sys.executable, "-m", "black", str(target)], cwd=repo_root) + monkeypatch.setattr(auto_type_hygiene, "ROOT", repo_root, raising=False) monkeypatch.setattr(auto_type_hygiene, "SRC_DIRS", [src_dir, tests_dir], raising=False) monkeypatch.setattr(auto_type_hygiene, "DRY_RUN", False, raising=False) @@ -223,7 +227,8 @@ def compute_expected_report_count() -> int: mypy_return_autofix.main() _run([sys.executable, "-m", "isort", str(sample_module)], cwd=repo_root) - _run([sys.executable, "-m", "black", str(repo_root)], cwd=repo_root) + for target in black_targets: + _run([sys.executable, "-m", "black", str(target)], cwd=repo_root) _run( [ sys.executable, @@ -238,7 +243,8 @@ def compute_expected_report_count() -> int: ) _run([sys.executable, "-m", "ruff", "check", str(repo_root)], cwd=repo_root) - _run([sys.executable, "-m", "black", "--check", str(repo_root)], cwd=repo_root) + for target in black_targets: + _run([sys.executable, "-m", "black", "--check", str(target)], cwd=repo_root) _run( [ sys.executable, diff --git a/tests/workflows/test_ci_cosmetic_repair.py b/tests/workflows/test_ci_cosmetic_repair.py index ed0fec4b0..3eae53c18 100644 --- a/tests/workflows/test_ci_cosmetic_repair.py +++ b/tests/workflows/test_ci_cosmetic_repair.py @@ -397,7 +397,7 @@ def fake_run(cmd, *, cwd=None): class FakeDateTime: @staticmethod - def utcnow() -> dt_datetime: + def now(tz=None) -> dt_datetime: return dt_datetime(2024, 1, 2, 3, 4, 5) monkeypatch.setattr(ci_cosmetic_repair, "_run", fake_run)