Increasing coverage and simplifying coverage report creation.

.gitignore

@@ -32,8 +32,14 @@
 # Test binary, built with `go test -c`
 *.test
 
-# Output of the go coverage tool, specifically when used with LiteIDE
+# Output of the go coverage tool
 *.out
+*.summary
+/*report.xml
+/test.summary
+/*.html
+/test/coverage/**/*
+
 
 # Dependency directories (remove the comment below to include it)
 # vendor/
@@ -55,7 +61,8 @@ go.work.sum
 .LSOverride
 
 # Icon must end with two \r
-Icon
+Icon
+
 
 # Thumbnails
 ._*

Makefile

@@ -96,12 +96,12 @@ cmd/lint/en_US.dic:
 
 .PHONY: unit-test
 unit-test:
-ifdef CI
-	go install gotest.tools/[email protected] && \
-	gotestsum --junitfile unit-test-report.xml -- -timeout 0 -v -race -coverprofile=coverage.unit.out -covermode=atomic $$(go list ./... | grep -v github.com/confluentinc/cli/v4/test)
-else
-	go test -timeout 0 -v -coverprofile=coverage.unit.out -covermode=atomic $$(go list ./... | grep -v github.com/confluentinc/cli/v4/test) $(UNIT_TEST_ARGS)
-endif
+	@if [ ! -f "$(shell go env GOPATH)/bin/gotestsum$(shell go env GOEXE)" ]; then \
+		go install gotest.tools/[email protected] ; \
+	fi
+	@"$(shell go env GOPATH)/bin/gotestsum$(shell go env GOEXE)" --junitfile unit-test-report.xml -- \
+		-timeout 0 -v -race -coverprofile=coverage.unit.out -covermode=atomic \
+		$$(go list ./... | grep -v github.com/confluentinc/cli/v4/test)
 
 .PHONY: build-for-integration-test
 build-for-integration-test:
@@ -121,18 +121,14 @@ endif
 
 .PHONY: integration-test
 integration-test:
-ifdef CI
-	go install gotest.tools/[email protected] && \
-	export GOCOVERDIR=test/coverage && \
-	rm -rf $${GOCOVERDIR} && mkdir $${GOCOVERDIR} && \
-	gotestsum --junitfile integration-test-report.xml -- -timeout 0 -v -race $$(go list ./... | grep github.com/confluentinc/cli/v4/test) && \
-	go tool covdata textfmt -i $${GOCOVERDIR} -o coverage.integration.out
-else
-	export GOCOVERDIR=test/coverage && \
-	rm -rf $${GOCOVERDIR} && mkdir $${GOCOVERDIR} && \
-	go test -timeout 0 -v $$(go list ./... | grep github.com/confluentinc/cli/v4/test) $(INTEGRATION_TEST_ARGS) && \
-	go tool covdata textfmt -i $${GOCOVERDIR} -o coverage.integration.out
-endif
+	@if [ ! -f "$(shell go env GOPATH)/bin/gotestsum$(shell go env GOEXE)" ]; then \
+		go install gotest.tools/[email protected] ; \
+	fi
+	@rm -rf test/coverage && mkdir -p test/coverage && \
+	GOCOVERDIR=test/coverage "$(shell go env GOPATH)/bin/gotestsum$(shell go env GOEXE)" --junitfile integration-test-report.xml -- \
+		-timeout 0 -v -race $$(go list ./... | grep github.com/confluentinc/cli/v4/test) && \
+	go tool covdata textfmt -i test/coverage -o coverage.integration.out
+
 
 .PHONY: test
 test: unit-test integration-test
@@ -142,10 +138,17 @@ generate-packaging-patch:
 	diff -u Makefile debian/Makefile | sed "1 s_Makefile_cli/Makefile_" > debian/patches/standard_build_layout.patch
 
 .PHONY: coverage
-coverage: ## Merge coverage data from unit and integration tests into coverage.txt
+coverage:
 	@echo "Merging coverage data..."
-	@echo "mode: atomic" > coverage.txt
-	@tail -n +2 coverage.unit.out >> coverage.txt
-	@tail -n +2 coverage.integration.out >> coverage.txt
+	@echo "mode: atomic" > coverage.out
+	@tail -n +2 coverage.unit.out >> coverage.out || echo "No unit coverage found"
+	@tail -n +2 coverage.integration.out >> coverage.out || echo "No integration coverage found"
+
+ifeq ($(CI),true)
+	@cp coverage.out coverage.txt
 	@echo "Coverage data saved to: coverage.txt"
-	@artifact push workflow coverage.txt
+	@artifact push workflow coverage.txt
+else
+	@go tool cover -func=coverage.out
+	@go tool cover -html=coverage.out -o coverage.html
+endif

internal/byok/command_create_test.go

@@ -3,8 +3,9 @@ package byok
 import (
 	"testing"
 
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
+	byokv1 "github.com/confluentinc/ccloud-sdk-go-v2/byok/v1"
 )
 
 func TestRemoveKeyVersionFromAzureKeyId(t *testing.T) {
@@ -44,30 +45,113 @@ func TestRemoveKeyVersionFromAzureKeyId(t *testing.T) {
 }
 
 func TestGcpMetadataCustomRoleName(t *testing.T) {
-	t.Run("success, custom role name generated", func(t *testing.T) {
-		metadata := gcpPolicyMetadata{
-			keyRing: "testKeyRing",
-			key:     "testKey",
-		}
-		customRoleName := metadata.getCustomRoleName()
-		assert.Equal(t, "testKeyRing_testKey_custom_kms_role", customRoleName)
-	})
+	tests := []struct {
+		name     string
+		keyRing  string
+		key      string
+		expected string
+	}{
+		{
+			name:     "success, custom role name generated",
+			keyRing:  "testKeyRing",
+			key:      "testKey",
+			expected: "testKeyRing_testKey_custom_kms_role",
+		},
+		{
+			name:     "success, hyphens replaced",
+			keyRing:  "test-key-ring",
+			key:      "test-key",
+			expected: "test_key_ring_test_key_custom_kms_role",
+		},
+		{
+			name:     "failure, unsupported characters return default",
+			keyRing:  "test&key&ring",
+			key:      "test&key",
+			expected: "custom_kms_role",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			metadata := gcpPolicyMetadata{
+				keyRing: test.keyRing,
+				key:     test.key,
+			}
+			actual := metadata.getCustomRoleName()
+			require.Equal(t, test.expected, actual)
+		})
+	}
+}
 
-	t.Run("success, hyphens replaced", func(t *testing.T) {
-		metadata := gcpPolicyMetadata{
-			keyRing: "test-key-ring",
-			key:     "test-key",
-		}
-		customRoleName := metadata.getCustomRoleName()
-		assert.Equal(t, "test_key_ring_test_key_custom_kms_role", customRoleName)
-	})
+func TestGetPolicyCommand(t *testing.T) {
+	awsRoles := []string{"arn:aws:iam::123456789012:role/test-role"}
+	azureKeyID := "https://vault-name.vault.azure.net/keys/key-name"
+	azureAppID := "00000000-0000-0000-0000-000000000000"
+	gcpKeyID := "projects/proj/locations/loc/keyRings/ring/cryptoKeys/key/cryptoKeyVersions/1"
+	gcpGroup := "[email protected]"
 
-	t.Run("failure, default role name returned", func(t *testing.T) {
-		metadata := gcpPolicyMetadata{
-			keyRing: "test&key&ring",
-			key:     "test&key",
-		}
-		customRoleName := metadata.getCustomRoleName()
-		assert.Equal(t, "custom_kms_role", customRoleName)
-	})
+	tests := []struct {
+		name     string
+		key      byokv1.ByokV1Key
+		wantErr  bool
+		contains string
+	}{
+		{
+			name: "aws with valid role",
+			key: byokv1.ByokV1Key{
+				Key: &byokv1.ByokV1KeyKeyOneOf{
+					ByokV1AwsKey: &byokv1.ByokV1AwsKey{
+						Roles: &awsRoles,
+					},
+				},
+			},
+			wantErr:  false,
+			contains: "arn:aws:iam",
+		},
+		{
+			name: "azure with valid input",
+			key: byokv1.ByokV1Key{
+				Key: &byokv1.ByokV1KeyKeyOneOf{
+					ByokV1AzureKey: &byokv1.ByokV1AzureKey{
+						KeyId:         azureKeyID,
+						ApplicationId: &azureAppID,
+					},
+				},
+			},
+			wantErr:  false,
+			contains: "az role assignment create",
+		},
+		{
+			name: "gcp with valid key",
+			key: byokv1.ByokV1Key{
+				Key: &byokv1.ByokV1KeyKeyOneOf{
+					ByokV1GcpKey: &byokv1.ByokV1GcpKey{
+						KeyId:         gcpKeyID,
+						SecurityGroup: &gcpGroup,
+					},
+				},
+			},
+			wantErr:  false,
+			contains: "[email protected]",
+		},
+
+		{
+			name: "unknown key type returns empty string",
+			key:  byokv1.ByokV1Key{Key: &byokv1.ByokV1KeyKeyOneOf{}},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			actual, err := getPolicyCommand(test.key)
+			if test.wantErr {
+				require.Error(t, err)
+			} else {
+				require.NoError(t, err)
+				if test.contains != "" {
+					require.Contains(t, actual, test.contains)
+				}
+			}
+		})
+	}
 }

internal/login/command_test.go

@@ -733,6 +733,53 @@ func TestLoginWithExistingContext(t *testing.T) {
 	}
 }
 
+func TestSuspendedOrganizationError(t *testing.T) {
+	req := require.New(t)
+
+	mockLoginCredentialsManager := &climock.LoginCredentialsManager{
+		GetCloudCredentialsFromEnvVarFunc: func(_ string) func() (*pauth.Credentials, error) {
+			return func() (*pauth.Credentials, error) {
+				return &pauth.Credentials{
+					Username: promptUser,
+					Password: promptPassword,
+				}, nil
+			}
+		},
+		GetCloudCredentialsFromPromptFunc: func(_ string) func() (*pauth.Credentials, error) {
+			return func() (*pauth.Credentials, error) {
+				return nil, nil
+			}
+		},
+		SetCloudClientFunc: func(_ *ccloudv1.Client) {},
+	}
+
+	mockAuthTokenHandler := &climock.AuthTokenHandler{
+		GetCCloudTokensFunc: func(_ pauth.CCloudClientFactory, _ string, _ *pauth.Credentials, _ bool, _ string) (string, string, error) {
+			return "", "", &ccloudv1.SuspendedOrganizationError{}
+		},
+	}
+
+	auth := &ccloudv1mock.Auth{
+		UserFunc: func() (*ccloudv1.GetMeReply, error) {
+			return &ccloudv1.GetMeReply{
+				User: &ccloudv1.User{
+					Id:    23,
+					Email: promptUser,
+				},
+				Organization: &ccloudv1.Organization{ResourceId: organizationId1},
+				Accounts:     []*ccloudv1.Account{{Id: "env-596", Name: "Default"}},
+			}, nil
+		},
+	}
+	userInterface := &ccloudv1mock.UserInterface{}
+
+	loginCmd, _ := newLoginCmd(auth, userInterface, true, req, mockAuthTokenHandler, mockLoginCredentialsManager, LoginOrganizationManager)
+	_, err := pcmd.ExecuteCommand(loginCmd)
+	req.Error(err)
+	req.IsType(&errors.ErrorWithSuggestionsImpl{}, err)
+	req.Contains(err.Error(), "suspended")
+}
+
 func TestValidateUrl(t *testing.T) {
 	req := require.New(t)
 	suite := []struct {

test/fixtures/output/flink/endpoint/use-empty.golden

@@ -0,0 +1,4 @@
+Error: accepts 1 arg(s), received 0
+
+Suggestions:
+    Please run "confluent flink endpoint list" to see all available Flink endpoints, or "confluent flink region use" to switch to a different cloud or region. 

test/fixtures/output/flink/endpoint/use-no-cloud.golden

@@ -0,0 +1,4 @@
+Error: Current Flink cloud provider is empty
+
+Suggestions:
+    Please run `confluent flink region use --cloud <cloud> --region <region>` to set the Flink cloud provider first.

test/fixtures/output/flink/endpoint/use-no-region.golden

@@ -0,0 +1,6 @@
+error: coverage meta-data emit failed: output directory "/test/coverage" inaccessible (err: stat /test/coverage: no such file or directory); no coverage data written
+Error: Current Flink cloud provider is empty
+
+Suggestions:
+    Please run confluent flink region use --cloud <cloud> --region <region> to set the Flink cloud provider first.
+error: coverage counter data emit failed: output directory "/test/coverage" inaccessible (err: stat /test/coverage: no such file or directory); no coverage data written 

test/fixtures/output/flink/region/unset-region.golden

@@ -0,0 +1 @@
+Unset the current Flink region 

test/flink_test.go

@@ -587,3 +587,32 @@ func (s *CLITestSuite) TestFlinkStatmentExceptionList() {
 		s.runIntegrationTest(test)
 	}
 }
+
+func (s *CLITestSuite) TestFlinkEmptyCloudProvider() {
+	tests := []CLITest{
+		{args: "flink endpoint use http://127.0.0.1:1026", fixture: "flink/endpoint/use-no-cloud.golden", exitCode: 1},
+	}
+
+	for _, test := range tests {
+		test.login = "cloud"
+		test.workflow = true
+		s.runIntegrationTest(test)
+	}
+}
+
+func (s *CLITestSuite) TestFlinkEmptyRegion() {
+	tests := []CLITest{
+		// The CLI architecture makes it impossible to have a cloud provider set without a region,
+		// since region unset command always unsets both cloud and region, and region use requires both.
+		// If neither cloud provider nor region is set, the cloud provider check fails first
+		// before it can check for the region being empty.
+		{args: "flink region unset", fixture: "flink/region/unset.golden"},
+		{args: "flink endpoint use http://127.0.0.1:1026", fixture: "flink/endpoint/use-no-cloud.golden", exitCode: 1},
+	}
+
+	for _, test := range tests {
+		test.login = "cloud"
+		test.workflow = true
+		s.runIntegrationTest(test)
+	}
+}