Skip to content

Commit 66df147

Browse files
LeadGoEngineerPaperclip-Paperclip
andcommitted
feat(compat): add Subcommands affordance to Runner for cobra-based CLIs
The original Runner shape assumed --since/--until lived on the root binary. crono is cobra-based — date flags live on `biometrics`, `exercises`, `nutrition`, `servings`, `notes` — so the dates suite red-x'd HelpDocumentsDateFlags against it. Liftoff and Withings will hit the same pattern. Changes: - Runner gains a Subcommands []string field declaring the subcommands under which the contract surface lives, plus a WithSubcommand(name) composition helper that prepends the subcommand to argv on Run. - dates.RunContract dispatches per-subcommand when Subcommands is non-empty, scoping each pass under `subcommand=NAME/...` so any single-subcommand regression surfaces as a named failure rather than masking the rest. Flat CLIs leave Subcommands empty and the bundle runs against the root binary unchanged. - stubcli grows a second mode (STUBCLI_MODE=cobra) that mirrors a cobra-based CLI: root --help lists subcommands without mentioning the date flags, and only `biometrics` carries --since/--until. The cobra-mode self-test fails fast if the Runner ever stops prepending the subcommand. - New compat/internal/argecho helper and compat_test.go give the WithSubcommand prepend a focused unit test against a binary that just echoes os.Args. - README + CONTRIBUTING document the affordance and the cobra-style integration shape. Addresses LeadGoEngineer review on PR #6 — the "first machine-attested contract test" should not ship with a known gap that prevents its only consumer from going green. Refs CONTRACT.md §3. Co-Authored-By: Paperclip <[email protected]>
1 parent 1882283 commit 66df147

8 files changed

Lines changed: 340 additions & 26 deletions

File tree

CONTRIBUTING.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +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 (`compat/dates`, future `compat/formats`, `compat/auth`, `compat/prime`). Each 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.
7980
- 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.
8081
- 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.
8182
- 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: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,28 @@ func TestContractDates(t *testing.T) {
3737
}
3838
```
3939

40-
Then in your CI workflow:
40+
### Cobra-based CLIs: date flags on subcommands
41+
42+
When `--since`/`--until` live on subcommands (the crono / liftoff / withings pattern), set `Subcommands` and the suite will dispatch per-subcommand:
43+
44+
```go
45+
func TestContractDates(t *testing.T) {
46+
bin := os.Getenv("EXPORT_CLI_BIN")
47+
if bin == "" {
48+
t.Skip("EXPORT_CLI_BIN not set; skipping compat suite")
49+
}
50+
dates.RunContract(t, compat.Runner{
51+
Binary: bin,
52+
Subcommands: []string{
53+
"biometrics", "exercises", "nutrition", "servings", "notes",
54+
},
55+
})
56+
}
57+
```
58+
59+
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.
60+
61+
### CI workflow
4162

4263
```yaml
4364
- name: build
@@ -59,6 +80,8 @@ The exporter does not need a separate `go.mod` for compat tests — the standard
5980
| `HelpIsHermetic` | §5 | `--help` succeeds with all HTTP proxies pointed at an unreachable address. |
6081
| `FlagValidationIsHermetic` | §5 | A parse failure also produces no successful outbound request. |
6182

83+
When `compat.Runner.Subcommands` is set, every row above runs once per declared subcommand under `subcommand=NAME/...`.
84+
6285
## What it does NOT cover yet
6386

6487
The actual local-midnight semantics of `--since 2026-04-15` (the harmonization that just landed across crono/liftoff/withings) is still **human-attested** in the status table. Asserting it black-box requires either:
@@ -78,3 +101,5 @@ When that affordance lands, the test belongs here as `dates.LocalMidnightSemanti
78101
## Self-test
79102

80103
This module has its own test that runs the suite against a stub CLI in `internal/stubcli/`. The stub is intentionally narrow — it exists so `go test ./...` from this module's root proves the library compiles and the assertions fire correctly, without depending on any of the real export-CLIs. Failures in the self-test mean the library has a bug; failures in an exporter's compat test mean the exporter drifted from the contract.
104+
105+
The stub has two modes (`STUBCLI_MODE=flat` and `STUBCLI_MODE=cobra`). The flat-mode self-test exercises the original Runner shape; the cobra-mode self-test exercises `Subcommands`-based dispatch. In cobra mode, the stub's root `--help` deliberately omits `--since/--until`, so the cobra-mode self-test fails fast if `compat.Runner` ever stops prepending the subcommand. There is also a focused unit test for `Runner.WithSubcommand` using an `argecho` helper that just prints `os.Args`.

compat/compat.go

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,29 @@ type Runner struct {
3636

3737
// Timeout is the per-invocation timeout. Zero means 10 seconds.
3838
Timeout time.Duration
39+
40+
// Subcommands declares the subcommands under which the contract
41+
// surface lives — for CLIs (typically cobra-based) where flags like
42+
// --since and --until are attached to data-producing subcommands
43+
// rather than the root binary. Examples: crono's `biometrics`,
44+
// `exercises`, `nutrition`, `servings`, `notes` each accept their
45+
// own --since/--until.
46+
//
47+
// Empty means the surface is on the root binary; section bundles
48+
// invoke the binary directly. Non-empty means each section's
49+
// RunContract iterates the list and verifies the contract once per
50+
// subcommand via t.Run, so a regression in any single subcommand
51+
// surfaces as a named subtest failure rather than masking the rest.
52+
//
53+
// The Runner itself does not look at this field; section bundles
54+
// (e.g. compat/dates) read it and dispatch via WithSubcommand.
55+
Subcommands []string
56+
57+
// subcommand, when non-empty, is prepended to args on every Run
58+
// call. Set via WithSubcommand; section bundles use it to dispatch
59+
// per-subcommand. Callers do not need to set it directly — set
60+
// Subcommands instead and let the bundle compose the dispatch.
61+
subcommand string
3962
}
4063

4164
// Result captures everything observable about one CLI invocation. All
@@ -68,7 +91,11 @@ func (r Runner) Run(ctx context.Context, args ...string) (Result, error) {
6891
runCtx, cancel := context.WithTimeout(ctx, timeout)
6992
defer cancel()
7093

71-
cmd := exec.CommandContext(runCtx, r.Binary, args...)
94+
fullArgs := args
95+
if r.subcommand != "" {
96+
fullArgs = append([]string{r.subcommand}, args...)
97+
}
98+
cmd := exec.CommandContext(runCtx, r.Binary, fullArgs...)
7299
// Default to an empty env so tests are hermetic. Callers opt into
73100
// passing TZ, HOME, etc. via Runner.Env.
74101
if r.Env != nil {
@@ -118,3 +145,22 @@ func (r Runner) WithEnv(kv ...string) Runner {
118145
out.Env = append(append([]string(nil), r.Env...), kv...)
119146
return out
120147
}
148+
149+
// WithSubcommand returns a copy of r whose Run prepends sub as the
150+
// first command-line argument. Section bundles use this internally to
151+
// dispatch per-subcommand when Runner.Subcommands is non-empty;
152+
// integrators normally set Subcommands and let the bundle do it.
153+
//
154+
// Calling WithSubcommand again replaces (not stacks) the previous
155+
// value; nested subcommand paths are out of scope for the current
156+
// contract.
157+
func (r Runner) WithSubcommand(sub string) Runner {
158+
out := r
159+
out.subcommand = sub
160+
return out
161+
}
162+
163+
// Subcommand returns the subcommand that Run will prepend to args, or
164+
// the empty string if none is set. Section bundles use this in subtest
165+
// names so failures point at the offending subcommand.
166+
func (r Runner) Subcommand() string { return r.subcommand }

compat/compat_test.go

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
package compat_test
2+
3+
import (
4+
"context"
5+
"os/exec"
6+
"path/filepath"
7+
"runtime"
8+
"strings"
9+
"testing"
10+
"time"
11+
12+
"github.com/quantcli/common/compat"
13+
)
14+
15+
// TestWithSubcommand_PrependsArg checks that WithSubcommand causes Run
16+
// to emit the subcommand as argv[1] in front of the caller's args.
17+
// We use a small Go program that echoes its os.Args back so the assertion
18+
// is independent of any system command's flag semantics.
19+
func TestWithSubcommand_PrependsArg(t *testing.T) {
20+
bin := buildArgEcho(t)
21+
r := compat.Runner{Binary: bin}.WithSubcommand("biometrics")
22+
23+
res, err := r.Run(context.Background(), "--help")
24+
if err != nil {
25+
t.Fatalf("run: %v", err)
26+
}
27+
if res.ExitCode != 0 {
28+
t.Fatalf("exit %d; stderr=%q", res.ExitCode, res.StderrString())
29+
}
30+
got := strings.TrimRight(res.StdoutString(), "\n")
31+
want := "biometrics\n--help"
32+
if got != want {
33+
t.Errorf("argv mismatch:\n got:\n%s\nwant:\n%s", got, want)
34+
}
35+
}
36+
37+
// TestRun_NoSubcommandPassthrough checks that the default zero
38+
// subcommand leaves args untouched.
39+
func TestRun_NoSubcommandPassthrough(t *testing.T) {
40+
bin := buildArgEcho(t)
41+
r := compat.Runner{Binary: bin}
42+
43+
res, err := r.Run(context.Background(), "--help")
44+
if err != nil {
45+
t.Fatalf("run: %v", err)
46+
}
47+
got := strings.TrimRight(res.StdoutString(), "\n")
48+
if got != "--help" {
49+
t.Errorf("argv mismatch: got %q want %q", got, "--help")
50+
}
51+
}
52+
53+
// TestWithSubcommand_DoesNotMutateReceiver asserts that WithSubcommand
54+
// returns a copy and leaves the parent runner unchanged. Section
55+
// bundles rely on this when iterating Subcommands.
56+
func TestWithSubcommand_DoesNotMutateReceiver(t *testing.T) {
57+
bin := buildArgEcho(t)
58+
parent := compat.Runner{Binary: bin}
59+
_ = parent.WithSubcommand("biometrics")
60+
if parent.Subcommand() != "" {
61+
t.Errorf("parent.Subcommand() = %q; want empty after WithSubcommand on copy", parent.Subcommand())
62+
}
63+
}
64+
65+
// TestRun_TimeoutReturnsError exercises the timeout branch so the
66+
// Runner's error contract (non-zero exit is not an error; timeout is)
67+
// stays load-bearing.
68+
func TestRun_TimeoutReturnsError(t *testing.T) {
69+
sleeper, err := exec.LookPath("sleep")
70+
if err != nil {
71+
t.Skip("sleep not on PATH; skipping timeout exercise")
72+
}
73+
r := compat.Runner{Binary: sleeper, Timeout: 50 * time.Millisecond}
74+
_, runErr := r.Run(context.Background(), "5")
75+
if runErr == nil {
76+
t.Fatal("expected timeout error, got nil")
77+
}
78+
if !strings.Contains(runErr.Error(), "timed out") {
79+
t.Errorf("expected timeout error, got: %v", runErr)
80+
}
81+
}
82+
83+
// buildArgEcho compiles the tiny argecho helper into a temp dir. It is
84+
// the simplest possible echo-args binary: it prints each os.Args[1:]
85+
// entry on its own line to stdout. We build it instead of relying on
86+
// /bin/echo so the test works on any GOOS.
87+
func buildArgEcho(t *testing.T) string {
88+
t.Helper()
89+
out := filepath.Join(t.TempDir(), "argecho")
90+
if runtime.GOOS == "windows" {
91+
out += ".exe"
92+
}
93+
cmd := exec.Command("go", "build", "-o", out, "github.com/quantcli/common/compat/internal/argecho")
94+
if output, err := cmd.CombinedOutput(); err != nil {
95+
t.Fatalf("go build argecho: %v\n%s", err, output)
96+
}
97+
return out
98+
}

compat/dates/dates.go

Lines changed: 39 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,10 @@
33
//
44
// What is machine-attested here:
55
//
6-
// - The CLI documents `--since` and `--until` in its top-level help.
6+
// - The CLI documents `--since` and `--until` in the help output of
7+
// whatever entry point owns them — the root binary for flat CLIs,
8+
// or each declared subcommand for cobra-based CLIs (set via
9+
// compat.Runner.Subcommands).
710
// - An invalid `--since` value produces a non-zero exit with an error
811
// on stderr and an empty stdout.
912
// - A flag-validation failure does not perform a network request.
@@ -52,12 +55,37 @@ import (
5255
// the others. The bundle is safe to run in parallel with other compat
5356
// suites; individual subtests are not marked parallel because they
5457
// shell out to the same binary and the OS may serialize them anyway.
58+
//
59+
// If r.Subcommands is non-empty, RunContract iterates the list and
60+
// runs the four assertions once per subcommand under a
61+
// "subcommand=NAME" t.Run group. This is how cobra-based CLIs (e.g.
62+
// crono's `biometrics`, `exercises`, `nutrition`, `servings`, `notes`)
63+
// attest the contract on every data-producing subcommand. If empty,
64+
// the assertions run once against the root binary — the right shape
65+
// for CLIs that put --since/--until at the top level.
5566
func RunContract(t *testing.T, r compat.Runner) {
5667
t.Helper()
5768
if r.Binary == "" {
5869
t.Fatal("dates: compat.Runner.Binary is empty")
5970
}
6071

72+
if len(r.Subcommands) == 0 {
73+
runContractOne(t, r)
74+
return
75+
}
76+
for _, sub := range r.Subcommands {
77+
sub := sub
78+
t.Run("subcommand="+sub, func(t *testing.T) {
79+
runContractOne(t, r.WithSubcommand(sub))
80+
})
81+
}
82+
}
83+
84+
// runContractOne runs the four date-flag assertions against a single
85+
// invocation surface — either the root binary (when r has no
86+
// subcommand prefix) or a specific subcommand of it.
87+
func runContractOne(t *testing.T, r compat.Runner) {
88+
t.Helper()
6189
t.Run("HelpDocumentsDateFlags", func(t *testing.T) {
6290
helpDocumentsDateFlags(t, r)
6391
})
@@ -73,9 +101,11 @@ func RunContract(t *testing.T, r compat.Runner) {
73101
}
74102

75103
// helpDocumentsDateFlags asserts that the CLI documents `--since` and
76-
// `--until` somewhere in its top-level `--help` output. This is the
77-
// minimum binding between the contract and the binary: an exporter that
78-
// quietly drops one of the flags will fail this test.
104+
// `--until` somewhere in the `--help` output of the configured entry
105+
// point — root binary or subcommand, depending on how the integrator
106+
// configured compat.Runner. This is the minimum binding between the
107+
// contract and the binary: an exporter that quietly drops one of the
108+
// flags will fail this test.
79109
func helpDocumentsDateFlags(t *testing.T, r compat.Runner) {
80110
t.Helper()
81111
res := r.MustRun(t, "--help")
@@ -97,12 +127,11 @@ func helpDocumentsDateFlags(t *testing.T, r compat.Runner) {
97127
// invalidSinceValueFails asserts that a malformed `--since` value causes
98128
// the CLI to exit non-zero with an error on stderr and an empty stdout.
99129
//
100-
// The flag is passed to whatever subcommand the CLI puts first; we use a
101-
// known-bad value so any subcommand that accepts `--since` will reject
102-
// it at parse time. If a CLI accepts `--since` only on subcommands, the
103-
// integrator overrides this test by passing their own subcommand name
104-
// via a future SubcommandHint hook — for now the contract requires that
105-
// at least one entry point rejects malformed dates.
130+
// We use a known-bad value so any entry point that accepts `--since`
131+
// will reject it at parse time. For cobra-based CLIs whose date flags
132+
// live on subcommands, the integrator sets compat.Runner.Subcommands;
133+
// RunContract then dispatches via WithSubcommand and this assertion
134+
// runs once per declared subcommand.
106135
func invalidSinceValueFails(t *testing.T, r compat.Runner) {
107136
t.Helper()
108137
// "obviously-not-a-date" should never parse as a keyword, absolute

compat/dates/dates_test.go

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,35 @@ import (
1010
"github.com/quantcli/common/compat/dates"
1111
)
1212

13-
// TestRunContract_AgainstStub builds the in-tree stub binary and runs
14-
// the compat suite against it. This is the library's own gate: if the
15-
// stub (which is contract-compliant by construction) starts failing
16-
// these tests, the library has a bug, not the stub.
17-
func TestRunContract_AgainstStub(t *testing.T) {
13+
// TestRunContract_AgainstStubFlat builds the in-tree stub binary in its
14+
// default (flat) mode and runs the compat suite against it. This is the
15+
// library's own gate for the original Runner shape: if the stub (which
16+
// is contract-compliant by construction) starts failing these tests,
17+
// the library has a bug, not the stub.
18+
func TestRunContract_AgainstStubFlat(t *testing.T) {
1819
bin := buildStub(t)
1920
dates.RunContract(t, compat.Runner{Binary: bin})
2021
}
2122

23+
// TestRunContract_AgainstStubSubcommand runs the same suite against
24+
// the stub in cobra mode, with a Subcommands declaration so the Runner
25+
// dispatches per-subcommand. In cobra mode the stub's root --help does
26+
// NOT mention --since/--until — only `biometrics --help` does — so this
27+
// test fails fast if compat.Runner ever stops prepending the
28+
// subcommand.
29+
//
30+
// This is the gate that pins down the contract for crono / liftoff /
31+
// withings, whose date flags live on cobra subcommands.
32+
func TestRunContract_AgainstStubSubcommand(t *testing.T) {
33+
bin := buildStub(t)
34+
r := compat.Runner{
35+
Binary: bin,
36+
Env: []string{"STUBCLI_MODE=cobra"},
37+
Subcommands: []string{"biometrics"},
38+
}
39+
dates.RunContract(t, r)
40+
}
41+
2242
// buildStub compiles the stub CLI into a temp directory and returns the
2343
// absolute path. It uses `go build` rather than relying on a checked-in
2444
// binary so the test is reproducible across platforms.

compat/internal/argecho/main.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// Command argecho prints each of its os.Args[1:] entries on its own
2+
// line to stdout, then exits zero. It is used by compat_test.go to
3+
// verify how Runner constructs argv (notably the WithSubcommand
4+
// prepend behavior) without depending on any system command's flag
5+
// parsing.
6+
package main
7+
8+
import (
9+
"fmt"
10+
"os"
11+
)
12+
13+
func main() {
14+
for _, a := range os.Args[1:] {
15+
fmt.Println(a)
16+
}
17+
}

0 commit comments

Comments
 (0)