Close the gaps mutation testing found - #20
Merged
Merged
Conversation
The wrappers in LogseqAPI are one-liners around call(), so what they assert is which endpoint goes out, not what comes back. Nothing held them to it: pointing delete_page at logseq.Editor.renamePage left all 842 tests green, and a destructive command reaching a different endpoint is the worst form this defect takes. The 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. A table mapping wrapper to endpoint would be no better. Copied out of api.py and checked against api.py, it pins down whatever is written there, mistake included. The wrapper's own name is the independent source: snake_case to camelCase gives the endpoint leaf, without exception across all 18. Also asserted: every endpoint sits in exactly one of the two method sets, and every mutating wrapper clears the cache when call() runs it.
The class around it walks the command registry precisely so a write command cannot be missed by a list nobody updated. The list of mutating wrappers it compares against stayed hand-written, and had already drifted: it carried replace_text, for which no wrapper exists. Harmless in that direction. A missing entry is not — it would excuse a write command from needing --dry-run, silently, which is the defect the registry walk was written to prevent one layer up. Now read off _MUTATING_METHODS and the api.py AST, so the two cannot disagree.
format_journal_date turns a date into the title Logseq stores the entry under, and nine call sites in the journal, edit and analysis commands write through it. A wrong name does not look wrong -- it addresses a different page, so add-journal-entry creates one instead of appending to the entry already there. Nothing tested it. Returning "th" from get_day_suffix for every day left all 842 tests green, although it turns "mar 3rd, 2025" into "mar 3th, 2025" under the format Logseq ships with by default. Covered: the ordinal rule including the 11-13 exception that breaks the last-digit shortcut, the default format, the tokens a configured :journal/page-title-format can use, and that every day of a year yields one stable, unique name.
is_journal_date is what makes create-page pass journal?: true to Logseq. Wrong in one direction an ordinary page lands in the journal timeline; wrong in the other the journal for that day exists twice, once as a plain page. Returning True for every name left all 842 tests green. The recogniser and the formatter are two halves of one claim, so the central test feeds format_journal_date output back in rather than restating the four formats by hand -- a name the tool writes but does not recognise is exactly what creates the duplicate. Checked across a full year, where ordinal suffixes and zero padding vary. Also pinned: the patterns are anchored, so a date inside a longer name does not make the page a journal.
find_backlinks is the fallback get-backlinks drops to when the native getPageLinkedReferences call fails. It builds a regex out of the page name, so a title carrying regex syntax -- C++, What is this?, Report (2025) -- compiled into a pattern that no longer matched the link it was built for. The command then reported no backlinks and exited 0. Output showing less than exists, with nothing to indicate it, is a defect this project has had before. Worse on this path: it only runs when something else has already broken. escape_regex is one line and dropping it left all 842 tests green. What is asserted here is not the call but the guarantee -- the name is matched literally, a metacharacter does not widen the match to a different page, and the fallback still finds the linking page when driven through the CLI.
"Projekt-Plan" was taken from a real graph, by way of the working notes this test was written from. It carried no personal data, but test fixtures should not borrow page names from anyone's graph at all -- the next borrowed name may not be harmless, and nothing would flag it. Replaced with an invented English name. No assertion changes.
The group callback turns the flag into api.cache_enabled = False. Replacing that line with pass left all 842 tests green: the one test on the subject sets the attribute itself, so it never runs the wiring the flag depends on. What the user loses is specific. --no-cache is what you reach for after changing something in Logseq, so a flag that does nothing answers with the state from before the change and gives no sign it did. Driven through the CLI and counted at the network boundary, since the claim is about requests going out rather than about an attribute being set.
blocks_to_markdown is the default output of get-page and get-journal-range, and nothing tested it. Two independent mutations survived the whole suite: dropping the properties-block branch, and dropping indentation entirely. Worth spelling out, because is_properties_block itself is covered -- by tests that exercise get-backlinks --with-context, a different caller. Coverage of a function says nothing about coverage of the branch that calls it. The properties branch is why the renderer is not a one-liner: Logseq stores a page's key:: value header without a bullet, and rendering it as a list item produces a page that no longer round-trips into the graph. blocks_with_ids promises in its docstring that its indentation matches. That is asserted by rendering one tree through both renderers rather than by writing the expected tab runs out twice.
test_doctor.py patches both probes out. That is right for what it tests -- how doctor reports a given answer -- but it means the probes themselves never ran: returning None from either left all 842 tests green. They decide the distinction their docstring calls the one that costs the most time by hand: "Logseq is not running" against "Logseq runs, but its HTTP API is off". Answered wrong, doctor sends the user to start an application that is already open, or into the settings of one that is closed. The port probe runs against a real socket bound on loopback and closed again in the fixture. The process probe runs against a stubbed pgrep, because a real one answers differently depending on whether the machine running the suite has Logseq open. Pinned along the way: unknown (None) stays distinguishable from not-running (False), and the lowercase pattern is still tried, which is the only one that matches on Linux.
_print_results is what a user sees without --json, and returning None from it left all 842 tests green. Its four branches exist because Datalog does not answer in one shape: rows wrapping a block, bare rows, plain maps, or scalars, each with a different place to look for a name. A branch reading the wrong field does not fail. It prints a blank line where a page name belongs, numbered as though a result were there. The 20-item cap is pinned too. It is what keeps a query matching a thousand blocks from filling a terminal, and the count line above the list still reports the full number, so a silent change to it misleads twice.
Two assertions left after the sweep, both real once traced. pyproject installs the console script as logseq_cli.cli:cli, so main() is not on the installed path at all. It carries `python logseq_cli/cli.py`, the way the tool runs from a checkout without installing. Emptying it left the whole suite green while that invocation fell silent. DatalogQueryError carries api_message and query as attributes because output.py reads e.query to build the JSON error payload, as its docstring states. The two sibling errors follow the same shape with value, and nothing reads them yet, so dropping the assignment changed nothing a test could see. Kept rather than deleted: naming the offending value is why the fields exist, and the three classes are meant to answer alike. Pinning that intent means the next JSON error path still finds the field there.
Both parametrised lists carried cases that fell together with their neighbours. Measured rather than judged: breaking one suffix branch failed seven date cases where one per suffix class is enough, and of the eight metacharacter names three did not exercise the escaping at all -- "50% capacity" has no metacharacter, and "a|b" and "Notes." still match unescaped, which is a separate claim already asserted on its own. Down 16 tests. Every mutation these files were written against is still caught, and the full sweep still reports no survivors.
test_api_cache.py has held _CACHEABLE_METHODS to its call sites since the read cache shipped; the mutating list had no such check. The endpoint binding added earlier only looked one way -- every wrapper reaches a listed method -- so an entry with no wrapper at all stayed invisible. Verified by adding a method name that nothing sends: the suite passed before this test and fails after it.
setBlockProperty and replaceText have no wrapper and no call site. All 19 call() invocations pass a literal method name, so nothing could reach them; they date from the initial import and described a tool that does not exist. Behaviour is unchanged for every command -- a method in neither list is read from the network each time and leaves the cache alone, and these two were in no code path. The guard added in the previous commit is what keeps the lists from collecting entries like this again.
muellerei
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes step 1 of the roadmap. No production behaviour changes except one
removal, noted below and in the CHANGELOG.
What the befund got wrong
The befund this branch started from listed six unprotected assertions. Its
mutation pattern was
def NAME\(([^)]*)\):, which matches no signaturecarrying a return annotation — 75 of the 170 functions in the package. The
substitution replaced nothing and
count=1reported no failure, so the "greensuite" was the unmutated suite.
That produced wrong findings in both directions: two protected assertions
reported as gaps, and every real gap behind an annotated signature missed.
What a working sweep found
Function bodies replaced by AST line span, every mutation verified to have
changed the file before the suite runs. 132 functions mutated, 0 silently
skipped, 24 survivors against the 3 that were known.
api.pydelete_pagecould callrenamePagehelpers.pyrender.pygroup.py--no-cachecould be made inertcommands/meta.pydoctor's probescommands/query.pysmart-queryTwo survivors were not gaps and are recorded as such:
cli.py:mainis thecheckout entry point (now covered by a subprocess test) and
datalog.py:__init__is a constructor, wherereturn Noneis correct.How the tests are written
Derived, not restated. A table mapping wrapper to endpoint would be copied out
of
api.pyand checked againstapi.py, pinning down whatever is writtenthere. The wrapper's own name is the independent source:
snake_casetocamelCasegives the endpoint leaf, without exception across all 18. The sameholds for the journal pair —
is_journal_dateis fed the output offormat_journal_dateacross a full year rather than restating four formats.Every test was verified against the mutation it targets: remove the assertion,
the test falls. The full sweep now reports 0 survivors.
The one behaviour change
_MUTATING_METHODSlistedsetBlockPropertyandreplaceText; neither has awrapper and neither was ever sent. The entries are the smaller half — the find
is that
_CACHEABLE_METHODShas been held to its call sites since the readcache shipped and the mutating list had no counterpart, which is why they
survived. Both lists are now bound to their wrappers in both directions.
CHANGELOG entry included. No release.
Size
842 tests to 999. Two parametrised lists were measured and cut back where cases
fell together with their neighbours (−16). Each remaining file was checked by
removing it and confirming its mutation goes uncaught without it.