Add unit tests for Drasi CLI #310

amansinghoriginal · 2025-09-19T03:39:58Z

Description

This pull request refactors the Drasi CLI's command structure to improve testability and introduces a comprehensive suite of unit tests for the core resource management commands (apply, delete, describe, list).

The primary goal is to build a safety net that mocks the Management API, enabling future refactoring of the API client with high confidence.

Key Changes

1. Refactoring for Testability (Dependency Injection)

The core change was to refactor the command constructors (e.g., NewListCommand) to support dependency injection, making them fully unit-testable.

Functional Options Pattern: Instead of using separate Testable*Command functions, the original New*Command functions were modified to accept an optional options struct (e.g., *listCmdOptions).
Dependency Factories: These options structs hold factory functions for the command's dependencies (primarily the PlatformClient).
- In production, the command uses the real factory.
- In tests, a mock factory is injected.
Accurate Code Coverage: This approach ensures that the unit tests execute same logic as production within the command's RunE function, providing accurate code coverage metrics.

2. Comprehensive Unit Test Suite

A new test suite has been added to cover the primary CLI commands:

New Test Files: apply_test.go, delete_test.go, describe_test.go, list_test.go.
Mocking Infrastructure: A new cli/testutil package was created to house mock implementations for:
- DrasiClient: Mocks the Management API interface.
- PlatformClient: Mocks the platform layer.
- TaskOutput: Mocks the UI/console output.
Test Coverage: The tests cover a wide range of scenarios, including:
- Successful command execution for all resource types.
- Handling of API errors (e.g., "not found", "connection failed").
- Graceful failure when platform or API clients cannot be created.
- Correct parsing of user arguments and file-based manifests (-f flag).
- Verification of user-facing output messages.

3. Code Quality Improvements

DrasiClient Interface: An interface was extracted from the concrete ApiClient struct, promoting a better, more modular design.
Shared Test Helper: The executeCommand test helper was consolidated into a single cmd/test_helper_test.go file to avoid code duplication across the test suite.

Type of change

This pull request is a minor refactor, code cleanup, test improvement, or other maintenance task and doesn't change the functionality of Drasi.

cli/cmd/apply_test.go

cli/cmd/describe_test.go

Copilot

Pull Request Overview

Refactors CLI commands to enable dependency injection for improved testability and adds comprehensive unit tests for apply, delete, describe, and list commands while introducing a DrasiClient interface abstraction. Key changes include introducing option structs for command constructors, adding mocks for platform and API layers, and expanding test coverage with realistic scenarios. Public SDK interface was modified (CreateDrasiClient now returns an interface), and supporting mocks plus minor output handling adjustments were added.

Introduced DrasiClient interface and updated PlatformClient to return it (abstraction / testability).
Added dependency-injected constructors for core commands (apply, delete, describe, list) plus extensive unit tests with mocks.
Added mocks and helper utilities (testutil package) and adjusted output handling to use command writers.

Reviewed Changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
cli/testutil/task_output_mock.go	Adds mock for TaskOutput to support unit tests.
cli/testutil/platform_client_mock.go	Adds mock PlatformClient including injectable DrasiClient.
cli/testutil/api_client_mock.go	Adds mock Drasi (management API) client.
cli/sdk/platform_client.go	Changes PlatformClient interface to return DrasiClient (interface abstraction).
cli/sdk/api_client.go	Introduces DrasiClient interface and assertion of implementation.
cli/go.mod	Adds test and Docker-related dependencies as direct requirements.
cli/cmd/test_helper_test.go	Shared test command execution helper.
cli/cmd/list_test.go	New test coverage for list command across scenarios.
cli/cmd/list.go	Adds dependency injection support; returns errors instead of printing; uses cmd output stream.
cli/cmd/describe_test.go	New tests for describe command including error paths.
cli/cmd/describe.go	Adds dependency injection and consistent error handling/output writing.
cli/cmd/delete_test.go	New tests for delete command including file + arg paths and failures.
cli/cmd/delete.go	Adds dependency injection (platform/output) and improves error propagation.
cli/cmd/apply_test.go	New tests for apply command including multi-manifest scenarios and failures.
cli/cmd/apply.go	Adds dependency injection (platform/output) and consistent error handling.
cli/.gitignore	Ignores test and coverage artifacts.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

cli/cmd/apply_test.go

cli/cmd/delete_test.go

cli/cmd/delete.go

cli/cmd/apply.go

cli/sdk/platform_client.go

cli/sdk/api_client.go

ruokun-niu · 2025-09-19T04:41:00Z

.github/workflows/build-test.yml

Will this workflow trigger the unit tests for the CLI?

Good point. Added a separate job test-cli in the workflow.
Thanks!

- Refactor CLI commands to support dependency injection via functional options pattern - Add DrasiClient interface to enable mocking of Management API - Create testutil package with mock implementations for DrasiClient, PlatformClient, and TaskOutput - Add comprehensive unit tests covering success/failure paths for all resource management commands - Fix error handling bugs: commands now properly return errors instead of printing - Fix output routing: use cmd.OutOrStdout() for testability - Individual command coverage: list 78.3%, describe 78.6%, apply 84.4%, delete 84.4% Signed-off-by: Aman Singh <aman.singh.original@gmail.com>

danielgerlag · 2025-09-23T20:26:03Z

cli/cmd/delete.go

+func NewDeleteCommand(opts ...*deleteCmdOptions) *cobra.Command {
+	// Use the real implementation by default
+	opt := &deleteCmdOptions{
+		platformClientFactory: func(namespace string) (sdk.PlatformClient, error) {


This platformClientFactory function seems to be repeated a lot.
Is there a cleaner way to organize it?

ruokun-niu · 2025-09-23T21:15:12Z

cli/cmd/list_test.go

+)
+
+
+func TestListCommand(t *testing.T) {


Just a heads up, the list command for Source and Reaction will now have a column called INGRESS URL

amansinghoriginal requested a review from a team as a code owner September 19, 2025 03:39

amansinghoriginal force-pushed the cli_tests branch from 7025ce5 to b726085 Compare September 19, 2025 03:40

amansinghoriginal requested a review from Copilot September 19, 2025 03:40

amansinghoriginal self-assigned this Sep 19, 2025

github-advanced-security bot found potential problems Sep 19, 2025

View reviewed changes

cli/cmd/apply_test.go Dismissed Show dismissed Hide dismissed

cli/cmd/describe_test.go Dismissed Show dismissed Hide dismissed

cli/cmd/describe_test.go Dismissed Show dismissed Hide dismissed

Copilot AI reviewed Sep 19, 2025

View reviewed changes

amansinghoriginal force-pushed the cli_tests branch 2 times, most recently from 6f139d9 to cd86675 Compare September 19, 2025 04:06

ruokun-niu reviewed Sep 19, 2025

View reviewed changes

amansinghoriginal force-pushed the cli_tests branch from cd86675 to d48b04e Compare September 19, 2025 05:16

amansinghoriginal force-pushed the cli_tests branch from d48b04e to 5660760 Compare September 19, 2025 05:39

amansinghoriginal requested a review from ruokun-niu September 19, 2025 17:41

amansinghoriginal assigned danielgerlag and unassigned danielgerlag Sep 19, 2025

amansinghoriginal requested review from agentofreality and danielgerlag September 19, 2025 17:41

amansinghoriginal linked an issue Sep 22, 2025 that may be closed by this pull request

Kubernetes Operator for Drasi #312

Open

1 task

danielgerlag approved these changes Sep 23, 2025

View reviewed changes

ruokun-niu reviewed Sep 23, 2025

View reviewed changes

ruokun-niu approved these changes Sep 23, 2025

View reviewed changes

Merge branch 'main' into cli_tests

35d4fef

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add unit tests for Drasi CLI #310

Add unit tests for Drasi CLI #310

amansinghoriginal commented Sep 19, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ruokun-niu Sep 19, 2025

Uh oh!

amansinghoriginal Sep 19, 2025

Uh oh!

danielgerlag Sep 23, 2025

Uh oh!

ruokun-niu Sep 23, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Add unit tests for Drasi CLI #310

Are you sure you want to change the base?

Add unit tests for Drasi CLI #310

Conversation

amansinghoriginal commented Sep 19, 2025

Description

Key Changes

1. Refactoring for Testability (Dependency Injection)

2. Comprehensive Unit Test Suite

3. Code Quality Improvements

Type of change

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ruokun-niu Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

amansinghoriginal Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

danielgerlag Sep 23, 2025

Choose a reason for hiding this comment

Uh oh!

ruokun-niu Sep 23, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants