"""Tests for cj-remove-block.py — idempotent removal of cj annotations by line range. The script removes lines [start, end] (1-indexed, inclusive) from an org file but validates first that those lines actually look like a cj annotation. Refusing on mismatch protects against accidentally trimming the wrong block when line numbers drift between scan and remove calls. """ import subprocess from pathlib import Path import pytest SCRIPT = Path(__file__).parent.parent / "cj-remove-block.py" @pytest.fixture(autouse=True) def isolated_tmpdir(tmp_path, monkeypatch): """Give every test in this module a private TMPDIR. The script backs up to the system temp dir under a name derived from the edited file's BASENAME. The real todo.org shares that basename, so any test operating on a fixture named todo.org writes something indistinguishable from a production backup — and an earlier version of this file globbed the shared /tmp and unlinked every match, so a routine `make test` destroyed Craig's real backups (found in review, 2026-07-24). Isolating at module scope rather than per-test is deliberate: the same bug was fixed once in the elisp sibling and left here, so relying on each new test to remember is exactly how it recurred. Autouse makes it structural. """ d = tmp_path / "_tmpdir" d.mkdir() # TMPDIR covers subprocess invocations of the script. monkeypatch.setenv("TMPDIR", str(d)) # tempfile.gettempdir() caches its answer on first call, so a test that # loads the module in-process would keep writing to the real /tmp no matter # what TMPDIR says. Override the cache too — this is the gap that made the # env-var-only version still leak one backup per suite run. import tempfile as _tempfile monkeypatch.setattr(_tempfile, "tempdir", str(d)) return d @pytest.fixture def run_remove(tmp_path): """Write content to a temp org file, run cj-remove-block, return new contents.""" def _run(content: str, start: int, end: int) -> str: f = tmp_path / "test.org" f.write_text(content) subprocess.run( ["python3", str(SCRIPT), "--file", str(f), "--start", str(start), "--end", str(end)], check=True, capture_output=True, ) return f.read_text() return _run @pytest.fixture def run_remove_expecting_failure(tmp_path): """Write content, run cj-remove-block expecting non-zero exit; return CalledProcessError.""" def _run(content: str, start: int, end: int): f = tmp_path / "test.org" f.write_text(content) with pytest.raises(subprocess.CalledProcessError) as excinfo: subprocess.run( ["python3", str(SCRIPT), "--file", str(f), "--start", str(start), "--end", str(end)], check=True, capture_output=True, ) return excinfo.value, f.read_text() # file should be unchanged on failure return _run # ---------------------------------------------------------------------- # Source-block removal # ---------------------------------------------------------------------- class TestCjRemoveBlockSourceBlock: """Removing #+begin_src cj: ... #+end_src blocks.""" def test_cj_remove_block_minimal_three_line_source_block(self, run_remove): """Normal: the three lines of a minimal source-block are removed.""" content = "* S\n#+begin_src cj: comment\nbody\n#+end_src\nafter\n" result = run_remove(content, start=2, end=4) assert result == "* S\nafter\n" def test_cj_remove_block_source_block_multiline_body(self, run_remove): """Normal: source-block with multi-line body removed cleanly.""" content = "* S\n#+begin_src cj: comment\nline 1\nline 2\nline 3\n#+end_src\nafter\n" result = run_remove(content, start=2, end=6) assert result == "* S\nafter\n" def test_cj_remove_block_preserves_lines_before_and_after(self, run_remove): """Normal: surrounding lines outside the range stay intact.""" content = "before\n#+begin_src cj: comment\nx\n#+end_src\nafter\n" result = run_remove(content, start=2, end=4) assert result == "before\nafter\n" def test_cj_remove_block_source_block_with_label_variant(self, run_remove): """Boundary: source-block with no trailing label (#+begin_src cj:) also removable.""" content = "* S\n#+begin_src cj:\nbody\n#+end_src\nafter\n" result = run_remove(content, start=2, end=4) assert result == "* S\nafter\n" def test_cj_remove_block_case_insensitive_fence(self, run_remove): """Boundary: case-variant fences (#+BEGIN_SRC / #+END_SRC) also removable.""" content = "* S\n#+BEGIN_SRC cj: comment\nbody\n#+END_SRC\nafter\n" result = run_remove(content, start=2, end=4) assert result == "* S\nafter\n" # ---------------------------------------------------------------------- # Legacy-inline removal # ---------------------------------------------------------------------- class TestCjRemoveBlockLegacyInline: """Removing single-line legacy `cj: ...` annotations.""" def test_cj_remove_block_legacy_inline_single_line(self, run_remove): """Normal: single legacy-inline cj line removed.""" content = "* S\ncj: legacy note\nafter\n" result = run_remove(content, start=2, end=2) assert result == "* S\nafter\n" def test_cj_remove_block_legacy_inline_at_eof(self, run_remove): """Boundary: legacy-inline cj at last line; file ends cleanly.""" content = "* S\ncj: at end\n" result = run_remove(content, start=2, end=2) assert result == "* S\n" # ---------------------------------------------------------------------- # Refusal-on-mismatch safety # ---------------------------------------------------------------------- class TestCjRemoveBlockSafety: """Refuses to remove if the specified range doesn't look like a cj annotation.""" def test_cj_remove_block_refuses_non_cj_single_line(self, run_remove_expecting_failure): """Error: a single non-cj line is rejected.""" err, post_content = run_remove_expecting_failure( "* S\nthis is not a cj line\nafter\n", start=2, end=2, ) assert err.returncode != 0 # File must be unchanged assert post_content == "* S\nthis is not a cj line\nafter\n" def test_cj_remove_block_refuses_mismatched_fence(self, run_remove_expecting_failure): """Error: multi-line range where line N isn't an opening fence is rejected.""" err, post_content = run_remove_expecting_failure( "* S\nbody1\nbody2\n#+end_src\nafter\n", start=2, end=4, ) assert err.returncode != 0 assert "body1" in post_content # file unchanged def test_cj_remove_block_refuses_missing_closing_fence(self, run_remove_expecting_failure): """Error: multi-line range where line M isn't a closing fence is rejected.""" err, post_content = run_remove_expecting_failure( "* S\n#+begin_src cj: comment\nbody\nnot-a-close\nafter\n", start=2, end=4, ) assert err.returncode != 0 assert "not-a-close" in post_content def test_cj_remove_block_refuses_out_of_bounds(self, run_remove_expecting_failure): """Error: range outside the file is rejected, file unchanged.""" err, post_content = run_remove_expecting_failure( "* S\nafter\n", start=5, end=7, ) assert err.returncode != 0 assert post_content == "* S\nafter\n" def test_cj_remove_block_refuses_inverted_range(self, run_remove_expecting_failure): """Error: end < start is rejected, file unchanged.""" original = "* S\n#+begin_src cj: comment\nbody\n#+end_src\n" err, post_content = run_remove_expecting_failure(original, start=4, end=2) assert err.returncode != 0 assert post_content == original class TestMultiBlockRangeRefused: """The validation exists to catch a drifted range, but it only checked the first and last lines of that range. A span from one block's opening fence to a LATER block's closing fence passed, and the removal silently deleted every line between — real prose, headings, whole tasks — with a zero exit. Drift is the skill's normal operating mode (respond-to-cj-comments edits the file as it processes, and a file under cj review usually holds several blocks), so this is the exact scenario the check was written for. Reproduced 2026-07-24.""" TWO_BLOCKS = ( "* Alpha\n" "#+begin_src cj:\n" "note A\n" "#+end_src\n" "KEEP THIS LINE\n" "* Beta\n" "#+begin_src cj:\n" "note B\n" "#+end_src\n" ) def test_range_spanning_two_blocks_is_refused(self, run_remove_expecting_failure): # Lines 2..9: block one's opener through block two's closer. err, content = run_remove_expecting_failure(self.TWO_BLOCKS, 2, 9) assert err.returncode == 1 assert "KEEP THIS LINE" in content, "content between the blocks was destroyed" assert "* Beta" in content, "a heading between the blocks was destroyed" def test_refusal_names_the_reason(self, run_remove_expecting_failure): err, _ = run_remove_expecting_failure(self.TWO_BLOCKS, 2, 9) assert "more than one" in err.stderr.decode().lower() def test_a_correct_single_block_range_still_removes(self, run_remove): # The fix must not over-tighten: the legitimate range still works. out = run_remove(self.TWO_BLOCKS, 2, 4) assert "note A" not in out assert "KEEP THIS LINE" in out assert "note B" in out, "the second block must be untouched" def test_a_nested_end_src_inside_the_range_is_refused(self, run_remove_expecting_failure): # Any #+end_src before the final line means the range covers >1 block. content = ( "#+begin_src cj:\n" "a\n" "#+end_src\n" "middle\n" "#+begin_src cj:\n" "b\n" "#+end_src\n" ) err, after = run_remove_expecting_failure(content, 1, 7) assert err.returncode == 1 assert "middle" in after class TestSafeMutation: """The script rewrites Craig's org files (todo.org, notes.org). It wrote with a bare write_text, which truncates the target on open, and took no backup — so a mid-write failure left the file truncated with no copy to recover from. lint-org.el, the other tool that mutates these files, backs up to a temp dir first. Match that, and make the write atomic. Every test here redirects TMPDIR to a private directory. The backup name derives from the file's basename, and the real todo.org shares it, so a test globbing the shared temp dir cannot tell its own artifact from a genuine backup — and an earlier version of this class globbed /tmp and unlinked every match, so a routine `make test` destroyed real backups (found in review, 2026-07-24). Never glob or delete across the shared temp dir.""" ONE_BLOCK = "* T\n#+begin_src cj:\nnote\n#+end_src\nkeep\n" def test_a_backup_is_written_before_mutating(self, tmp_path): import subprocess, glob, os bdir = tmp_path / "bk" bdir.mkdir() f = tmp_path / "todo.org" f.write_text(self.ONE_BLOCK) subprocess.run( ["python3", str(SCRIPT), "--file", str(f), "--start", "2", "--end", "4"], check=True, capture_output=True, env={**os.environ, "TMPDIR": str(bdir)}, ) backups = glob.glob(str(bdir / "todo.org.before-cj-remove.*")) assert backups, "no backup was written before mutating the org file" assert "note" in Path(max(backups)).read_text() def test_no_partial_file_when_the_write_fails(self, tmp_path, monkeypatch): import importlib.util spec = importlib.util.spec_from_file_location("crb", SCRIPT) mod = importlib.util.module_from_spec(spec) spec.loader.exec_module(mod) bdir = tmp_path / "bk" bdir.mkdir() monkeypatch.setenv("TMPDIR", str(bdir)) f = tmp_path / "todo.org" f.write_text(self.ONE_BLOCK) def boom(*a, **k): raise OSError("disk full") monkeypatch.setattr(mod.os, "replace", boom) with pytest.raises(OSError): mod.remove_range(f, 2, 4) # The original survives intact — no truncation, no partial. assert f.read_text() == self.ONE_BLOCK class TestBackupNeverOverwrites: """Same defect class as todo-cleanup's, and more reachable here: the respond-to-cj-comments skill removes several annotations in quick succession, so a second-resolution stamp collides and the later backup overwrote the earlier one with already-mutated content.""" TWO_BLOCKS = ( "* A\n#+begin_src cj:\nfirst\n#+end_src\n" "* B\n#+begin_src cj:\nsecond\n#+end_src\n" ) def test_consecutive_removals_each_keep_a_backup(self, tmp_path, monkeypatch): import subprocess, glob bdir = tmp_path / "bk" bdir.mkdir() monkeypatch.setenv("TMPDIR", str(bdir)) f = tmp_path / "todo.org" f.write_text(self.TWO_BLOCKS) original = f.read_text() # Remove the second block, then the first — back to back, same second. subprocess.run(["python3", str(SCRIPT), "--file", str(f), "--start", "6", "--end", "8"], check=True, capture_output=True, env={**__import__("os").environ, "TMPDIR": str(bdir)}) subprocess.run(["python3", str(SCRIPT), "--file", str(f), "--start", "2", "--end", "4"], check=True, capture_output=True, env={**__import__("os").environ, "TMPDIR": str(bdir)}) backups = glob.glob(str(bdir / "todo.org.before-cj-remove.*")) assert len(backups) == 2, f"expected 2 backups, got {len(backups)}" contents = [Path(b).read_text() for b in backups] assert original in contents, "no backup holds the true original"