Snapshot the template checks and listings, from packages loaded in memory - #152
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Add filtered template-call coverage and document both snapshot update commands.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds in-memory txtar snapshot coverage for Muxt analysis commands and updates contributor documentation.
Changes:
- Adds route, template listing, caller, call, and check fixtures.
- Adds a
loadtest-backed snapshot harness and configurations. - Updates snapshot testing guidance.
File summaries
| File | Summary | Review note |
|---|---|---|
internal/analysis/testdata/routes.txtar |
Route listing snapshot fixture. | — |
internal/analysis/testdata/check_wrong_field.txtar |
Wrong-field check fixture. | — |
internal/analysis/testdata/check_unused_templates.txtar |
Unused-template check fixture. | — |
internal/analysis/testdata/check_template_not_found.txtar |
Missing-template check fixture. | — |
internal/analysis/testdata/check_passes.txtar |
Passing check fixture. | — |
internal/analysis/testdata/check_bad_route_name.txtar |
Invalid-route-name check fixture. | — |
internal/analysis/testdata/calls.txtar |
Template-call listing fixture. | — |
internal/analysis/testdata/callers.txtar |
Template-caller listing fixture. | — |
internal/analysis/testdata/callers_match.txtar |
Filtered template-caller listing fixture. | — |
internal/analysis/snapshots_test.go |
Snapshot configuration cases. | Moderate (1 vote): add filtered list-template-calls coverage. |
internal/analysis/snapshot_test.go |
In-memory snapshot loading and comparison harness. | — |
CLAUDE.md |
Documents the snapshot workflow. | Nit (3 votes): document listings and both update commands. |
Review details
Suppressed comments (1)
internal/analysis/snapshots_test.go:36
list-template-callsalso accepts--matchandTemplateCallsConfiguration.FilterTemplateshas its own filtering path, but this new suite only snapshots unfiltered calls. Add a calls-match archive/config so the second template listing's filter behavior is covered, as it is for callers.
// muxt list-template-calls
archive: "calls",
config: analysis.TemplateCallsConfiguration{TemplatesVariables: []string{"templates"}},
- Files reviewed: 12/12 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
b9ac80c to
2c58360
Compare
28f15b7 to
2b9cff3
Compare
2c58360 to
acfb527
Compare
21633ce to
aebc4d8
Compare
acfb527 to
c0a92f8
Compare
aebc4d8 to
ed2e17c
Compare
be0c1ac to
c3314b4
Compare
ed2e17c to
452e06d
Compare
c3314b4 to
ab093a1
Compare
452e06d to
2a89249
Compare
ab093a1 to
30ccc07
Compare
288f984 to
8c594b8
Compare
30ccc07 to
14983bd
Compare
8c594b8 to
e0bb019
Compare
06390ef to
d848080
Compare
|
Heads up on what this branch holds now: merging #153 put the mutation commit on this branch (its base), so this PR carries both commits. I force-pushed the branch to the reviewed version of them — same content, plus the encoding/json/v2 fixes: the stray internal/analysis/configuration_json.go that had drifted into the mutation commit is gone, and an absent pattern in a mutation archive is null rather than "" (json/v2 reads "" as a pattern matching everything). The remaining json/v2 work — the module listing, the mutant overlay, the import map and --format=json — is #159, based on this branch. |
…mory muxt check is the command the integration suite runs most, and its reports -- a field the data type lacks, a template nothing renders, a route waiting for muxt generate -- were only stated there, behind a package load per script. internal/analysis/testdata/<command>/*.txtar holds cases for check, the route listing, and the template caller and call listings. The directory an archive is in names the command it runs, so a case runs alone as -run TestSnapshots/list-template-calls/calls, and the archive holds the configuration it runs with, in its own config.json, beside the command line that parses into it. A listing's configuration needs nothing to read as JSON: regexp.Regexp writes itself as the text it was compiled from, and internal/configjson says the rest -- a field the command line left alone is null rather than an empty list, and a member a configuration does not declare is an error. TestSnapshots loads the case through loadtest and internal/load's hydration, as the commands do, and compares what the analysis reports. Assisted-by: Claude:claude-opus-5 gofumpt
The mutation run's planning -- which templates each ExecuteTemplate call reaches, with what dot, and which variations apply -- read the loaded templates, the checker built on them, and the syntax trees it searched for the string literal a template was written in. So stating any of it took a module on disk and the go command. Planning now runs on an input: the source.Package internal/load reads. loadInput builds one with the go command; planFrom and revisionOf read nothing else. A template's string literal is found by parsing its file's text, which planning already holds, instead of searching the loader's syntax trees. With that, nothing reads load.Templates, and it goes. internal/mutation/testdata/test-template-mutations/*.txtar snapshots a dry run's report for ten cases -- a literal template, partials and trims, a template pattern, skipped mutants, the operand budget, other delimiters, a --diff revision, and the errors a plan returns -- each loaded through loadtest and planned in milliseconds. An archive holds the configuration it plans with, in its own config.json: regexp.Regexp writes itself as the text it was compiled from, so a pattern reads as the command line wrote it, and a pattern the command line left alone is null -- "" would be a pattern matching everything. As in the other two suites, the directory names the command, so a case runs alone as -run TestSnapshots/test-template-mutations/diff. CLAUDE.md's recipes for a feature, a bug and an error now start from the unit test at the layer that owns the behavior, and say what the in-memory loader cannot stand in for. Assisted-by: Claude:claude-opus-5 gofumpt
d848080 to
9fa2787
Compare
e0bb019 to
8f20fbf
Compare
The snapshot archives hold the configuration they run with, and reading
one is encoding/json/v2: it knows how to read a *regexp.Regexp, which v1
could only do through a second struct carrying the pattern's source.
Everywhere else muxt reads or writes JSON went with it, so one library
answers "how does muxt read JSON": the module list, the overlay each
mutant is delivered through, the generated file's import map, and a
command's --format=json result.
Two differences the standard library documents, and the integration suite
insisted on:
- omitempty in v2 omits an empty JSON value -- null, "", [], {} -- and no
longer a zero number or a false. The fields that meant the latter say
omitzero now, so a mutant that took no measurable time still reports no
seconds, and a module listing still leaves out the flags a package does
not set.
- Reading is case sensitive, and a member the target does not declare is
an error rather than silence. Every name muxt reads was already written
by muxt or by the go command, so nothing had to change.
- Writing makes no promise about map order, writes a nil list or map as
[] or {}, and escapes neither <, >, & nor U+2028 and U+2029, where v1
sorted map keys, wrote null and escaped them. --format=json is read by
scripts, so it is written with the options that say v1's choices, and
TestWriteResultJSON states each one. A listing's import map now writes
through the encoder it is handed (MarshalJSONTo), so those options reach
it too, rather than a separate Marshal deciding its order.
The reports and listings are byte for byte what they were.
Assisted-by: Claude:claude-opus-5 gofumpt
txtar snapshots for muxt check, list-routes and the two template listings, loaded through loadtest.
Part 7 of 8, stacked on #151. Replaces #145.
🤖 Generated with Claude Code
https://claude.ai/code/session_01UjkpA4fvZ7xQAsY65Beprp