update the test coverage by jaxxstorm · Pull Request #51 · jaxxstorm/tscli

jaxxstorm · 2026-02-24T18:03:17Z

Signed-off-by: Lee Briggs lee@leebriggs.co.uk

Signed-off-by: Lee Briggs <lee@leebriggs.co.uk>

Copilot

Pull request overview

Adds a Go-based CLI testing and contract/coverage framework for tscli, including a pinned OpenAPI snapshot and a generated CLI↔OpenAPI coverage-gap report, plus CI/make targets to run these checks deterministically.

Changes:

Add CLI unit/integration test harnesses (mock API server, output assertions, manifest-based command coverage checks).
Pin Tailscale OpenAPI schema + metadata and add contract tests to validate snapshot integrity and command→operation mappings.
Add a coverage-gap report generator (JSON/Markdown + baseline diff) and wire it into CI and make targets.

Reviewed changes

Copilot reviewed 28 out of 33 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
`test/coveragegaps/main.go`	Implements coverage-gap report generation + baseline diffing.
`test/coveragegaps/main_test.go`	Adds basic tests for coverage-gap loader helpers.
`pkg/tscli/client.go`	Adds configurable API `base-url` override support in client creation.
`pkg/tscli/do_contract_test.go`	Adds tests for `Do` response decoding and error behavior.
`pkg/contract/openapi/tailscale-v2-openapi.yaml`	(Referenced) Pinned OpenAPI snapshot used by contract tests.
`pkg/contract/openapi/snapshot-metadata.yaml`	Records snapshot provenance + checksum.
`pkg/contract/openapi/openapi_snapshot_test.go`	Validates snapshot checksum/shape and mapping consistency.
`pkg/contract/openapi/command-operation-map.yaml`	Defines command→OpenAPI operation mappings.
`internal/testutil/apimock/server.go`	Adds an `httptest`-based API mock server utility.
`internal/testutil/apimock/fixtures.go`	Adds reusable JSON fixture helpers for mock responses.
`cmd/tscli/main.go`	Binds `TSCLI_BASE_URL` env var for base URL overrides.
`cmd/tscli/test_harness_test.go`	Adds shared CLI execution harness with captured stdout/stderr.
`cmd/tscli/output_assertions_test.go`	Adds output-mode assertions for json/yaml/pretty/human.
`cmd/tscli/local_commands_test.go`	Adds version command smoke test + config precedence test.
`cmd/tscli/group_integration_test.go`	Adds mock-backed integration tests across command groups.
`cmd/tscli/command_coverage_test.go`	Adds leaf-command manifest verification + help smoke tests.
`cmd/tscli/testdata/leaf_commands.txt`	Stores expected leaf command surface for coverage checks.
`cmd/tscli/get/device/cli_test.go`	Updates device command test transport routing logic.
`coverage/coverage-gaps.json`	Adds generated machine-readable coverage-gap report artifact.
`coverage/coverage-gaps.md`	Adds generated human-readable coverage-gap report artifact.
`coverage/coverage-gaps-baseline.json`	Adds baseline report used for regression diffing.
`coverage/coverage-gaps-diff.md`	Adds generated baseline diff artifact.
`openspec/changes/add-cli-test-coverage/.openspec.yaml`	Adds OpenSpec metadata for this change proposal.
`openspec/changes/add-cli-test-coverage/proposal.md`	Documents motivation and scope for test coverage work.
`openspec/changes/add-cli-test-coverage/design.md`	Documents test/contract/coverage design decisions.
`openspec/changes/add-cli-test-coverage/tasks.md`	Tracks implementation tasks for coverage initiative.
`openspec/changes/add-cli-test-coverage/coverage-gaps-baseline.md`	Provides baseline narrative for coverage gaps.
`openspec/changes/add-cli-test-coverage/specs/cli-command-test-coverage/spec.md`	Specifies command-surface coverage requirements.
`openspec/changes/add-cli-test-coverage/specs/tailscale-api-mock-integration-tests/spec.md`	Specifies mocked API integration test requirements.
`openspec/changes/add-cli-test-coverage/specs/tailscale-model-contract-consistency/spec.md`	Specifies contract consistency requirements vs pinned schema.
`Makefile`	Adds `test-` and `coverage-gaps-` make targets.
`README.md`	Documents new test targets and OpenAPI snapshot workflow.
`.github/workflows/ci.yml`	Adds CI workflow running Go tests + coverage-gap check + artifacts.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-24T18:09:31Z

pkg/tscli/client.go

+		parsed, err := url.Parse(baseURL)
+		if err != nil {
+			return nil, fmt.Errorf("invalid base-url: %w", err)
+		}


base-url validation is too permissive: url.Parse accepts relative URLs (e.g. "localhost:8080" or "/foo") without error, which will later fail during request construction with less clear errors. Consider validating that the parsed URL has an http/https scheme and a non-empty host before setting client.BaseURL.

Suggested change

}

}

if parsed.Scheme != "http" && parsed.Scheme != "https" {

return nil, fmt.Errorf("invalid base-url: scheme must be http or https, got %q", parsed.Scheme)

}

if parsed.Host == "" {

return nil, fmt.Errorf("invalid base-url: host must not be empty")

}

Copilot · 2026-02-24T18:09:32Z

test/coveragegaps/main.go

+	b.WriteString(fmt.Sprintf("- Uncovered operations: `%d`\n", len(rep.UncoveredOps)))
+	b.WriteString(fmt.Sprintf("- Covered commands: `%d`\n", len(rep.CoveredCommands)))
+	b.WriteString(fmt.Sprintf("- Unmapped commands: `%d`\n", len(rep.UnmappedCommands)))
+	b.WriteString(fmt.Sprintf("- Unknown mapped commands: `%d`\n", len(rep.UnknownCommands)))
+	b.WriteString("\n## Unmapped Commands\n\n")


The markdown report never outputs the actual UncoveredOps list (only the count) and jumps straight to unmapped commands. The requirements/docs in this PR describe the report as including lists of covered/uncovered operations, so consider adding an "Uncovered Operations" section (optionally truncated/top-N).

Copilot · 2026-02-24T18:09:32Z

cmd/tscli/command_coverage_test.go

+		children := nonBuiltinChildren(cmd)
+		if len(children) == 0 {
+			out = append(out, strings.Join(path, " "))
+			return
+		}
+		for _, child := range children {


leafCommands treats any command with children as non-leaf, even if it has a Run/RunE and is directly executable (e.g. get device/delete device have subcommands but also run). This means the manifest and coverage checks will miss runnable commands and will misclassify mappings as "unknown". Consider including runnable commands even when they have subcommands, or restructuring the command tree so runnable commands are true leaves.

Suggested change

children := nonBuiltinChildren(cmd)

if len(children) == 0 {

out = append(out, strings.Join(path, " "))

return

}

for _, child := range children {

// Treat any runnable command as a "leaf" for coverage/manifest purposes,

// even if it has subcommands.

if cmd.Runnable() {

out = append(out, strings.Join(path, " "))

}

for _, child := range nonBuiltinChildren(cmd) {

Copilot · 2026-02-24T18:09:32Z

Makefile

+
+test-unit:
+	go test ./test/coveragegaps ./pkg/...
+	go test ./cmd/tscli -run 'Test(Leaf|Version|Config|Do|Load)'


make test-unit / make test-integration omit several cmd/tscli tests (e.g. TestCreateKeyValidation, TestDeleteDeviceRequiresFlag, TestListDevicesAllFlag). That makes the documented test targets incomplete and can give a false sense of coverage. Consider broadening the -run patterns (or using build tags) so test-unit + test-integration cover all cmd/tscli tests.

Suggested change

go test ./cmd/tscli -run 'Test(Leaf|Version|Config|Do|Load)'

go test ./cmd/tscli

Copilot · 2026-02-24T18:09:33Z

cmd/tscli/test_harness_test.go

+	rOut, wOut, _ := os.Pipe()
+	rErr, wErr, _ := os.Pipe()


os.Pipe() errors are ignored here. If pipe creation fails, the test harness can deadlock or panic later when copying/closing. Handle the returned errors and fail the test early (e.g. t.Fatalf) or return an error from captureStdIO.

Suggested change

rOut, wOut, _ := os.Pipe()

rErr, wErr, _ := os.Pipe()

rOut, wOut, err := os.Pipe()

if err != nil {

return "", "", err

}

rErr, wErr, err := os.Pipe()

if err != nil {

_ = rOut.Close()

_ = wOut.Close()

return "", "", err

}

Copilot · 2026-02-24T18:09:33Z

test/coveragegaps/main.go

+	if err := os.MkdirAll(filepath.Dir(*jsonOut), 0o755); err != nil {
+		fatalf("create output directory: %v", err)
+	}
+	if err := writeJSON(*jsonOut, rep); err != nil {
+		fatalf("write json report: %v", err)


Only the directory for --json-out is created. If --md-out or --diff-out point to a different (non-existent) directory, the tool will fail when writing those files. Consider creating output directories for each output path (or using the common parent directory of all outputs).

update the test coverage

ab0f924

Signed-off-by: Lee Briggs <lee@leebriggs.co.uk>

Copilot AI review requested due to automatic review settings February 24, 2026 18:03

Copilot started reviewing on behalf of jaxxstorm February 24, 2026 18:03 View session

jaxxstorm merged commit f16e810 into main Feb 24, 2026
3 checks passed

jaxxstorm deleted the coverage branch February 24, 2026 18:08

Copilot AI reviewed Feb 24, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update the test coverage#51

update the test coverage#51
jaxxstorm merged 1 commit intomainfrom
coverage

jaxxstorm commented Feb 24, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 24, 2026

Uh oh!

Copilot AI Feb 24, 2026

Uh oh!

Copilot AI Feb 24, 2026

Uh oh!

Copilot AI Feb 24, 2026

Uh oh!

Copilot AI Feb 24, 2026

Uh oh!

Copilot AI Feb 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-		}
+		}
+		if parsed.Scheme != "http" && parsed.Scheme != "https" {
+			return nil, fmt.Errorf("invalid base-url: scheme must be http or https, got %q", parsed.Scheme)
+		}
+		if parsed.Host == "" {
+			return nil, fmt.Errorf("invalid base-url: host must not be empty")
+		}

	go test ./cmd/tscli -run 'Test(Leaf\|Version\|Config\|Do\|Load)'
	go test ./cmd/tscli

-	rOut, wOut, _ := os.Pipe()
-	rErr, wErr, _ := os.Pipe()
+	rOut, wOut, err := os.Pipe()
+	if err != nil {
+		return "", "", err
+	}
+	rErr, wErr, err := os.Pipe()
+	if err != nil {
+		_ = rOut.Close()
+		_ = wOut.Close()
+		return "", "", err
+	}

Conversation

jaxxstorm commented Feb 24, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants