Implement Isolated, Deterministic Test Suite for ruletype CLI Subcommands

[DharunMR]

Summary

This PR introduces a robust, table driven testing architecture for the ruletype CLI command lifecycle (create, apply, get, list, delete). Historically, CLI commands can be brittle to test due to tightly coupled network calls and unpredictable standard output. This implementation establishes a new pilot pattern for CLI testing in Minder by enforcing strict network boundaries (via gRPC mocking) and deterministic output verification (via Golden Files and I/O interception).

Dependency Injection & Mocking Strategy

To isolate the CLI logic from the actual Minder control plane, this test suite leverages gomock to stub the RuleTypeServiceClient.

• Global Variable Overriding: The CLI commands rely on a package-level variable (getRuleTypeClient) to instantiate the gRPC client. During testing, we intercept this by overriding the variable with our generated mockv1.MockRuleTypeServiceClient
• State Restoration: To prevent test pollution, t.Cleanup() is utilized to restore the original getRuleTypeClient function pointer after every subtest execution.
• Concurrency Trade-off (//nolint:paralleltest): Because we are mutating a shared package level variable to inject the mock, running these tests with t.Parallel() would introduce race conditions and non-deterministic panics. Test functions are intentionally run serially and explicitly documented with the linter suppression comment.

Deterministic Output Verification (Golden Files)

Testing CLI tools requires validating not just the exit codes, but the exact string formatting (Tables, JSON, YAML) presented to the user.

• I/O Interception: We use a combination of bytes.Buffer (attached to cmd.SetOut and cmd.SetErr) and os.Pipe() to cleanly capture os.Stdout. This ensures that underlying libraries (like table writers) that bypass cobra's standard buffers are still perfectly captured.
• Golden File Assertions: The captured output is asserted against pre-verified .golden files stored in testdata/. This makes future UI/UX regressions immediately obvious during CI. (To update these files locally, run go test ./... -update).

Testing

• Unit Tests: Executed go test -v ./cmd/cli/app/ruletype (100% pass rate on new logic).
• Linting: Executed make lint confirming zero regressions or stylistic violations.

Proposed Future Roadmap & Refactoring

If this isolated mock driven testing approach aligns with the maintainer's vision for the CLI, I propose the following follow-up work:

• Horizontal Expansion: Propagate this exact table driven structure to untested commands (e.g., profiles, repos, entities, etc).
• Centralized Testkit: Extract the generic boilerplate specifically checkGoldenFile, loadFixture and the os.Pipe() stdout capturing logic out of the ruletype package and into a shared internal/cli/testutils or pkg/testkit package to heavily reduce code duplication across the CLI test suites.
• CI/CD Integration (GitHub Actions): Implement a dedicated GitHub Actions workflow triggered specifically on path changes within cmd/cli/**. This will automatically run the isolated CLI test suite and verify golden files on all future pull requests, preventing output regressions and enforcing this new testing standard automatically.

[evankanderson]

This is very cool! A few comments -- I'd like to iterate on this a little bit, but I think you picked about the right "size" of example to catch interesting cases.

This seems a bit like WireMock, but I'm not sure pulling in that dependency is worthwhile.

[evankanderson]

What do you think of making the function to get a typed proto interface a generic method which takes a context.Context? I think that would look like:

ctx := withRpcContext(context.Background, mockClient)
cmd.SetContext(ctx)

And then the getter might look like:

func rpcContext[IF interface](ctx context.Context) IF {
	if iface := ctx.Value(IF); iface != nil {
		return iface
	}
	return getFullRpcStub(ctx)
}

The main benefit would be that you wouldn't need to worry about the global getRuleTypeClient, but we could also simplify our current RunE -> cli.GRPCClientWrapRunE pattern to regular RunE functions.

(I hadn't thought about this or this pattern until tonight, and I find this testing idea interesting!)

[DharunMR]

I completely agree that moving away from global state makes the tests much more robust. In the latest commit I’ve moved to this context approach.

[evankanderson]

This comment no longer appears to be correct.

[DharunMR]

You're right, my previous comment was a bit unclear. Since make lint is still flagging this as unparam in this specific file, I've updated the comment to clarify that I'm keeping the signature generic for architectural consistency across the ruletype package's test suite

[evankanderson]

It would be interesting if loadFixture could return a full RPC service, where the JSON included the function name for the responses, and then the mockSetup code could be replaced with rpcResponses: "mock_ruletypes_response.json".

[DharunMR]

That is a really interesting idea moving to a json approach would definitely clean up the test structs and reduce the gomock boilerplate across the tests. I spent some time thinking about how we would actually implement that and I have two main thoughts based on my understanding

To make this work, we would need to build a dynamic dispatcher inside loadFixture to route those JSON method strings into actual gomock expectations. That is a pretty massive and feels a bit too complex
My other hesitation is when testing multiple edge cases the json fixtures would often be 95% identical, differing by only a few lines (e.g., just changing an error code). Burying those 2-3 meaningful lines inside massive duplicated json payloads actually makes the tests harder to read and PRs much harder to review.

This is based on my understanding so far. Could you let me know if I'm mistaken?

[DharunMR]

@evankanderson
Just to confirm, I didn't introduce any new test frameworks or dependencies for this setup. I built the fixture logic entirely using Go's standard library (os, path/filepath, encoding/json) layered on top of our existing testify and gomock tooling.

[coveralls]

Coverage is 58.849% — DharunMR:testing-ruletype-cmd into mindersec:main. No base build found for mindersec:main.