Skip to content

Commit de9219e

Browse files
Terastar-PaperclipLeadGoEngineerPaperclip-Paperclip
authored
feat(compat): support nested subcommand paths in Runner.WithSubcommand (#12)
Liftoff-export's data-producing leaves live two levels deep (`liftoff-export workouts stats`, `liftoff-export bodyweights list`). The pre-existing Runner.WithSubcommand passed the value as a single argv entry, so a nested path arrived at cobra as one literal "workouts stats" command name and was rejected as unknown. Switch the internal representation to []string, split via strings.Fields on WithSubcommand intake, and prepend each segment as its own argv entry. Single-word callers (crono's `biometrics`, `exercises`, ...) keep working unchanged because Fields("biometrics") returns ["biometrics"]. The empty string still clears the path, so the API stays reversible without an extra method. Subcommand() returns the path joined with single spaces, which is what callers passed in. Section bundles already use it for t.Run("subcommand="+sub, ...) names, so nested-path failures surface as `subcommand=workouts stats/...` subtests instead of masking. Two new self-tests in compat_test.go lock in the split-on-whitespace and empty-clear behaviors via the existing argecho helper: - TestWithSubcommand_NestedPathSplitsOnWhitespace exercises the liftoff shape end-to-end (two segments + flag args). - TestWithSubcommand_EmptyStringClears asserts the reset path. README + CONTRIBUTING gain a one-paragraph note pointing partial- codec exporters at the nested-path usage, so future onboarding heartbeats don't trip on the same surprise. Unblocks QUA-16 (liftoff onboarding as second consumer of compat/formats). Co-authored-by: LeadGoEngineer <leadgoengineer@quantcli.local> Co-authored-by: Paperclip <noreply@paperclip.ing>
1 parent f2ca5ac commit de9219e

4 files changed

Lines changed: 99 additions & 19 deletions

File tree

CONTRIBUTING.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ Rules:
7676
- Anyone changing `CONTRACT.md` is also expected to update or add tests under `compat/` that exercise the new behavior against every `*-export-cli`.
7777
- The harness is deliberately black-box: it shells out to the binary and asserts on stdout, stderr, and exit code only. It must not import a CLI's internal packages.
7878
- One subpackage per contract section. The naming convention is `compat/<section>` where `<section>` is the CONTRACT.md section being attested — currently `compat/dates` (§2–§3) and `compat/formats` (§4); `compat/auth` (§5) and `compat/prime` (§6) are expected to follow. Each subpackage exposes a single entry point — `RunContract(t, runner)` — that exporters call from one build-tagged `_test.go` file.
79-
- Cobra-based exporters whose contract surface lives on subcommands set `compat.Runner.Subcommands`; section bundles dispatch per-subcommand under a `subcommand=NAME/...` subtree. Flat CLIs leave the field empty and the bundle runs against the root binary.
79+
- Cobra-based exporters whose contract surface lives on subcommands set `compat.Runner.Subcommands`; section bundles dispatch per-subcommand under a `subcommand=NAME/...` subtree. Flat CLIs leave the field empty and the bundle runs against the root binary. Nested paths (e.g. `liftoff-export workouts stats`) are passed as a single space-separated entry — the dispatcher splits on whitespace so each segment is its own argv entry.
8080
- A PR that changes the contract without touching `compat/` is incomplete. Either update the tests in the same PR or open a follow-up issue and link it from the PR body before merging — the Lead Go Engineer holds the line on this.
8181
- Compat tests run in CI on every PR and on `main`. A failing compat test on `main` means at least one shipped CLI no longer matches the contract, and that's a release-blocker incident, not a flake.
8282
- The Status table in `CONTRACT.md` distinguishes **machine-attested** rows (covered by `compat/`) from **human-attested** rows (still verified by reviewer judgment). Promoting a row from human to machine attestation is itself a worthwhile PR.

compat/README.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,20 @@ func TestContractDates(t *testing.T) {
6767

6868
Each subcommand is verified under a `subcommand=NAME/...` subtree, so a regression in any single one fails as a named subtest instead of masking the rest.
6969

70+
Nested-path subcommands are supported by passing a space-separated string. The dispatcher splits on whitespace so each segment becomes its own argv entry, which is what cobra's command tree resolves against:
71+
72+
```go
73+
dates.RunContract(t, compat.Runner{
74+
Binary: bin,
75+
Subcommands: []string{
76+
"bodyweights list", "bodyweights stats",
77+
"workouts list", "workouts stats",
78+
},
79+
})
80+
```
81+
82+
This is the pattern liftoff-export uses, where `--since`/`--until` live on two-level leaves (`liftoff-export workouts stats`) rather than top-level subcommands.
83+
7084
### CI workflow
7185

7286
```yaml

compat/compat.go

Lines changed: 38 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"errors"
1919
"fmt"
2020
"os/exec"
21+
"strings"
2122
"testing"
2223
"time"
2324
)
@@ -44,6 +45,14 @@ type Runner struct {
4445
// `exercises`, `nutrition`, `servings`, `notes` each accept their
4546
// own --since/--until.
4647
//
48+
// Each entry is one space-separated subcommand path. Single-word
49+
// entries like "biometrics" target a top-level subcommand; nested
50+
// paths like "workouts stats" target a leaf under a parent (as in
51+
// liftoff's `liftoff-export workouts stats`). The dispatcher splits
52+
// on whitespace before invoking the binary, so the cobra command
53+
// tree receives separate argv entries rather than one space-joined
54+
// argument.
55+
//
4756
// Empty means the surface is on the root binary; section bundles
4857
// invoke the binary directly. Non-empty means each section's
4958
// RunContract iterates the list and verifies the contract once per
@@ -109,11 +118,13 @@ type Runner struct {
109118
// SkipDataPath: true is the typical liftoff/crono shape today.
110119
SkipDataPath bool
111120

112-
// subcommand, when non-empty, is prepended to args on every Run
113-
// call. Set via WithSubcommand; section bundles use it to dispatch
114-
// per-subcommand. Callers do not need to set it directly — set
115-
// Subcommands instead and let the bundle compose the dispatch.
116-
subcommand string
121+
// subcommandParts, when non-empty, is prepended to args on every Run
122+
// call as separate argv entries. Set via WithSubcommand from a
123+
// space-separated path (e.g. "workouts stats"); section bundles use
124+
// it to dispatch per-subcommand. Callers do not need to set it
125+
// directly — set Subcommands instead and let the bundle compose the
126+
// dispatch.
127+
subcommandParts []string
117128
}
118129

119130
// Result captures everything observable about one CLI invocation. All
@@ -147,8 +158,8 @@ func (r Runner) Run(ctx context.Context, args ...string) (Result, error) {
147158
defer cancel()
148159

149160
fullArgs := args
150-
if r.subcommand != "" {
151-
fullArgs = append([]string{r.subcommand}, args...)
161+
if len(r.subcommandParts) > 0 {
162+
fullArgs = append(append([]string(nil), r.subcommandParts...), args...)
152163
}
153164
cmd := exec.CommandContext(runCtx, r.Binary, fullArgs...)
154165
// Default to an empty env so tests are hermetic. Callers opt into
@@ -207,24 +218,33 @@ func (r Runner) WithEnv(kv ...string) Runner {
207218
return out
208219
}
209220

210-
// WithSubcommand returns a copy of r whose Run prepends sub as the
211-
// first command-line argument. Section bundles use this internally to
212-
// dispatch per-subcommand when Runner.Subcommands is non-empty;
213-
// integrators normally set Subcommands and let the bundle do it.
221+
// WithSubcommand returns a copy of r whose Run prepends sub as a
222+
// command-line path before the caller's args. Section bundles use this
223+
// internally to dispatch per-subcommand when Runner.Subcommands is
224+
// non-empty; integrators normally set Subcommands and let the bundle
225+
// do it.
226+
//
227+
// sub is a space-separated path: "biometrics" targets a single
228+
// top-level subcommand; "workouts stats" targets a leaf under a parent
229+
// (each whitespace-separated word becomes its own argv entry, so the
230+
// cobra command tree resolves them as separate names rather than one
231+
// space-joined argument). Leading/trailing whitespace is ignored; the
232+
// empty string clears any previously-set path.
214233
//
215234
// Calling WithSubcommand again replaces (not stacks) the previous
216-
// value; nested subcommand paths are out of scope for the current
217-
// contract.
235+
// path.
218236
func (r Runner) WithSubcommand(sub string) Runner {
219237
out := r
220-
out.subcommand = sub
238+
out.subcommandParts = strings.Fields(sub)
221239
return out
222240
}
223241

224-
// Subcommand returns the subcommand that Run will prepend to args, or
225-
// the empty string if none is set. Section bundles use this in subtest
226-
// names so failures point at the offending subcommand.
227-
func (r Runner) Subcommand() string { return r.subcommand }
242+
// Subcommand returns the subcommand path that Run will prepend to
243+
// args, or the empty string if none is set. Nested paths come back
244+
// joined with single spaces (e.g. "workouts stats"). Section bundles
245+
// use this in subtest names so failures point at the offending
246+
// subcommand.
247+
func (r Runner) Subcommand() string { return strings.Join(r.subcommandParts, " ") }
228248

229249
// SupportsFormat reports whether name is one of the §4 codecs the
230250
// exporter declares it implements.

compat/compat_test.go

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,52 @@ func TestWithSubcommand_PrependsArg(t *testing.T) {
3434
}
3535
}
3636

37+
// TestWithSubcommand_NestedPathSplitsOnWhitespace covers liftoff-style
38+
// CLIs whose data-producing leaves live two levels deep
39+
// (e.g. `liftoff-export workouts stats`). Without the split, the cobra
40+
// resolver would receive one literal "workouts stats" argv entry and
41+
// reject it as an unknown command. The argecho-based assertion locks
42+
// in that each whitespace-separated word is its own argv entry, in
43+
// order, before the caller's args.
44+
func TestWithSubcommand_NestedPathSplitsOnWhitespace(t *testing.T) {
45+
bin := buildArgEcho(t)
46+
r := compat.Runner{Binary: bin}.WithSubcommand("workouts stats")
47+
48+
res, err := r.Run(context.Background(), "--format", "json")
49+
if err != nil {
50+
t.Fatalf("run: %v", err)
51+
}
52+
if res.ExitCode != 0 {
53+
t.Fatalf("exit %d; stderr=%q", res.ExitCode, res.StderrString())
54+
}
55+
got := strings.TrimRight(res.StdoutString(), "\n")
56+
want := "workouts\nstats\n--format\njson"
57+
if got != want {
58+
t.Errorf("argv mismatch:\n got:\n%s\nwant:\n%s", got, want)
59+
}
60+
if r.Subcommand() != "workouts stats" {
61+
t.Errorf("Subcommand() = %q; want %q", r.Subcommand(), "workouts stats")
62+
}
63+
}
64+
65+
// TestWithSubcommand_EmptyStringClears asserts that passing the empty
66+
// string (or whitespace-only) clears any previously-set path, which
67+
// keeps the API reversible without an extra method.
68+
func TestWithSubcommand_EmptyStringClears(t *testing.T) {
69+
bin := buildArgEcho(t)
70+
r := compat.Runner{Binary: bin}.WithSubcommand("biometrics").WithSubcommand("")
71+
if r.Subcommand() != "" {
72+
t.Errorf("Subcommand() = %q; want empty after clear", r.Subcommand())
73+
}
74+
res, err := r.Run(context.Background(), "--help")
75+
if err != nil {
76+
t.Fatalf("run: %v", err)
77+
}
78+
if got := strings.TrimRight(res.StdoutString(), "\n"); got != "--help" {
79+
t.Errorf("argv mismatch: got %q want %q", got, "--help")
80+
}
81+
}
82+
3783
// TestRun_NoSubcommandPassthrough checks that the default zero
3884
// subcommand leaves args untouched.
3985
func TestRun_NoSubcommandPassthrough(t *testing.T) {

0 commit comments

Comments
 (0)