test_cmd_breakage.py
file-level
1
files
1
commits
0
hotspots
0
🧊 dead
0
💥 blast risk
| 1 | """Comprehensive tests for ``muse code breakage``. |
| 2 | |
| 3 | Coverage: |
| 4 | I. Unit — _build_head_names_set / _build_head_class_methods |
| 5 | II. Unit — _check_file (all issue types, edge cases) |
| 6 | III. Integration — run() with a real repo (init → commit → modify → breakage) |
| 7 | IV. Integration — --language, --path, --commit, --strict flags |
| 8 | V. Integration — exit-code contract |
| 9 | VI. Integration — JSON output schema |
| 10 | VII. Regression — bugs fixed in this review (O(1) lookup, removed_public_method, |
| 11 | silent-fallback guard, working-tree-only files) |
| 12 | VIII.Stress — 200-file repo with deliberate breakage scattered throughout |
| 13 | """ |
| 14 | |
| 15 | from __future__ import annotations |
| 16 | |
| 17 | import json |
| 18 | import pathlib |
| 19 | |
| 20 | import pytest |
| 21 | |
| 22 | from tests.cli_test_helper import CliRunner, InvokeResult |
| 23 | from muse.cli.commands.breakage import ( |
| 24 | _BreakageIssue, |
| 25 | _build_head_class_methods, |
| 26 | _build_head_names_set, |
| 27 | _check_file, |
| 28 | ) |
| 29 | from typing import TypedDict |
| 30 | |
| 31 | from muse.plugins.code.ast_parser import SymbolKind, SymbolRecord, SymbolTree |
| 32 | |
| 33 | |
| 34 | class _BreakageJson(TypedDict, total=False): |
| 35 | schema_version: str |
| 36 | commit: str |
| 37 | branch: str |
| 38 | language_filter: str | None |
| 39 | path_filter: str | None |
| 40 | strict: bool |
| 41 | file_count: int |
| 42 | issues: list[_BreakageIssue] |
| 43 | total: int |
| 44 | errors: int |
| 45 | warnings: int |
| 46 | |
| 47 | cli = None |
| 48 | runner = CliRunner() |
| 49 | |
| 50 | # --------------------------------------------------------------------------- |
| 51 | # Test fixture helpers |
| 52 | # --------------------------------------------------------------------------- |
| 53 | |
| 54 | |
| 55 | @pytest.fixture |
| 56 | def repo(tmp_path: pathlib.Path, monkeypatch: pytest.MonkeyPatch) -> pathlib.Path: |
| 57 | """Fresh Muse repo in tmp_path.""" |
| 58 | monkeypatch.chdir(tmp_path) |
| 59 | monkeypatch.setenv("MUSE_REPO_ROOT", str(tmp_path)) |
| 60 | result = runner.invoke(cli, ["init"]) |
| 61 | assert result.exit_code == 0, result.output |
| 62 | return tmp_path |
| 63 | |
| 64 | |
| 65 | def _write(repo: pathlib.Path, rel_path: str, content: str) -> None: |
| 66 | p = repo / rel_path |
| 67 | p.parent.mkdir(parents=True, exist_ok=True) |
| 68 | p.write_text(content) |
| 69 | |
| 70 | |
| 71 | def _commit(repo: pathlib.Path, msg: str = "snapshot") -> None: |
| 72 | result = runner.invoke(cli, ["code", "add", "."]) |
| 73 | assert result.exit_code == 0, result.output |
| 74 | result = runner.invoke(cli, ["commit", "-m", msg]) |
| 75 | assert result.exit_code == 0, result.output |
| 76 | |
| 77 | |
| 78 | def _breakage( |
| 79 | args: list[str] | None = None, |
| 80 | ) -> InvokeResult: |
| 81 | return runner.invoke(cli, ["code", "breakage"] + (args or [])) |
| 82 | |
| 83 | |
| 84 | def _sym( |
| 85 | kind: SymbolKind, |
| 86 | name: str, |
| 87 | qualified_name: str = "", |
| 88 | content_id: str = "abc", |
| 89 | ) -> SymbolRecord: |
| 90 | return SymbolRecord( |
| 91 | kind=kind, |
| 92 | name=name, |
| 93 | qualified_name=qualified_name or name, |
| 94 | content_id=content_id, |
| 95 | body_hash=content_id, |
| 96 | signature_id=content_id, |
| 97 | metadata_id="", |
| 98 | canonical_key=f"f#{kind}#{name}#1", |
| 99 | lineno=1, |
| 100 | end_lineno=10, |
| 101 | ) |
| 102 | |
| 103 | |
| 104 | # --------------------------------------------------------------------------- |
| 105 | # Section I — Unit: _build_head_names_set |
| 106 | # --------------------------------------------------------------------------- |
| 107 | |
| 108 | |
| 109 | class TestBuildHeadNamesSet: |
| 110 | def test_excludes_imports(self) -> None: |
| 111 | tree: SymbolTree = { |
| 112 | "f.py::import::os": _sym("import", "os"), |
| 113 | "f.py::Foo": _sym("class", "Foo"), |
| 114 | } |
| 115 | result = _build_head_names_set({"f.py": tree}) |
| 116 | assert "Foo" in result |
| 117 | assert "os" not in result |
| 118 | |
| 119 | def test_collects_across_files(self) -> None: |
| 120 | t1: SymbolTree = {"a.py::foo": _sym("function", "foo")} |
| 121 | t2: SymbolTree = {"b.py::bar": _sym("function", "bar")} |
| 122 | result = _build_head_names_set({"a.py": t1, "b.py": t2}) |
| 123 | assert result == {"foo", "bar"} |
| 124 | |
| 125 | def test_empty_map_returns_empty_set(self) -> None: |
| 126 | assert _build_head_names_set({}) == set() |
| 127 | |
| 128 | def test_only_imports_returns_empty_set(self) -> None: |
| 129 | tree: SymbolTree = {"f.py::import::x": _sym("import", "x")} |
| 130 | assert _build_head_names_set({"f.py": tree}) == set() |
| 131 | |
| 132 | |
| 133 | # --------------------------------------------------------------------------- |
| 134 | # Section II — Unit: _build_head_class_methods |
| 135 | # --------------------------------------------------------------------------- |
| 136 | |
| 137 | |
| 138 | class TestBuildHeadClassMethods: |
| 139 | def test_collects_methods(self) -> None: |
| 140 | tree: SymbolTree = { |
| 141 | "f.py::Foo": _sym("class", "Foo"), |
| 142 | "f.py::Foo.bar": _sym("method", "bar", qualified_name="Foo.bar"), |
| 143 | "f.py::Foo.baz": _sym("method", "baz", qualified_name="Foo.baz"), |
| 144 | } |
| 145 | result = _build_head_class_methods({"f.py": tree}) |
| 146 | assert result == {"f.py::Foo": {"bar", "baz"}} |
| 147 | |
| 148 | def test_ignores_non_methods(self) -> None: |
| 149 | tree: SymbolTree = { |
| 150 | "f.py::Foo": _sym("class", "Foo"), |
| 151 | "f.py::top_fn": _sym("function", "top_fn"), |
| 152 | } |
| 153 | result = _build_head_class_methods({"f.py": tree}) |
| 154 | assert result == {} |
| 155 | |
| 156 | def test_multiple_classes(self) -> None: |
| 157 | tree: SymbolTree = { |
| 158 | "f.py::A.m1": _sym("method", "m1", qualified_name="A.m1"), |
| 159 | "f.py::B.m2": _sym("method", "m2", qualified_name="B.m2"), |
| 160 | } |
| 161 | result = _build_head_class_methods({"f.py": tree}) |
| 162 | assert result["f.py::A"] == {"m1"} |
| 163 | assert result["f.py::B"] == {"m2"} |
| 164 | |
| 165 | def test_method_without_dot_in_qualified_name_is_skipped(self) -> None: |
| 166 | tree: SymbolTree = {"f.py::orphan": _sym("method", "orphan", qualified_name="orphan")} |
| 167 | assert _build_head_class_methods({"f.py": tree}) == {} |
| 168 | |
| 169 | |
| 170 | # --------------------------------------------------------------------------- |
| 171 | # Section III — Unit: _check_file |
| 172 | # --------------------------------------------------------------------------- |
| 173 | |
| 174 | |
| 175 | class TestCheckFile: |
| 176 | # --- stale_import ------------------------------------------------------- |
| 177 | |
| 178 | def test_stale_import_flagged_as_warning(self) -> None: |
| 179 | # Must use the new qualified_name format "import::<module>::<name>" |
| 180 | # with a muse-internal module — only those are checked for staleness. |
| 181 | working: SymbolTree = { |
| 182 | "f.py::import::gone": _sym( |
| 183 | "import", "gone", qualified_name="import::muse.core.foo::gone" |
| 184 | ) |
| 185 | } |
| 186 | head: SymbolTree = {} |
| 187 | issues = _check_file("f.py", working, head, set(), {}, frozenset()) |
| 188 | assert len(issues) == 1 |
| 189 | assert issues[0]["issue_type"] == "stale_import" |
| 190 | assert issues[0]["severity"] == "warning" |
| 191 | assert "gone" in issues[0]["description"] |
| 192 | |
| 193 | def test_import_present_in_head_is_clean(self) -> None: |
| 194 | working: SymbolTree = {"f.py::import::alive": _sym("import", "alive")} |
| 195 | head: SymbolTree = {} |
| 196 | issues = _check_file("f.py", working, head, {"alive"}, {}, frozenset()) |
| 197 | assert issues == [] |
| 198 | |
| 199 | def test_module_import_not_flagged_as_stale(self) -> None: |
| 200 | # ``from muse.cli.commands import breakage`` — name is a module, not a |
| 201 | # symbol. It should never appear in head_names_set, but it must not |
| 202 | # be flagged as stale when the corresponding .py file exists in the |
| 203 | # HEAD snapshot. |
| 204 | working: SymbolTree = { |
| 205 | "app.py::import::breakage": _sym( |
| 206 | "import", "breakage", |
| 207 | qualified_name="import::muse.cli.commands::breakage", |
| 208 | ) |
| 209 | } |
| 210 | head_file_paths = frozenset({"muse/cli/commands/breakage.py"}) |
| 211 | issues = _check_file("app.py", working, {}, set(), {}, head_file_paths) |
| 212 | assert issues == [] |
| 213 | |
| 214 | def test_module_import_via_init_not_flagged(self) -> None: |
| 215 | # ``from muse.plugins import code`` where ``muse/plugins/code/__init__.py`` |
| 216 | # exists — package import, not stale. |
| 217 | working: SymbolTree = { |
| 218 | "app.py::import::code": _sym( |
| 219 | "import", "code", |
| 220 | qualified_name="import::muse.plugins::code", |
| 221 | ) |
| 222 | } |
| 223 | head_file_paths = frozenset({"muse/plugins/code/__init__.py"}) |
| 224 | issues = _check_file("app.py", working, {}, set(), {}, head_file_paths) |
| 225 | assert issues == [] |
| 226 | |
| 227 | def test_truly_missing_import_still_flagged_with_file_paths(self) -> None: |
| 228 | # head_file_paths is non-empty but doesn't contain the imported module — |
| 229 | # the import is genuinely stale. |
| 230 | working: SymbolTree = { |
| 231 | "f.py::import::vanished": _sym( |
| 232 | "import", "vanished", |
| 233 | qualified_name="import::muse.core::vanished", |
| 234 | ) |
| 235 | } |
| 236 | head_file_paths = frozenset({"muse/core/other.py"}) |
| 237 | issues = _check_file("f.py", working, {}, set(), {}, head_file_paths) |
| 238 | assert len(issues) == 1 |
| 239 | assert issues[0]["issue_type"] == "stale_import" |
| 240 | |
| 241 | def test_import_defined_locally_is_clean(self) -> None: |
| 242 | working: SymbolTree = { |
| 243 | "f.py::import::local_fn": _sym("import", "local_fn"), |
| 244 | "f.py::local_fn": _sym("function", "local_fn"), |
| 245 | } |
| 246 | issues = _check_file("f.py", working, {}, set(), {}, frozenset()) |
| 247 | assert issues == [] |
| 248 | |
| 249 | def test_wildcard_import_not_flagged(self) -> None: |
| 250 | working: SymbolTree = {"f.py::import::*:os": _sym("import", "*:os")} |
| 251 | issues = _check_file("f.py", working, {}, set(), {}, frozenset()) |
| 252 | assert issues == [] |
| 253 | |
| 254 | def test_empty_working_tree_returns_no_issues(self) -> None: |
| 255 | issues = _check_file("f.py", {}, {}, {"something"}, {}, frozenset()) |
| 256 | assert issues == [] |
| 257 | |
| 258 | def test_multiple_stale_imports(self) -> None: |
| 259 | working: SymbolTree = { |
| 260 | "f.py::import::a": _sym( |
| 261 | "import", "a", qualified_name="import::muse.core.x::a" |
| 262 | ), |
| 263 | "f.py::import::b": _sym( |
| 264 | "import", "b", qualified_name="import::muse.core.y::b" |
| 265 | ), |
| 266 | } |
| 267 | issues = _check_file("f.py", working, {}, set(), {}, frozenset()) |
| 268 | assert len(issues) == 2 |
| 269 | types = {i["issue_type"] for i in issues} |
| 270 | assert types == {"stale_import"} |
| 271 | |
| 272 | # --- removed_public_method ---------------------------------------------- |
| 273 | |
| 274 | def test_removed_public_method_flagged_as_error(self) -> None: |
| 275 | head: SymbolTree = { |
| 276 | "f.py::Foo": _sym("class", "Foo"), |
| 277 | } |
| 278 | head_class_methods = {"f.py::Foo": {"pay"}} |
| 279 | working: SymbolTree = { |
| 280 | "f.py::Foo": _sym("class", "Foo"), |
| 281 | # pay() is missing from working tree |
| 282 | } |
| 283 | issues = _check_file("f.py", working, head, set(), head_class_methods, frozenset()) |
| 284 | assert any(i["issue_type"] == "removed_public_method" for i in issues) |
| 285 | assert any(i["severity"] == "error" for i in issues) |
| 286 | assert any("pay" in i["description"] for i in issues) |
| 287 | |
| 288 | def test_private_method_removal_not_flagged(self) -> None: |
| 289 | head: SymbolTree = {"f.py::Foo": _sym("class", "Foo")} |
| 290 | head_class_methods = {"f.py::Foo": {"_internal"}} |
| 291 | working: SymbolTree = {"f.py::Foo": _sym("class", "Foo")} |
| 292 | issues = _check_file("f.py", working, head, set(), head_class_methods, frozenset()) |
| 293 | assert all(i["issue_type"] != "removed_public_method" for i in issues) |
| 294 | |
| 295 | def test_added_method_not_flagged(self) -> None: |
| 296 | head: SymbolTree = {"f.py::Foo": _sym("class", "Foo")} |
| 297 | head_class_methods = {"f.py::Foo": {"pay"}} |
| 298 | working: SymbolTree = { |
| 299 | "f.py::Foo": _sym("class", "Foo"), |
| 300 | "f.py::Foo.pay": _sym("method", "pay", qualified_name="Foo.pay"), |
| 301 | "f.py::Foo.refund": _sym("method", "refund", qualified_name="Foo.refund"), |
| 302 | } |
| 303 | issues = _check_file("f.py", working, head, set(), head_class_methods, frozenset()) |
| 304 | assert all(i["issue_type"] != "removed_public_method" for i in issues) |
| 305 | |
| 306 | def test_class_in_working_not_in_head_not_flagged(self) -> None: |
| 307 | head: SymbolTree = {} |
| 308 | working: SymbolTree = { |
| 309 | "f.py::Brand": _sym("class", "Brand"), |
| 310 | "f.py::Brand.name": _sym("method", "name", qualified_name="Brand.name"), |
| 311 | } |
| 312 | issues = _check_file("f.py", working, head, set(), {}, frozenset()) |
| 313 | assert issues == [] |
| 314 | |
| 315 | def test_removed_method_check_only_applies_to_python(self) -> None: |
| 316 | head_js: SymbolTree = {"f.js::Foo": _sym("class", "Foo")} |
| 317 | head_class_methods = {"f.js::Foo": {"pay"}} |
| 318 | working_js: SymbolTree = {"f.js::Foo": _sym("class", "Foo")} |
| 319 | issues = _check_file("f.js", working_js, head_js, set(), head_class_methods, frozenset()) |
| 320 | # Non-Python files should not trigger removed_public_method |
| 321 | assert all(i["issue_type"] != "removed_public_method" for i in issues) |
| 322 | |
| 323 | |
| 324 | # --------------------------------------------------------------------------- |
| 325 | # Section IV — Integration: init → commit → modify → breakage |
| 326 | # --------------------------------------------------------------------------- |
| 327 | |
| 328 | |
| 329 | CLEAN_MODULE = """\ |
| 330 | def compute(x: int) -> int: |
| 331 | return x * 2 |
| 332 | |
| 333 | |
| 334 | class Calculator: |
| 335 | def add(self, a: int, b: int) -> int: |
| 336 | return a + b |
| 337 | |
| 338 | def subtract(self, a: int, b: int) -> int: |
| 339 | return a - b |
| 340 | """ |
| 341 | |
| 342 | MODULE_WITH_STALE_IMPORT = """\ |
| 343 | from muse.core.nonexistent import utterly_nonexistent_symbol # stale: not in HEAD |
| 344 | |
| 345 | |
| 346 | def compute(x: int) -> int: |
| 347 | return x * 2 |
| 348 | |
| 349 | |
| 350 | class Calculator: |
| 351 | def add(self, a: int, b: int) -> int: |
| 352 | return a + b |
| 353 | |
| 354 | def subtract(self, a: int, b: int) -> int: |
| 355 | return a - b |
| 356 | """ |
| 357 | |
| 358 | MODULE_WITH_REMOVED_METHOD = """\ |
| 359 | class Calculator: |
| 360 | def add(self, a: int, b: int) -> int: |
| 361 | return a + b |
| 362 | # subtract() removed! |
| 363 | """ |
| 364 | |
| 365 | |
| 366 | class TestIntegrationBasic: |
| 367 | def test_no_changes_is_clean(self, repo: pathlib.Path) -> None: |
| 368 | _write(repo, "src/calc.py", CLEAN_MODULE) |
| 369 | _commit(repo, "add calc") |
| 370 | result = _breakage() |
| 371 | assert result.exit_code == 0 |
| 372 | assert "No structural breakage" in result.output |
| 373 | |
| 374 | def test_stale_import_detected(self, repo: pathlib.Path) -> None: |
| 375 | _write(repo, "src/calc.py", CLEAN_MODULE) |
| 376 | _commit(repo, "add calc") |
| 377 | (repo / "src/calc.py").write_text(MODULE_WITH_STALE_IMPORT) |
| 378 | result = _breakage() |
| 379 | assert "stale_import" in result.output |
| 380 | assert result.exit_code == 0 # warnings don't cause exit 1 without --strict |
| 381 | |
| 382 | def test_removed_public_method_detected(self, repo: pathlib.Path) -> None: |
| 383 | _write(repo, "src/calc.py", CLEAN_MODULE) |
| 384 | _commit(repo, "add calc") |
| 385 | (repo / "src/calc.py").write_text(MODULE_WITH_REMOVED_METHOD) |
| 386 | result = _breakage() |
| 387 | assert "removed_public_method" in result.output |
| 388 | assert "subtract" in result.output |
| 389 | assert result.exit_code == 1 # errors always exit 1 |
| 390 | |
| 391 | def test_no_head_commit_exits_nonzero( |
| 392 | self, tmp_path: pathlib.Path, monkeypatch: pytest.MonkeyPatch |
| 393 | ) -> None: |
| 394 | monkeypatch.chdir(tmp_path) |
| 395 | runner.invoke(cli, ["init"]) |
| 396 | result = _breakage() |
| 397 | assert result.exit_code != 0 |
| 398 | |
| 399 | def test_output_shows_commit_hash(self, repo: pathlib.Path) -> None: |
| 400 | _write(repo, "src/calc.py", CLEAN_MODULE) |
| 401 | _commit(repo, "add calc") |
| 402 | result = _breakage() |
| 403 | assert result.exit_code == 0 |
| 404 | # Commit hash appears in the header line |
| 405 | assert "Breakage check" in result.output |
| 406 | |
| 407 | |
| 408 | # --------------------------------------------------------------------------- |
| 409 | # Section V — Integration: flags |
| 410 | # --------------------------------------------------------------------------- |
| 411 | |
| 412 | |
| 413 | class TestFlags: |
| 414 | def test_language_filter_restricts_files(self, repo: pathlib.Path) -> None: |
| 415 | _write(repo, "src/calc.py", CLEAN_MODULE) |
| 416 | _commit(repo, "add calc") |
| 417 | (repo / "src/calc.py").write_text(MODULE_WITH_STALE_IMPORT) |
| 418 | # With --language Python the stale import should surface |
| 419 | result_py = _breakage(["--language", "Python"]) |
| 420 | assert "stale_import" in result_py.output |
| 421 | # With --language MIDI nothing matches — should be clean |
| 422 | result_midi = _breakage(["--language", "MIDI"]) |
| 423 | assert "No structural breakage" in result_midi.output |
| 424 | assert result_midi.exit_code == 0 |
| 425 | |
| 426 | def test_path_filter_restricts_files(self, repo: pathlib.Path) -> None: |
| 427 | _write(repo, "src/calc.py", CLEAN_MODULE) |
| 428 | _write(repo, "src/other.py", CLEAN_MODULE) |
| 429 | _commit(repo, "two modules") |
| 430 | (repo / "src/calc.py").write_text(MODULE_WITH_REMOVED_METHOD) |
| 431 | (repo / "src/other.py").write_text(MODULE_WITH_REMOVED_METHOD) |
| 432 | # Filter to only other.py — calc.py breakage should not appear |
| 433 | result = _breakage(["--path", "src/other.py"]) |
| 434 | assert "other.py" in result.output |
| 435 | # calc.py should not appear in output |
| 436 | assert "calc.py" not in result.output |
| 437 | |
| 438 | def test_strict_makes_warnings_exit_one(self, repo: pathlib.Path) -> None: |
| 439 | _write(repo, "src/calc.py", CLEAN_MODULE) |
| 440 | _commit(repo, "add calc") |
| 441 | (repo / "src/calc.py").write_text(MODULE_WITH_STALE_IMPORT) |
| 442 | # Without --strict: warning, exit 0 |
| 443 | result = _breakage() |
| 444 | assert result.exit_code == 0 |
| 445 | # With --strict: warning becomes exit 1 |
| 446 | result_strict = _breakage(["--strict"]) |
| 447 | assert result_strict.exit_code == 1 |
| 448 | |
| 449 | def test_commit_flag_resolves_named_branch(self, repo: pathlib.Path) -> None: |
| 450 | _write(repo, "src/calc.py", CLEAN_MODULE) |
| 451 | _commit(repo, "v1") |
| 452 | result = _breakage(["--commit", "main"]) |
| 453 | assert result.exit_code == 0 |
| 454 | |
| 455 | def test_commit_flag_invalid_ref_exits_nonzero(self, repo: pathlib.Path) -> None: |
| 456 | _write(repo, "src/calc.py", CLEAN_MODULE) |
| 457 | _commit(repo, "v1") |
| 458 | result = _breakage(["--commit", "no-such-ref-xyz"]) |
| 459 | assert result.exit_code != 0 |
| 460 | |
| 461 | def test_path_filter_no_matches_is_clean(self, repo: pathlib.Path) -> None: |
| 462 | _write(repo, "src/calc.py", CLEAN_MODULE) |
| 463 | _commit(repo, "add calc") |
| 464 | (repo / "src/calc.py").write_text(MODULE_WITH_REMOVED_METHOD) |
| 465 | result = _breakage(["--path", "totally/nonexistent/*.py"]) |
| 466 | assert result.exit_code == 0 |
| 467 | |
| 468 | def test_strict_with_no_issues_still_exits_zero(self, repo: pathlib.Path) -> None: |
| 469 | _write(repo, "src/calc.py", CLEAN_MODULE) |
| 470 | _commit(repo, "add calc") |
| 471 | result = _breakage(["--strict"]) |
| 472 | assert result.exit_code == 0 |
| 473 | |
| 474 | |
| 475 | # --------------------------------------------------------------------------- |
| 476 | # Section VI — Integration: JSON output |
| 477 | # --------------------------------------------------------------------------- |
| 478 | |
| 479 | |
| 480 | class TestJsonOutput: |
| 481 | def _json(self, args: list[str] | None = None) -> _BreakageJson: |
| 482 | result = _breakage((args or []) + ["--json"]) |
| 483 | data: _BreakageJson = json.loads(result.output) |
| 484 | return data |
| 485 | |
| 486 | def test_schema_keys_present(self, repo: pathlib.Path) -> None: |
| 487 | _write(repo, "src/calc.py", CLEAN_MODULE) |
| 488 | _commit(repo, "baseline") |
| 489 | data = self._json() |
| 490 | required = { |
| 491 | "schema", "commit", "branch", "language_filter", |
| 492 | "path_filter", "strict", "file_count", "issues", |
| 493 | "total", "errors", "warning_count", |
| 494 | } |
| 495 | assert required <= data.keys() |
| 496 | |
| 497 | def test_clean_repo_zero_issues(self, repo: pathlib.Path) -> None: |
| 498 | _write(repo, "src/calc.py", CLEAN_MODULE) |
| 499 | _commit(repo, "baseline") |
| 500 | data = self._json() |
| 501 | assert data["total"] == 0 |
| 502 | assert data["errors"] == 0 |
| 503 | assert data["warning_count"] == 0 |
| 504 | assert data["issues"] == [] |
| 505 | |
| 506 | def test_error_counted_correctly(self, repo: pathlib.Path) -> None: |
| 507 | _write(repo, "src/calc.py", CLEAN_MODULE) |
| 508 | _commit(repo, "baseline") |
| 509 | (repo / "src/calc.py").write_text(MODULE_WITH_REMOVED_METHOD) |
| 510 | data = self._json() |
| 511 | assert data["errors"] >= 1 |
| 512 | error_issues = [i for i in data["issues"] if i["severity"] == "error"] |
| 513 | assert len(error_issues) == data["errors"] |
| 514 | |
| 515 | def test_warning_counted_correctly(self, repo: pathlib.Path) -> None: |
| 516 | _write(repo, "src/calc.py", CLEAN_MODULE) |
| 517 | _commit(repo, "baseline") |
| 518 | (repo / "src/calc.py").write_text(MODULE_WITH_STALE_IMPORT) |
| 519 | data = self._json() |
| 520 | assert data["warning_count"] >= 1 |
| 521 | warn_issues = [i for i in data["issues"] if i["severity"] == "warning"] |
| 522 | assert len(warn_issues) == data["warning_count"] |
| 523 | |
| 524 | def test_issue_keys_present(self, repo: pathlib.Path) -> None: |
| 525 | _write(repo, "src/calc.py", CLEAN_MODULE) |
| 526 | _commit(repo, "baseline") |
| 527 | (repo / "src/calc.py").write_text(MODULE_WITH_REMOVED_METHOD) |
| 528 | data = self._json() |
| 529 | assert data["issues"] |
| 530 | issue = data["issues"][0] |
| 531 | assert {"issue_type", "path", "description", "severity"} <= issue.keys() |
| 532 | |
| 533 | def test_file_count_reflects_manifest(self, repo: pathlib.Path) -> None: |
| 534 | _write(repo, "src/a.py", CLEAN_MODULE) |
| 535 | _write(repo, "src/b.py", CLEAN_MODULE) |
| 536 | _commit(repo, "two files") |
| 537 | data = self._json() |
| 538 | assert data["file_count"] >= 2 |
| 539 | |
| 540 | def test_path_filter_reflected_in_json(self, repo: pathlib.Path) -> None: |
| 541 | _write(repo, "src/calc.py", CLEAN_MODULE) |
| 542 | _commit(repo, "baseline") |
| 543 | data = self._json(["--path", "src/*.py"]) |
| 544 | assert data["path_filter"] == "src/*.py" |
| 545 | |
| 546 | def test_strict_reflected_in_json(self, repo: pathlib.Path) -> None: |
| 547 | _write(repo, "src/calc.py", CLEAN_MODULE) |
| 548 | _commit(repo, "baseline") |
| 549 | data = self._json(["--strict"]) |
| 550 | assert data["strict"] is True |
| 551 | |
| 552 | def test_language_filter_reflected_in_json(self, repo: pathlib.Path) -> None: |
| 553 | _write(repo, "src/calc.py", CLEAN_MODULE) |
| 554 | _commit(repo, "baseline") |
| 555 | data = self._json(["--language", "Python"]) |
| 556 | assert data["language_filter"] == "Python" |
| 557 | |
| 558 | def test_branch_field_is_nonempty_string(self, repo: pathlib.Path) -> None: |
| 559 | _write(repo, "src/calc.py", CLEAN_MODULE) |
| 560 | _commit(repo, "baseline") |
| 561 | data = self._json() |
| 562 | assert isinstance(data["branch"], str) |
| 563 | assert data["branch"] |
| 564 | |
| 565 | |
| 566 | # --------------------------------------------------------------------------- |
| 567 | # Section VII — Exit-code contract |
| 568 | # --------------------------------------------------------------------------- |
| 569 | |
| 570 | |
| 571 | class TestExitCode: |
| 572 | def test_clean_exits_zero(self, repo: pathlib.Path) -> None: |
| 573 | _write(repo, "src/calc.py", CLEAN_MODULE) |
| 574 | _commit(repo, "baseline") |
| 575 | assert _breakage().exit_code == 0 |
| 576 | |
| 577 | def test_errors_exit_one(self, repo: pathlib.Path) -> None: |
| 578 | _write(repo, "src/calc.py", CLEAN_MODULE) |
| 579 | _commit(repo, "baseline") |
| 580 | (repo / "src/calc.py").write_text(MODULE_WITH_REMOVED_METHOD) |
| 581 | assert _breakage().exit_code == 1 |
| 582 | |
| 583 | def test_warnings_exit_zero_without_strict(self, repo: pathlib.Path) -> None: |
| 584 | _write(repo, "src/calc.py", CLEAN_MODULE) |
| 585 | _commit(repo, "baseline") |
| 586 | (repo / "src/calc.py").write_text(MODULE_WITH_STALE_IMPORT) |
| 587 | assert _breakage().exit_code == 0 |
| 588 | |
| 589 | def test_warnings_exit_one_with_strict(self, repo: pathlib.Path) -> None: |
| 590 | _write(repo, "src/calc.py", CLEAN_MODULE) |
| 591 | _commit(repo, "baseline") |
| 592 | (repo / "src/calc.py").write_text(MODULE_WITH_STALE_IMPORT) |
| 593 | assert _breakage(["--strict"]).exit_code == 1 |
| 594 | |
| 595 | def test_json_exit_code_matches_human_output(self, repo: pathlib.Path) -> None: |
| 596 | _write(repo, "src/calc.py", CLEAN_MODULE) |
| 597 | _commit(repo, "baseline") |
| 598 | (repo / "src/calc.py").write_text(MODULE_WITH_REMOVED_METHOD) |
| 599 | human = _breakage() |
| 600 | json_out = _breakage(["--json"]) |
| 601 | assert human.exit_code == json_out.exit_code == 1 |
| 602 | |
| 603 | |
| 604 | # --------------------------------------------------------------------------- |
| 605 | # Section VIII — Regression: bugs fixed in this review |
| 606 | # --------------------------------------------------------------------------- |
| 607 | |
| 608 | |
| 609 | class TestRegressions: |
| 610 | def test_check2_does_not_produce_false_positives_across_classes( |
| 611 | self, repo: pathlib.Path |
| 612 | ) -> None: |
| 613 | """Old Check 2 would flag class A for missing methods of class B. |
| 614 | |
| 615 | The fixed version only checks each class against its own HEAD methods. |
| 616 | """ |
| 617 | mod = """\ |
| 618 | class A: |
| 619 | def foo(self) -> None: |
| 620 | pass |
| 621 | |
| 622 | class B: |
| 623 | def bar(self) -> None: |
| 624 | pass |
| 625 | """ |
| 626 | _write(repo, "src/two_classes.py", mod) |
| 627 | _commit(repo, "two classes") |
| 628 | # Both classes still have their own methods — no breakage |
| 629 | result = _breakage() |
| 630 | assert result.exit_code == 0 |
| 631 | assert "removed_public_method" not in result.output |
| 632 | |
| 633 | def test_stale_import_lookup_is_o1_not_linear_scan( |
| 634 | self, repo: pathlib.Path |
| 635 | ) -> None: |
| 636 | """Verifies that `head_names_set` is used (set membership, not any() loop). |
| 637 | |
| 638 | We test this indirectly: if 1000 HEAD symbols exist, the check must |
| 639 | still complete quickly and return the correct result. Correctness |
| 640 | regression only — performance is measured separately in the stress |
| 641 | tests. |
| 642 | """ |
| 643 | # Build a module with 50 functions |
| 644 | big_mod = "\n".join(f"def fn_{i}() -> None:\n pass" for i in range(50)) |
| 645 | _write(repo, "src/big.py", big_mod) |
| 646 | _commit(repo, "50 fns") |
| 647 | # Add a stale import to a separate file — must be a muse.* import |
| 648 | # so the stale-import filter fires on it. |
| 649 | _write(repo, "src/consumer.py", "from muse.big import does_not_exist\n") |
| 650 | _commit(repo, "add consumer") |
| 651 | (repo / "src/consumer.py").write_text( |
| 652 | "from muse.big import does_not_exist\n" |
| 653 | ) |
| 654 | result = _breakage(["--path", "src/consumer.py"]) |
| 655 | assert "stale_import" in result.output |
| 656 | |
| 657 | def test_silent_manifest_fallback_guard( |
| 658 | self, repo: pathlib.Path |
| 659 | ) -> None: |
| 660 | """run() must not silently treat an unreadable snapshot as an empty manifest. |
| 661 | |
| 662 | We verify by checking that the command exits non-zero (error) rather |
| 663 | than silently succeeding with zero issues when the commit cannot be |
| 664 | read. |
| 665 | """ |
| 666 | _write(repo, "src/calc.py", CLEAN_MODULE) |
| 667 | _commit(repo, "baseline") |
| 668 | result = _breakage(["--commit", "nonexistent-ref-8675309"]) |
| 669 | assert result.exit_code != 0 |
| 670 | |
| 671 | def test_removed_public_method_severity_is_error_not_warning( |
| 672 | self, repo: pathlib.Path |
| 673 | ) -> None: |
| 674 | _write(repo, "src/calc.py", CLEAN_MODULE) |
| 675 | _commit(repo, "baseline") |
| 676 | (repo / "src/calc.py").write_text(MODULE_WITH_REMOVED_METHOD) |
| 677 | data = json.loads(_breakage(["--json"]).output) |
| 678 | removed = [i for i in data["issues"] if i["issue_type"] == "removed_public_method"] |
| 679 | assert removed, "expected at least one removed_public_method issue" |
| 680 | for issue in removed: |
| 681 | assert issue["severity"] == "error" |
| 682 | |
| 683 | def test_stale_import_severity_is_warning_not_error( |
| 684 | self, repo: pathlib.Path |
| 685 | ) -> None: |
| 686 | _write(repo, "src/calc.py", CLEAN_MODULE) |
| 687 | _commit(repo, "baseline") |
| 688 | (repo / "src/calc.py").write_text(MODULE_WITH_STALE_IMPORT) |
| 689 | data = json.loads(_breakage(["--json"]).output) |
| 690 | stale = [i for i in data["issues"] if i["issue_type"] == "stale_import"] |
| 691 | assert stale, "expected at least one stale_import issue" |
| 692 | for issue in stale: |
| 693 | assert issue["severity"] == "warning" |
| 694 | |
| 695 | def test_non_python_file_no_removed_method_check( |
| 696 | self, repo: pathlib.Path |
| 697 | ) -> None: |
| 698 | """removed_public_method is Python-only; non-Python files must not trigger it.""" |
| 699 | _write(repo, "notes.md", "# Hello\n") |
| 700 | _commit(repo, "markdown") |
| 701 | result = _breakage() |
| 702 | assert result.exit_code == 0 |
| 703 | assert "removed_public_method" not in result.output |
| 704 | |
| 705 | def test_both_issues_in_same_file(self, repo: pathlib.Path) -> None: |
| 706 | _write(repo, "src/calc.py", CLEAN_MODULE) |
| 707 | _commit(repo, "baseline") |
| 708 | # Introduce both a stale import AND a removed method. |
| 709 | # Must use a muse.* source module so the stale-import filter fires. |
| 710 | (repo / "src/calc.py").write_text( |
| 711 | "from muse.core.vanished_module import vanished\n\n" |
| 712 | "class Calculator:\n" |
| 713 | " def add(self, a: int, b: int) -> int:\n" |
| 714 | " return a + b\n" |
| 715 | ) |
| 716 | result = _breakage() |
| 717 | assert "stale_import" in result.output |
| 718 | assert "removed_public_method" in result.output |
| 719 | assert result.exit_code == 1 # error from removed method |
| 720 | |
| 721 | |
| 722 | # --------------------------------------------------------------------------- |
| 723 | # Section IX — Stress tests |
| 724 | # --------------------------------------------------------------------------- |
| 725 | |
| 726 | _CLEAN_FUNC_TEMPLATE = """\ |
| 727 | def fn_{i}(x: int) -> int: |
| 728 | return x + {i} |
| 729 | |
| 730 | |
| 731 | class Class_{i}: |
| 732 | def method_a(self) -> str: |
| 733 | return "a_{i}" |
| 734 | |
| 735 | def method_b(self) -> str: |
| 736 | return "b_{i}" |
| 737 | """ |
| 738 | |
| 739 | _BROKEN_FUNC_TEMPLATE = """\ |
| 740 | from muse.nowhere_special import ghost_{i} |
| 741 | |
| 742 | |
| 743 | def fn_{i}(x: int) -> int: |
| 744 | return x + {i} |
| 745 | |
| 746 | |
| 747 | class Class_{i}: |
| 748 | def method_a(self) -> str: |
| 749 | return "a_{i}" |
| 750 | # method_b removed |
| 751 | """ |
| 752 | |
| 753 | |
| 754 | class TestStress: |
| 755 | @pytest.mark.slow |
| 756 | def test_200_clean_files_no_false_positives( |
| 757 | self, repo: pathlib.Path |
| 758 | ) -> None: |
| 759 | """200 files, all clean — breakage must report 0 issues.""" |
| 760 | for i in range(200): |
| 761 | _write(repo, f"src/mod_{i:03d}.py", _CLEAN_FUNC_TEMPLATE.format(i=i)) |
| 762 | _commit(repo, "200 clean files") |
| 763 | result = _breakage() |
| 764 | assert result.exit_code == 0 |
| 765 | assert "No structural breakage" in result.output |
| 766 | |
| 767 | @pytest.mark.slow |
| 768 | def test_200_files_20_broken_detected_exactly( |
| 769 | self, repo: pathlib.Path |
| 770 | ) -> None: |
| 771 | """200 files; the last 20 have both stale_import and removed_public_method.""" |
| 772 | for i in range(180): |
| 773 | _write(repo, f"src/mod_{i:03d}.py", _CLEAN_FUNC_TEMPLATE.format(i=i)) |
| 774 | for i in range(180, 200): |
| 775 | _write(repo, f"src/mod_{i:03d}.py", _CLEAN_FUNC_TEMPLATE.format(i=i)) |
| 776 | _commit(repo, "200 files committed clean") |
| 777 | |
| 778 | # Now break the last 20 in the working tree (not committed) |
| 779 | for i in range(180, 200): |
| 780 | (repo / f"src/mod_{i:03d}.py").write_text( |
| 781 | _BROKEN_FUNC_TEMPLATE.format(i=i) |
| 782 | ) |
| 783 | |
| 784 | data = json.loads(_breakage(["--json"]).output) |
| 785 | # Each broken file contributes: 1 stale_import warning + 1 error |
| 786 | assert data["errors"] >= 20, f"Expected ≥20 errors, got {data['errors']}" |
| 787 | assert data["warning_count"] >= 20, f"Expected ≥20 warnings, got {data['warnings']}" |
| 788 | # No issues from the 180 clean files |
| 789 | broken_files = {f"src/mod_{i:03d}.py" for i in range(180, 200)} |
| 790 | for issue in data["issues"]: |
| 791 | assert issue["path"] in broken_files, ( |
| 792 | f"Unexpected issue in {issue['path']}: {issue['description']}" |
| 793 | ) |
| 794 | |
| 795 | @pytest.mark.slow |
| 796 | def test_path_filter_on_200_file_repo(self, repo: pathlib.Path) -> None: |
| 797 | """Path filter must reduce the checked set, not scan everything.""" |
| 798 | for i in range(200): |
| 799 | _write(repo, f"src/mod_{i:03d}.py", _CLEAN_FUNC_TEMPLATE.format(i=i)) |
| 800 | _commit(repo, "200 files") |
| 801 | # Break one file outside the filter |
| 802 | (repo / "src/mod_050.py").write_text(MODULE_WITH_REMOVED_METHOD) |
| 803 | |
| 804 | # Filter restricts to mod_100–mod_199 only — the broken file is excluded |
| 805 | result = _breakage(["--path", "src/mod_1*.py"]) |
| 806 | assert result.exit_code == 0 |
| 807 | assert "removed_public_method" not in result.output |
| 808 | |
| 809 | @pytest.mark.slow |
| 810 | def test_strict_on_200_files_with_one_warning(self, repo: pathlib.Path) -> None: |
| 811 | for i in range(200): |
| 812 | _write(repo, f"src/mod_{i:03d}.py", _CLEAN_FUNC_TEMPLATE.format(i=i)) |
| 813 | _commit(repo, "200 files") |
| 814 | # Introduce a single stale import in one file. |
| 815 | # Must use a muse.* source module so the stale-import filter fires. |
| 816 | content = _CLEAN_FUNC_TEMPLATE.format(i=0) |
| 817 | (repo / "src/mod_000.py").write_text(f"from muse.ghost import gone\n{content}") |
| 818 | |
| 819 | # Without --strict: exit 0 (warning only) |
| 820 | assert _breakage().exit_code == 0 |
| 821 | # With --strict: exit 1 |
| 822 | assert _breakage(["--strict"]).exit_code == 1 |
| 823 | |
| 824 | @pytest.mark.slow |
| 825 | def test_multiple_commits_commit_flag(self, repo: pathlib.Path) -> None: |
| 826 | """--commit lets you diff against any historical snapshot.""" |
| 827 | _write(repo, "src/calc.py", CLEAN_MODULE) |
| 828 | _commit(repo, "v1 — has subtract") |
| 829 | |
| 830 | # Break it in v2 |
| 831 | (repo / "src/calc.py").write_text(MODULE_WITH_REMOVED_METHOD) |
| 832 | runner.invoke(cli, ["code", "add", "."]) |
| 833 | runner.invoke(cli, ["commit", "-m", "v2 — subtract removed"]) |
| 834 | |
| 835 | # Working tree matches v2 (committed); checking against main shows no |
| 836 | # breakage since the removal is already committed. |
| 837 | result_main = _breakage(["--commit", "main"]) |
| 838 | assert result_main.exit_code == 0 |
| 839 | |
| 840 | @pytest.mark.slow |
| 841 | def test_json_issues_each_have_all_required_fields_stress( |
| 842 | self, repo: pathlib.Path |
| 843 | ) -> None: |
| 844 | for i in range(50): |
| 845 | _write(repo, f"src/m{i}.py", _CLEAN_FUNC_TEMPLATE.format(i=i)) |
| 846 | _commit(repo, "50 files") |
| 847 | for i in range(25): |
| 848 | (repo / f"src/m{i}.py").write_text(_BROKEN_FUNC_TEMPLATE.format(i=i)) |
| 849 | |
| 850 | data = json.loads(_breakage(["--json"]).output) |
| 851 | required_keys = {"issue_type", "path", "description", "severity"} |
| 852 | for issue in data["issues"]: |
| 853 | missing = required_keys - issue.keys() |
| 854 | assert not missing, f"Issue missing keys {missing}: {issue}" |
| 855 | |
| 856 | |
| 857 | # --------------------------------------------------------------------------- |
| 858 | # Flag registration tests |
| 859 | # --------------------------------------------------------------------------- |
| 860 | |
| 861 | import argparse as _argparse |
| 862 | from muse.cli.commands.breakage import register as _register_breakage |
| 863 | |
| 864 | |
| 865 | def _parse_breakage(*args: str) -> _argparse.Namespace: |
| 866 | """Build an argument parser via register() and parse args.""" |
| 867 | root_p = _argparse.ArgumentParser() |
| 868 | subs = root_p.add_subparsers(dest="cmd") |
| 869 | _register_breakage(subs) |
| 870 | return root_p.parse_args(["breakage", *args]) |
| 871 | |
| 872 | |
| 873 | class TestRegisterFlags: |
| 874 | def test_default_json_out_is_false(self) -> None: |
| 875 | ns = _parse_breakage() |
| 876 | assert ns.json_out is False |
| 877 | |
| 878 | def test_json_flag_sets_json_out(self) -> None: |
| 879 | ns = _parse_breakage("--json") |
| 880 | assert ns.json_out is True |
| 881 | |
| 882 | def test_j_shorthand_sets_json_out(self) -> None: |
| 883 | ns = _parse_breakage("-j") |
| 884 | assert ns.json_out is True |
| 885 | |
| 886 | def test_language_flag(self) -> None: |
| 887 | ns = _parse_breakage("--language", "Python") |
| 888 | assert ns.language == "Python" |
| 889 | |
| 890 | def test_strict_flag(self) -> None: |
| 891 | ns = _parse_breakage("--strict") |
| 892 | assert ns.strict is True |