test(cmd/mdsmith): add dedicated unit tests for runCheck/runFix - #784
Draft
jeduden wants to merge 4 commits into
Draft
test(cmd/mdsmith): add dedicated unit tests for runCheck/runFix#784jeduden wants to merge 4 commits into
jeduden wants to merge 4 commits into
Conversation
The 2026-08-02 architecture audit flagged runCheck and runFix — the check and fix CLI subcommand entry points — as missing dedicated in-process unit tests, only reachable via binary-spawn e2e tests. audit-checklist.md promotes a missing test on a CLI subcommand entry to blocker severity. Adds TestRunCheck_*/TestRunFix_* covering the flag-error, stdin, explicit-files, and config-discovered branches, matching the TestRunInit_* precedent already used for the init subcommand. Also records the full audit sweep in architecture-audit.md and files plans for the two structural tax findings (query-subcommand placement, internal/githooks package split). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016AbFuwvqqq2fBiYWa3ubap
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files
☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
- Strengthen TestRunCheck_Stdin_ChecksSource and TestRunCheck_Discovered_ChecksConfiguredFiles: both fed clean Markdown and asserted only exit code 0, which a silent stdin-routing or discovery bug could also produce. Switch to content with a trailing-space diagnostic and assert exit code 1 plus the diagnostic source. - Close the pipe read end in TestRunCheck_Stdin_ChecksSource, matching the close-both-ends convention used elsewhere in this file. - Correct the touched-file count in the audit log (148, not 134 — two-dot diff --stat is the accurate count) and the internal/githooks line count cited in plan/2608021916 (1,346, not 1,347). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016AbFuwvqqq2fBiYWa3ubap
Round 2 of the code-review cycle on PR #784 found that the stagingHelperShellFunc nice-to-have entry called it "the function" — it's a const string literal, not a function. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016AbFuwvqqq2fBiYWa3ubap
Contributor
There was a problem hiding this comment.
Pull request overview
Adds in-process unit tests for the mdsmith check/mdsmith fix CLI entrypoints (runCheck / runFix) as part of the 2026-08-02 architecture audit sweep, and records the audit results plus follow-up plans.
Changes:
- Add dedicated unit tests in
cmd/mdsmith/main_unit_test.gocoveringrunCheck/runFixrouting and key exit-code behaviors. - Append the 2026-08-02 sweep results to the architecture audit log and bump the
audit-fromcommit. - Add two new architecture “tax” plan files and register them in
PLAN.md.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| plan/2608021916_arch-fix-githooks-package-split.md | New plan capturing the SRP-driven split of internal/githooks into smaller responsibility-focused packages. |
| plan/2608021915_arch-fix-query-subcommand-placement.md | New plan capturing the refactor to move list query logic out of cmd/mdsmith/main.go. |
| PLAN.md | Registers the two newly added plan files in the plan catalog. |
| docs/development/architecture-audit.md | Records the 2026-08-02 audit findings, notes the new tests added, and updates the audit range. |
| cmd/mdsmith/main_unit_test.go | Adds the new unit tests for runCheck / runFix behaviors. |
Suppressed comments (2)
cmd/mdsmith/main_unit_test.go:862
runCheckresolves files vialoadAndResolve(), which callsloadConfig()and will walk up the directory tree to discover.mdsmith.ymlunless it hits a.gitboundary. Add a.gitmarker under the temp dir to keep the test isolated from any parent config and to avoid the rootward discovery walk.
dir := t.TempDir()
t.Chdir(dir)
require.NoError(t, os.WriteFile(filepath.Join(dir, "dirty.md"),
[]byte("# Title\n\nHello \n"), 0o644))
cmd/mdsmith/main_unit_test.go:920
runFix→fixFilesusesloadAndResolve(), which will try to discover config by walking up directories until a.gitboundary. Without a.gitmarker in the temp dir, this test can become environment-dependent (and does extra filesystem walking). Add the.gitmarker before callingrunFix.
dir := t.TempDir()
t.Chdir(dir)
path := filepath.Join(dir, "fixme.md")
require.NoError(t, os.WriteFile(path, []byte("# Title\n\nHello \n"), 0o644))
Copilot review on #784 flagged TestRunCheck_Stdin_ChecksSource, TestRunCheck_Files_ExitsOneOnDiagnostics, and TestRunFix_Files_FixesGivenFile: none write their own .mdsmith.yml, so loadConfig's upward discovery walk (internal/config/load.go) would search from the temp dir all the way to the filesystem root looking for one, since nothing marks the temp dir as a repo boundary. Add a gitBoundary test helper that drops a .git marker directory, matching the .git-boundary check config.Discover already stops at, and use it in the three affected tests. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016AbFuwvqqq2fBiYWa3ubap
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.
Summary
Scheduled architecture audit (2026-08-02) of the 134 files touched
since the last sweep, following
docs/development/architecture/audit-checklist.md.The one blocker:
runCheck(cmd/mdsmith/check.go) andrunFix(
cmd/mdsmith/fix.go) — thecheckandfixCLI subcommand entrypoints — had no dedicated in-process unit test, only binary-spawn e2e
coverage.
audit-checklist.mdpromotes a missing unit test toblocker severity when the function is on a public surface such as a
CLI subcommand entry.
TestRunCheck_UnknownFlag_ExitsTwo,TestRunCheck_Stdin_ChecksSource,TestRunCheck_Files_ExitsOneOnDiagnostics,TestRunCheck_Discovered_ChecksConfiguredFiles,TestRunFix_UnknownFlag_ExitsTwo,TestRunFix_StdinArg_ExitsTwo,TestRunFix_Files_FixesGivenFile, andTestRunFix_Discovered_FixesConfiguredFilestocmd/mdsmith/main_unit_test.go, matching theTestRunInit_*precedent set by the 2026-07-19 audit cycle.
docs/development/architecture-audit.md(blockers/tax/nice-to-have,bumps
audit-from).relocate
list querysubcommand logic out ofmain.go.split
internal/githooksby responsibility (SRP).reverse-layer imports, no Liskov breaks. The prior cycle's
isClaimeddedup (plan 2607191918) is confirmed resolved.Test plan
go build ./...go test ./cmd/mdsmith/...go vet ./...go tool -modfile=tools/go.mod golangci-lint run(0 issues)mdsmith check .(clean, 569 files)Generated by Claude Code