diff --git a/scripts/classify-conflict.py b/scripts/classify-conflict.py index 7914085c..9e86fd40 100644 --- a/scripts/classify-conflict.py +++ b/scripts/classify-conflict.py @@ -52,6 +52,8 @@ Exit codes: 0 every block classified, none needs a human decision + (also: `--all` and git reports nothing unmerged - a clean merge is a + state, not a usage error; see the note in `main()`) 1 at least one block is `overlapping` (human must decide) 2 usage error / no conflict blocks found """ @@ -637,6 +639,22 @@ def main(argv: list[str] | None = None) -> int: paths = [p for p in paths if not (p in seen or seen.add(p))] if not paths: + if args.all: + # `--all` was *answered*, not misused: git reports nothing unmerged. + # This is rc 0 rather than rc 2 because a clean merge is a state, not a + # malformed invocation - and the message must say so, because the old + # one ("pass files, or --all for every unmerged path") told the caller + # to pass the flag they had just passed, at the one moment when the + # silence is the interesting signal: `cyc20260913-082711` hit this with + # a merge that resolved *cleanly* and produced a tree that fails the + # doc-count guard (issue #1158), so "no conflicts" must not be read as + # "the tree is fine" - hence the pointer to the tool that measures that. + print( + "no unmerged paths: the merge is clean or already resolved - " + "nothing to classify. A clean merge is not evidence of a healthy " + "tree; check the resulting tree with scripts/check-merge-sequence.py" + ) + return 0 print( "error: no paths given (pass files, or --all for every unmerged path)", file=sys.stderr, diff --git a/tests/test_classify_conflict.py b/tests/test_classify_conflict.py index 77d26560..7f3dca38 100644 --- a/tests/test_classify_conflict.py +++ b/tests/test_classify_conflict.py @@ -461,6 +461,56 @@ class TestCli: def test_no_paths_is_a_usage_error(self, mod, capsys) -> None: assert mod.main([]) == 2 + def test_all_with_nothing_unmerged_is_a_state_not_a_usage_error( + self, mod, capsys, monkeypatch + ) -> None: + """`--all` answered with an empty list is rc 0, not the usage error (cyc20260913-082711). + + Measured before this: with `--all` passed explicitly and no unmerged paths + (a merge that resolved cleanly), the tool printed "error: no paths given + (pass files, or --all for every unmerged path)" and exited 2 - telling the + caller to pass the flag they had just passed, and reporting a clean merge + as a malformed invocation. Hit in practice at the moment a clean merge had + produced a tree that fails the doc-count guard, i.e. exactly when the + silence needed an explanation rather than a usage complaint. + """ + monkeypatch.setattr(mod, "_unmerged_paths", lambda: []) + rc = mod.main(["--all"]) + captured = capsys.readouterr() + assert rc == 0, "a clean merge is a state, not a usage error" + assert "nothing to classify" in captured.out + assert "no paths given" not in captured.out + captured.err, ( + "the caller did pass --all; the message must not ask for it again" + ) + assert captured.err == "", "this is not an error, so nothing goes to stderr" + + # The pointer must be to a real script: a hint at a renamed or deleted tool + # is worse than no hint, and it is read at the one moment the reader has just + # merged something and wants to know whether the resulting tree is healthy. + referenced = [t for t in captured.out.split() if t.endswith(".py")] + assert referenced, f"the message no longer points at a tool: {captured.out!r}" + for name in referenced: + assert (REPO_ROOT / name).is_file(), ( + f"the message points at {name}, which does not exist in the repo" + ) + + def test_all_still_classifies_when_paths_are_unmerged( + self, mod, tmp_path, capsys, monkeypatch + ) -> None: + """The other direction: the new early return must not swallow the normal path.""" + f = tmp_path / "x.py" + f.write_text( + "<<<<<<< HEAD\ndef ours_only():\n pass\n=======\n" + "def theirs_only():\n pass\n>>>>>>> origin/master\n", + encoding="utf-8", + ) + monkeypatch.setattr(mod, "_unmerged_paths", lambda: [str(f)]) + rc = mod.main(["--all"]) + out = capsys.readouterr().out + assert rc == 0 + assert "disjoint" in out.lower() or "KEEP BOTH" in out + assert "nothing to classify" not in out + def test_missing_file_is_an_error(self, mod, tmp_path) -> None: assert mod.main([str(tmp_path / "nope.txt")]) == 2