Unit tests for `pkg/vmcp/cli/` serve and validate logic

[yrobla]

Description

Add a comprehensive unit test suite in pkg/vmcp/cli/ for the Serve and Validate entry points and their helper functions (loadAndValidateConfig, discoverBackends, createSessionFactory, loadAuthServerConfig). Tests use go.uber.org/mock (gomock) and write temporary config files to disk; no running server or Kubernetes cluster is required. This work was deliberately separated from #4879 to keep each PR within the 400-line / 10-file review limit.

Context

#4879 creates pkg/vmcp/cli/ as a new library package, but intentionally defers thorough unit test coverage to keep its PR size manageable. This item delivers that coverage. Because pkg/vmcp/cli/ is business logic (not a thin CLI wrapper), organizational standards require thorough unit tests alongside the source files — the same principle that governs every other package under pkg/. The tests validate the config loading pipeline, validation rejection paths, backend discovery wiring, session factory HMAC-secret behavior (including the Kubernetes fail-fast rule), and the auth server config side-loading logic.

Dependencies: Depends on #4879 (the pkg/vmcp/cli/ package must exist before tests can be written against it)
Blocks: (none — can merge independently after #4879)

Acceptance Criteria

• pkg/vmcp/cli/serve_test.go and pkg/vmcp/cli/validate_test.go exist, each carrying the required SPDX copyright header
• loadAndValidateConfig unit tests cover: valid config round-trip, missing config file (error), malformed YAML (error), and config that fails semantic validation (error)
• loadAuthServerConfig unit tests cover: file absent (returns nil, nil), valid YAML file (returns parsed struct), and malformed YAML file (returns error)
• discoverBackends unit tests cover: static mode (non-empty cfg.Backends slice → NewUnifiedBackendDiscovererWithStaticBackends path) and empty-backends warning path
• createSessionFactory unit tests cover: HMAC secret set via env var (factory created successfully), no secret + non-Kubernetes environment (factory created with warning), and no secret + Kubernetes environment (error returned)
• Validate unit tests cover: missing config path (error), valid config file (nil return), and invalid config (error)
• All test cases use t.Cleanup for teardown (not defer) and random ports via net.Listen("tcp", ":0") where port allocation is needed
• Mocks for aggregator.Aggregator, aggregator.BackendDiscoverer, and vmcpauth.OutgoingAuthRegistry are sourced from the existing generated mocks in pkg/vmcp/aggregator/mocks/ and pkg/vmcp/client/mocks/; no hand-written mocks are introduced
• go test ./pkg/vmcp/cli/... passes with no failures
• go vet ./pkg/vmcp/cli/... passes with no issues
• No new external Go module dependencies are introduced
• All tests pass
• Code reviewed and approved

Technical Approach

Recommended Implementation

Write two test files (serve_test.go and validate_test.go) in package cli (same package as the source, allowing access to unexported helpers if needed) or package cli_test (external test package). Prefer package cli_test unless a helper function is unexported and cannot be tested through the exported API — in that case use package cli to keep coverage complete.

For each function under test:

1. Write config YAML into a t.TempDir() path using os.WriteFile.
2. Call the function under test directly (no Cobra command invocation needed).
3. Assert the returned error or value with require.NoError / require.Error / require.ErrorContains from github.com/stretchr/testify/require.

For createSessionFactory, manipulate the VMCP_SESSION_HMAC_SECRET environment variable with t.Setenv (automatically restored after test). Mock runtime.IsKubernetesRuntime() behavior by setting KUBERNETES_SERVICE_HOST env var (the standard signal the container runtime package uses to detect Kubernetes context).

For discoverBackends, the dynamic Kubernetes discovery path inside discoverBackends requires a live API server and is therefore not unit-testable. Focus unit tests on the static mode path (non-empty cfg.Backends) which is the relevant path for the local vMCP use case. The dynamic path is covered by existing integration tests in test/integration/vmcp/.

Patterns & Frameworks

• SPDX headers: Every new .go file must open with // SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. and // SPDX-License-Identifier: Apache-2.0
• Test assertions: github.com/stretchr/testify/require for fatal assertions (fail immediately), assert for non-fatal checks in table-driven subtests
• Table-driven tests: Use for _, tc := range tests { t.Run(tc.name, func(t *testing.T) {...}) } for functions with multiple input variants (config loading, validation, HMAC secret scenarios)
• Temp directories: Use t.TempDir() for config files — automatically cleaned up after each test
• Env var manipulation: Use t.Setenv(key, value) — automatically restores the original value after the test
• Teardown: Use t.Cleanup(func() {...}) for any resources allocated inside tests, not defer
• Port allocation: Use net.Listen("tcp", ":0") when a free port is needed; close the listener before passing the port
• Gomock: Initialize with ctrl := gomock.NewController(t) (no manual ctrl.Finish() needed — testify handles this automatically via t.Cleanup). Use existing generated mocks — never hand-write mocks.
• No new mocks generated: All required mock types already exist in pkg/vmcp/aggregator/mocks/, pkg/vmcp/session/mocks/, and pkg/vmcp/client/mocks/; reuse them

Code Pointers

• pkg/vmcp/cli/serve.go (created by Extract shared vMCP logic into pkg/vmcp/cli/ (serve + validate) #4879) — Primary file under test; contains Serve, loadAndValidateConfig, discoverBackends, createSessionFactory, loadAuthServerConfig, getStatusReportingInterval
• pkg/vmcp/cli/validate.go (created by Extract shared vMCP logic into pkg/vmcp/cli/ (serve + validate) #4879) — Secondary file under test; contains Validate and ValidateConfig
• pkg/vmcp/config/config.go — Config struct definition; needed to construct valid test configs
• pkg/vmcp/aggregator/mocks/mock_interfaces.go — MockBackendDiscoverer and MockAggregator; import for mocking discoverer interactions
• pkg/vmcp/session/mocks/mock_factory.go — MockMultiSessionFactory if session factory assertions are needed
• pkg/vmcp/client/mocks/mock_outgoing_registry.go — MockOutgoingAuthRegistry for outgoing auth mocking
• test/integration/vmcp/helpers/vmcp_server.go — Reference for how NewVMCPServer constructs and wires components; mirrors the wiring sequence in pkg/vmcp/cli/serve.go
• cmd/vmcp/app/commands.go — Original source of all five functions; useful to understand the exact behavior being tested before Extract shared vMCP logic into pkg/vmcp/cli/ (serve + validate) #4879 extracts them
• .claude/rules/testing.md — Canonical test conventions: Ginkgo vs testify choice (testify for unit tests), gomock usage, parallel test safety, port allocation

Component Interfaces

The test files interact only with the exported API surface defined in #4879. Key signatures to target:

// Validate path — trivially testable with a temp YAML file
func Validate(ctx context.Context, cfg ValidateConfig) error

type ValidateConfig struct {
    ConfigPath string
}

// Serve helpers — test individually since Serve itself blocks
func loadAndValidateConfig(configPath string) (*config.Config, error)
func loadAuthServerConfig(configPath string) (*authserverconfig.RunConfig, error)
func discoverBackends(ctx context.Context, cfg *config.Config) ([]vmcp.Backend, vmcp.BackendClient, vmcpauth.OutgoingAuthRegistry, error)
func createSessionFactory(outgoingRegistry vmcpauth.OutgoingAuthRegistry, agg aggregator.Aggregator) (vmcpsession.MultiSessionFactory, error)

Note: If #4879 keeps these as unexported functions, test them from package cli (same package) rather than package cli_test. The choice should be driven by what #4879 ships.

Sample table-driven test skeleton for loadAndValidateConfig:

func TestLoadAndValidateConfig(t *testing.T) {
    t.Parallel()
    tests := []struct {
        name        string
        setupFile   func(dir string) string // returns config path
        wantErr     bool
        errContains string
    }{
        {
            name: "valid config",
            setupFile: func(dir string) string {
                path := filepath.Join(dir, "vmcp.yaml")
                require.NoError(t, os.WriteFile(path, []byte(validConfigYAML), 0600))
                return path
            },
        },
        {
            name:        "missing file",
            setupFile:   func(dir string) string { return filepath.Join(dir, "nonexistent.yaml") },
            wantErr:     true,
            errContains: "configuration loading failed",
        },
        {
            name: "malformed YAML",
            setupFile: func(dir string) string {
                path := filepath.Join(dir, "bad.yaml")
                require.NoError(t, os.WriteFile(path, []byte(":::invalid yaml:::"), 0600))
                return path
            },
            wantErr:     true,
            errContains: "configuration loading failed",
        },
    }
    for _, tc := range tests {
        t.Run(tc.name, func(t *testing.T) {
            t.Parallel()
            dir := t.TempDir()
            path := tc.setupFile(dir)
            cfg, err := loadAndValidateConfig(path)
            if tc.wantErr {
                require.Error(t, err)
                require.ErrorContains(t, err, tc.errContains)
                require.Nil(t, cfg)
            } else {
                require.NoError(t, err)
                require.NotNil(t, cfg)
            }
        })
    }
}

Testing Strategy

Unit Tests

• TestValidate_MissingConfigPath: call Validate with empty ConfigPath → expect error containing "no configuration file specified"
• TestValidate_ValidConfig: write a minimal valid YAML to temp dir, call Validate → expect nil error
• TestValidate_InvalidConfig: write YAML missing required fields (e.g. missing groupRef), call Validate → expect error containing "validation failed"
• TestLoadAndValidateConfig_Valid: valid YAML round-trip → returns non-nil *config.Config with correct fields populated
• TestLoadAndValidateConfig_MissingFile: non-existent path → error contains "configuration loading failed"
• TestLoadAndValidateConfig_MalformedYAML: ::: syntax error → error contains "configuration loading failed"
• TestLoadAndValidateConfig_FailsValidation: YAML that parses but has missing required fields → error contains "validation failed"
• TestLoadAuthServerConfig_Absent: no sibling file → returns nil, nil
• TestLoadAuthServerConfig_ValidFile: write valid authserver-config.yaml → returns populated *RunConfig
• TestLoadAuthServerConfig_MalformedFile: write ::: into auth server file → returns error
• TestDiscoverBackends_StaticMode: construct config.Config with non-empty Backends slice → expect backends returned equal to input count; no Kubernetes API needed
• TestDiscoverBackends_EmptyGroupWarning: construct config.Config with empty Backends slice in non-Kubernetes environment → expect empty slice with no error (warns but does not fail)
• TestCreateSessionFactory_WithHMACSecret: set VMCP_SESSION_HMAC_SECRET via t.Setenv → expect non-nil factory, nil error
• TestCreateSessionFactory_NoSecret_NonKubernetes: no VMCP_SESSION_HMAC_SECRET, ensure KUBERNETES_SERVICE_HOST is unset → expect non-nil factory, nil error (development mode warning)
• TestCreateSessionFactory_NoSecret_Kubernetes: set KUBERNETES_SERVICE_HOST via t.Setenv, clear VMCP_SESSION_HMAC_SECRET → expect nil factory, error containing "VMCP_SESSION_HMAC_SECRET environment variable is required"
• TestCreateSessionFactory_ShortHMACSecret: set VMCP_SESSION_HMAC_SECRET to a value shorter than 32 bytes → expect non-nil factory, nil error (only a warning is logged; the factory is still created)

Integration Tests

• Existing test/integration/vmcp/vmcp_integration_test.go continues to pass unchanged (no behavior regression from Extract shared vMCP logic into pkg/vmcp/cli/ (serve + validate) #4879 extraction)

Edge Cases

• Auth server config file path is derived from filepath.Dir(configPath) — test with a config file that is in a nested subdirectory to confirm path joining is correct
• VMCP_SESSION_HMAC_SECRET that is exactly 32 bytes long should produce no warning and a valid factory

Out of Scope

• Testing the full Serve function end-to-end (it blocks on a running HTTP server; end-to-end coverage belongs in E2E tests: quick mode and config-file mode #4888 E2E tests)
• Dynamic Kubernetes backend discovery path in discoverBackends (requires a live API server; covered by existing integration tests and future E2E tests in E2E tests: quick mode and config-file mode #4888)
• Modifying cmd/vmcp/app/commands.go (that is Thin-wrap standalone vmcp binary over pkg/vmcp/cli/ #4880)
• Tests for pkg/vmcp/cli/init.go or pkg/vmcp/cli/embedding_manager.go (those are Add init.go to pkg/vmcp/cli/ for config scaffolding #4882 and Implement EmbeddingServiceManager in pkg/vmcp/cli/ #4884 respectively)
• Any changes to the standalone vmcp binary behavior
• Adding new gomock-generated mock files (reuse existing mocks in pkg/vmcp/*/mocks/)

References

• RFC THV-0059 — Phase 1 implementation plan; this item is the unit test deliverable for Phase 1
• GitHub Issue #4808 — Parent tracking issue
• Extract shared vMCP logic into pkg/vmcp/cli/ (serve + validate) #4879 — Creates pkg/vmcp/cli/serve.go and validate.go that this item tests
• .claude/rules/testing.md — ToolHive test conventions (gomock, Ginkgo vs testify, port allocation, t.Cleanup)
• .claude/rules/go-style.md — SPDX headers, error handling conventions
• test/integration/vmcp/helpers/vmcp_server.go — Pattern reference for wiring vMCP components in tests