Close the gaps mutation testing found

CHANGELOG.md

@@ -87,6 +87,22 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- `_MUTATING_METHODS` no longer lists `logseq.Editor.setBlockProperty` and
+  `logseq.Editor.replaceText`. Neither has a wrapper and neither was ever sent:
+  all 19 `call()` invocations pass a literal method name, so no input could
+  reach them. They date from the initial import and described a tool that does
+  not exist.
+
+  The entries are the smaller half. The find is the check that was missing:
+  `_CACHEABLE_METHODS` has been held to its call sites since the read cache
+  shipped, and the mutating list had no counterpart, which is why two entries
+  survived there for the life of the project. Both lists are now bound to the
+  wrappers that send them, in both directions.
+
+  No behaviour changes for any command. A method in neither list is read from
+  the network every time and leaves the cache untouched, and these two were in
+  no code path to begin with.
+
 - The commands moved out of `cli.py` into `logseq_cli/commands/`, one module per
   group of commands, with the click group in `group.py`, the result and error
   helpers in `output.py` and the block rendering in `render.py`. `cli.py` is now

logseq_cli/api.py

@@ -33,8 +33,6 @@
     "logseq.Editor.removeBlock",
     "logseq.Editor.upsertBlockProperty",
     "logseq.Editor.removeBlockProperty",
-    "logseq.Editor.setBlockProperty",
-    "logseq.Editor.replaceText",
     "logseq.Editor.insertBatchBlock",
     "logseq.Editor.moveBlock",
 })

logseq_cli/helpers.py

@@ -3,7 +3,6 @@
 import calendar
 import json
 import datetime
-from collections import Counter
 from pathlib import Path
 
 import click

tests/test_api_endpoint_binding.py

@@ -0,0 +1,153 @@
+"""Each API wrapper must reach the endpoint its own name promises.
+
+The wrappers in :class:`LogseqAPI` are one-liners around ``call()``, so what
+they assert is not a return value but *which* endpoint goes out with which
+arguments. Nothing held them to that: a mutation pointing ``delete_page`` at
+``logseq.Editor.renamePage`` left the whole suite green, and a destructive
+command silently calling a different endpoint is the worst version of this
+defect.
+
+The existing set-vs-reality test in ``test_api_cache.py`` cannot catch it. It
+asserts a method name appears *somewhere* in ``api.py``; after swapping two
+endpoints between wrappers both names are still there.
+
+These tests therefore derive the expectation instead of restating it. A table
+mapping wrapper to endpoint would be copied out of ``api.py`` and checked
+against ``api.py`` — it would pin down whatever is written there, including a
+mistake. The wrapper's own name is the independent source: ``snake_case``
+turned to ``camelCase`` is the endpoint's leaf, without exception across all
+18 wrappers.
+"""
+
+import ast
+import pathlib
+
+import pytest
+
+from logseq_cli.api import _CACHEABLE_METHODS, _MUTATING_METHODS
+
+
+def _camel(snake: str) -> str:
+    head, *tail = snake.split("_")
+    return head + "".join(word.capitalize() for word in tail)
+
+
+def _wrapper_endpoints():
+    """{wrapper name: endpoint} read off the AST, not a list kept by hand."""
+    source = pathlib.Path(
+        pathlib.Path(__file__).resolve().parent.parent / "logseq_cli" / "api.py"
+    ).read_text(encoding="utf-8")
+    tree = ast.parse(source)
+    cls = next(
+        node for node in tree.body
+        if isinstance(node, ast.ClassDef) and node.name == "LogseqAPI"
+    )
+    found = {}
+    for node in cls.body:
+        if not isinstance(node, ast.FunctionDef):
+            continue
+        endpoints = [
+            sub.value for sub in ast.walk(node)
+            if isinstance(sub, ast.Constant)
+            and isinstance(sub.value, str)
+            and sub.value.startswith("logseq.")
+        ]
+        if len(endpoints) == 1:
+            found[node.name] = endpoints[0]
+        elif endpoints:
+            raise AssertionError(
+                f"{node.name} names more than one endpoint: {endpoints}. "
+                "The name-derived check below cannot decide which one is meant."
+            )
+    return found
+
+
+WRAPPERS = _wrapper_endpoints()
+
+
+def test_every_wrapper_was_found():
+    """Guards the reader itself: an empty result would make every test pass."""
+    assert len(WRAPPERS) >= 18, WRAPPERS
+
+
+@pytest.mark.parametrize("wrapper", sorted(WRAPPERS))
+def test_endpoint_leaf_matches_wrapper_name(wrapper):
+    """``get_page_blocks_tree`` must call ``…getPageBlocksTree``, not another read."""
+    endpoint = WRAPPERS[wrapper]
+    leaf = endpoint.rsplit(".", 1)[1]
+    assert leaf == _camel(wrapper), (
+        f"{wrapper}() calls {endpoint}; its name promises "
+        f"…{_camel(wrapper)}"
+    )
+
+
+@pytest.mark.parametrize("wrapper", sorted(WRAPPERS))
+def test_endpoint_is_classified_for_the_cache(wrapper):
+    """Every endpoint is either cacheable or mutating — never unclassified.
+
+    An endpoint in neither set is read from the network every time *and* leaves
+    a stale cache behind it, which is the failure mode that is hardest to see:
+    both halves look like they work.
+    """
+    endpoint = WRAPPERS[wrapper]
+    assert (endpoint in _CACHEABLE_METHODS) != (endpoint in _MUTATING_METHODS), (
+        f"{endpoint} ({wrapper}) must be in exactly one of "
+        "_CACHEABLE_METHODS / _MUTATING_METHODS"
+    )
+
+
+def test_every_mutating_method_has_a_wrapper():
+    """The other direction: no entry without a call site.
+
+    test_api_cache.py holds _CACHEABLE_METHODS to this and the mutating list
+    had no counterpart, which is how two entries survived that were never
+    sent -- setBlockProperty and replaceText, dating from the initial import.
+    The find was not the entries but the missing check: an inventory nobody
+    verifies describes what the tool once did, not what it does.
+    """
+    unused = sorted(_MUTATING_METHODS - set(WRAPPERS.values()))
+    assert not unused, (
+        f"listed as mutating but no wrapper sends them: {unused}"
+    )
+
+
+def test_every_cacheable_method_has_a_wrapper():
+    """Same guard for the read list, bound to wrappers rather than to text.
+
+    test_api_cache.py asserts the name appears somewhere in api.py; that stays
+    true for an entry whose wrapper was deleted. This binds it to a call site.
+    """
+    unused = sorted(_CACHEABLE_METHODS - set(WRAPPERS.values()))
+    assert not unused, (
+        f"listed as cacheable but no wrapper sends them: {unused}"
+    )
+
+
+def test_cache_invalidation_covers_every_mutating_wrapper():
+    """Every wrapper classified as mutating must clear the cache when called.
+
+    ``call()`` clears the cache in an ``elif`` on ``_MUTATING_METHODS``. A write
+    missing from that set would leave reads answering from a cache the write
+    just invalidated — success reported, stale data served.
+    """
+    import json
+    from unittest.mock import MagicMock, patch
+
+    from logseq_cli.api import LogseqAPI
+
+    mutating = {w: e for w, e in WRAPPERS.items() if e in _MUTATING_METHODS}
+    assert mutating, "no mutating wrapper found — the reader is broken"
+
+    for wrapper, endpoint in sorted(mutating.items()):
+        api = LogseqAPI(host="localhost", port="12315", token="t")
+        api._cache[("logseq.Editor.getPage", json.dumps(["Foo"]))] = (
+            {"stale": True}, float("inf"),
+        )
+        resp = MagicMock(status_code=200)
+        resp.json.return_value = {}
+        resp.raise_for_status = MagicMock()
+        with patch("requests.post", return_value=resp):
+            api.call(endpoint, [])
+        assert api._cache == {}, (
+            f"{wrapper}() calls {endpoint} without clearing the cache"
+        )

tests/test_backlinks_bruteforce_scan.py

@@ -0,0 +1,122 @@
+"""The brute-force backlink scan, and the escaping it depends on.
+
+``find_backlinks`` is the fallback ``get-backlinks`` drops to when the native
+``getPageLinkedReferences`` fails. It builds a regex out of the page name, so a
+name carrying regex metacharacters — ``C++``, ``What is this?``,
+``Report (2025)`` — compiles into a pattern that no longer matches the link it
+was built for. The command then reports no backlinks and exits 0.
+
+That is the failure mode this project has had before: output that shows less
+than exists, with nothing to indicate it. Worse here, because the path only
+runs when something has already gone wrong.
+
+``escape_regex`` is one line, ``return re.escape(s)``, and dropping the escaping
+left all 842 tests green. The assertion is not the line — it is that a page name
+is matched literally, whatever characters it contains.
+"""
+
+import re
+from unittest.mock import MagicMock, patch
+
+import pytest
+
+from logseq_cli.cli import cli
+from logseq_cli.helpers import escape_regex, find_backlinks
+from tests.conftest import split_runner
+
+
+# Names that are legal Logseq page titles and also regex syntax. Each one is a
+# different way the unescaped pattern goes wrong: a quantifier with nothing to
+# repeat, an optional character, a group, an alternation, a wildcard.
+# One name per way an unescaped pattern breaks. Measured, not guessed: each of
+# these finds nothing without escaping, and each fails for a different reason.
+# Names whose metacharacters happen to still match ("a|b", "Notes.") are not
+# listed here -- widening is a separate claim, asserted once below.
+METACHARACTER_NAMES = [
+    "C++",             # + quantifies the character before it
+    "What is this?",   # ? makes it optional
+    "Report (2025)",   # () opens a group
+    "Budget [2025]",   # [] opens a character class
+    "foo*bar",         # * quantifies
+]
+
+
+def _api(pages):
+    """pages: {page name: page text}. get_page_blocks_tree answers one block."""
+    api = MagicMock()
+    api.get_all_pages.return_value = [{"originalName": n} for n in pages]
+
+    def _tree(name):
+        text = pages.get(name)
+        return [{"content": text, "uuid": f"u-{name}", "children": []}] if text else []
+
+    api.get_page_blocks_tree.side_effect = _tree
+    return api
+
+
+class TestEscaping:
+    """The one line, stated as what it guarantees rather than what it calls."""
+
+    @pytest.mark.parametrize("name", METACHARACTER_NAMES)
+    def test_escaped_name_matches_itself_literally(self, name):
+        pattern = re.compile(escape_regex(name))
+        assert pattern.search(f"see [[{name}]] here"), name
+
+    @pytest.mark.parametrize("name", METACHARACTER_NAMES)
+    def test_escaped_name_is_a_valid_pattern(self, name):
+        """An unescaped ``C++`` raises; the caller has no try/except for that."""
+        re.compile(escape_regex(name))
+
+    def test_escaping_does_not_widen_the_match(self):
+        """``a|b`` must not match a page containing only ``b``."""
+        pattern = re.compile(escape_regex("a|b"))
+        assert not pattern.search("see [[b]] here")
+        assert pattern.search("see [[a|b]] here")
+
+
+class TestScanFindsNamesWithMetacharacters:
+    """The function, driven the way the command drives it."""
+
+    @pytest.mark.parametrize("name", METACHARACTER_NAMES)
+    def test_linking_page_is_found(self, name):
+        api = _api({name: "the target page",
+                    "Linking Page": f"mentions [[{name}]] in passing",
+                    "Unrelated": "no links here"})
+        assert find_backlinks(api, name) == ["Linking Page"]
+
+    def test_does_not_match_a_different_page(self):
+        """Without escaping, ``a|b`` would report a page linking only ``b``."""
+        api = _api({"a|b": "target",
+                    "False Friend": "links [[b]] only"})
+        assert find_backlinks(api, "a|b") == []
+
+    def test_self_reference_is_excluded(self):
+        api = _api({"C++": "a page that mentions [[C++]] itself",
+                    "Other": "also mentions [[C++]]"})
+        assert find_backlinks(api, "C++") == ["Other"]
+
+    def test_match_is_case_insensitive_and_tolerates_inner_spaces(self):
+        api = _api({"C++": "target", "L": "see [[  c++  ]] there"})
+        assert find_backlinks(api, "C++") == ["L"]
+
+
+class TestFallbackPathReachesTheScan:
+    """get-backlinks falls back to the scan when the native API fails.
+
+    Driven through the CLI, because the escaping only matters on the path that
+    runs when something else already broke.
+    """
+
+    @pytest.mark.parametrize("name", ["C++", "Report (2025)"])
+    def test_backlinks_reported_after_native_api_fails(self, name):
+        api = _api({name: "target",
+                    "Linking Page": f"mentions [[{name}]]"})
+        api.get_page_linked_references.side_effect = RuntimeError("unexpected format")
+        with patch("logseq_cli.group.LogseqAPI", return_value=api):
+            result = split_runner().invoke(
+                cli, ["get-backlinks", "--page", name, "--json"])
+        assert result.exit_code == 0, result.output
+        import json
+        payload = json.loads(result.stdout)
+        entry = payload[0] if isinstance(payload, list) else payload
+        assert "Linking Page" in json.dumps(entry), result.stdout