Conversation
This was referenced Sep 14, 2026
There was a problem hiding this comment.
🟢 Approval recommended
No unresolved review comments remain.
Pull request overview
Refactors CLI commands to build typed configurations before execution, with tests for command-line parsing and -C handling.
Changes:
- Extracts command runners into
internal/cli/run.go. - Injects runners into CLI commands.
- Adds configuration and rejection tests.
- Documents the testing approach.
File summaries
| File | Description |
|---|---|
internal/cli/run.go |
Implements command runners. |
internal/cli/configurations_test.go |
Tests parsed configurations and rejected arguments. |
internal/cli/commands.go |
Connects commands to injected runners. |
CLAUDE.md |
Documents CLI configuration testing. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
crhntr
force-pushed
the
split/4-checker
branch
from
September 16, 2026 01:36
cb82817 to
4ab8fad
Compare
crhntr
force-pushed
the
split/5-command-configurations
branch
from
September 16, 2026 01:36
d562b63 to
9f9dc50
Compare
crhntr
force-pushed
the
split/4-checker
branch
from
September 16, 2026 06:07
4ab8fad to
4ffbdd1
Compare
crhntr
force-pushed
the
split/5-command-configurations
branch
from
September 16, 2026 06:07
9f9dc50 to
a822bf1
Compare
crhntr
force-pushed
the
split/4-checker
branch
from
September 16, 2026 06:22
4ffbdd1 to
d527e46
Compare
crhntr
force-pushed
the
split/5-command-configurations
branch
from
September 16, 2026 06:22
a822bf1 to
0deba16
Compare
crhntr
force-pushed
the
split/5-command-configurations
branch
from
September 16, 2026 06:38
0deba16 to
cc7dcfb
Compare
crhntr
force-pushed
the
split/4-checker
branch
from
September 16, 2026 06:38
d527e46 to
a8856ac
Compare
crhntr
force-pushed
the
split/5-command-configurations
branch
from
September 16, 2026 06:42
cc7dcfb to
c575f80
Compare
crhntr
force-pushed
the
split/4-checker
branch
from
September 16, 2026 06:42
a8856ac to
8ebd9fa
Compare
crhntr
force-pushed
the
split/5-command-configurations
branch
from
September 16, 2026 06:58
c575f80 to
da4e93d
Compare
crhntr
force-pushed
the
split/4-checker
branch
from
September 16, 2026 06:58
8ebd9fa to
5f2f882
Compare
A large share of the integration suite checks that flags mean what they say, and it could only do that by running the whole command: load a package, generate, compile. Deciding what a command line means happens before any of that, but nothing let a test stop there. Each command now builds its configuration -- flags parsed, defaults applied, what cannot work rejected -- and hands it to a runner, which loads the package and runs the implementation. Commands wires the real runners, in run.go; configurationOf wires ones that record what they are handed. TestCommandLineConfigurations states what command lines parse into, as composite literals, and TestCommandLineRejections the command lines refused, with the error a user sees. TestChangeDirectory states the directory -C runs a command in. None of them loads anything. Six commands have runners: the route listing, check, generate, the two template listings and the mutation run. --format is still read when a result is written, and generate-fake-server and explore-module still load in their own RunE; they are left for later. A generated file's header now records the version from the configuration rather than asking the build info again, so the version a run writes is the one its configuration holds. Assisted-by: Claude:claude-opus-5 gofumpt
crhntr
force-pushed
the
split/4-checker
branch
from
September 16, 2026 07:26
5f2f882 to
fd50bdf
Compare
crhntr
force-pushed
the
split/5-command-configurations
branch
from
September 16, 2026 07:26
da4e93d to
336b576
Compare
generate's own tests covered a few percent of it: what a handler looks like for a form struct, an sse route, a status code in the name or a redirect in the template was only stated by the integration suite, which compiles and runs every scratch module and takes over a minute. internal/load/loadtest builds what a package load returns without loading the package graph: it writes the files, type checks them in memory, and imports the official standard library from the export data go list reports for it, reading it with gcexportdata -- as go/packages does -- and giving each package the imports go list reports, which is how check finds fmt behind html/template. The standard library is whichever the go command in use provides. A test binary pays a few hundred milliseconds once to read it. TestStandardLibrary states load.StandardLibrary's answers against it, and TestHydration the order load's hydration reports a missing package, a missing receiver and a variable that does not evaluate. internal/generate/testdata/generate/*.txtar holds one case per feature: the receiver's Go source, the templates, and the files and log lines generation produces. An archive holds the configuration it generates with, in its own config.json, beside the command line that parses into it; nothing outside the archive says what a case is. The directory an archive is in names the command, so a case runs alone as -run TestSnapshots/generate/sse. Reading it is encoding/json/v2, which the module now asks for Go 1.27 to have. TestSnapshots loads the case through loadtest and load.GenerateSource, as muxt generate does, generates, and compares; the 32 cases cover most of the package. It also fails on a generated file that imports a package it does not use. go test -run TestSnapshots -update rewrites the want/ files, and the diff is the review. A snapshot says what the generator does, not that the result compiles or serves requests; that stays the integration suite's job. Assisted-by: Claude:claude-opus-5 gofumpt
…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
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
A few loops spelled out what the slices and maps packages already name: gathering a map's keys and sorting them, asking whether a list holds something, copying a list, and a union of keys de-duplicated by hand. - slices.Sorted(maps.Keys(m)) where keys were gathered and then sorted: the module listing's directories, muxttest.Check's file names, and the template source files generation walks. sort is no longer imported anywhere in muxt's own code. - slices.ContainsFunc where a loop or an IndexFunc whose index went unused only asked whether something is there: marshalJSON refusing the execute callback, and a receiver method already in the interface. - slices.Clone for File.ImportSpecs' copy; its callers only range over the result or take its length. - sortedKeys in the generate snapshot harness appends every map's keys, sorts and compacts, rather than checking Contains before each append. Loops that find an element to change in place, build a different type, or return early with more than a yes or no stay as they were. Every generated file and report is unchanged. Assisted-by: Claude:claude-opus-5 gofumpt
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
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.
Each command builds its configuration and hands it to a runner. TestCommandLineConfigurations and TestCommandLineRejections state what command lines parse into, or why they are refused, without loading anything. TestChangeDirectory covers -C.
Part 5 of 8, stacked on #149. Replaces #145.
🤖 Generated with Claude Code
https://claude.ai/code/session_01UjkpA4fvZ7xQAsY65Beprp