CDI-601: Improve Ping CLI's Error System

.github/workflows/code-analysis-lint-test.yaml

@@ -143,8 +143,6 @@ jobs:
           TEST_PINGONE_WORKER_CLIENT_ID: ${{ secrets.TEST_PINGONE_WORKER_CLIENT_ID }}
           TEST_PINGONE_WORKER_CLIENT_SECRET: ${{ secrets.TEST_PINGONE_WORKER_CLIENT_SECRET }}
           TEST_PINGONE_REGION_CODE: ${{ secrets.TEST_PINGONE_REGION_CODE }}
-          TEST_PINGCLI_DEVOPS_USER: ${{ secrets.TEST_PINGCLI_DEVOPS_USER }}
-          TEST_PINGCLI_DEVOPS_KEY: ${{ secrets.TEST_PINGCLI_DEVOPS_KEY }}
 
   onfailure:
     if: ${{ always() && github.event_name == 'schedule' && contains(needs.*.result, 'failure') }}

.golangci.yml

@@ -9,6 +9,7 @@ linters:
     - decorder
     - dupword
     - durationcheck
+    - err113
     - errcheck
     - errchkjson
     - errname
@@ -53,7 +54,6 @@ linters:
     - staticcheck
     - tagalign
     - testableexamples
-    - testpackage
     - thelper
     - tparallel
     - unconvert

Makefile

@@ -2,6 +2,7 @@
 # This simplifies complex commands and makes the file more readable.
 .ONESHELL:
 SHELL := $(shell which bash || which sh)
+.SHELLFLAGS := -ec
 
 # ====================================================================================
 # VARIABLES
@@ -119,7 +120,6 @@ protogen: ## Generate Go code from .proto files
 
 test: _check_ping_env ## Run all tests
 	@echo "  > Test: Running all Go tests..."
-	set -e
 	for dir in $(TEST_DIRS); do
 		echo "    -> $$dir"
 		$(GOTEST) $$dir
@@ -171,7 +171,7 @@ _check_ping_env:
 
 _check_docker:
 	@echo "  > Docker: Checking if the Docker daemon is running..."
-	$(DOCKER) info > /dev/null 2>&1
+	$(DOCKER) info > /dev/null
 	echo "✅ Docker daemon is running."
 
 _run_pf_container:
@@ -188,7 +188,6 @@ _run_pf_container:
 
 _wait_for_pf:
 	@echo "  > Docker: Waiting for container to become healthy (up to 4 minutes)..."
-	set -e
 	timeout=240
 	while test $$timeout -gt 0; do
 		status=$$(docker inspect --format='{{json .State.Health.Status}}' $(CONTAINER_NAME) 2>/dev/null || echo "")

cmd/common/cobra_utils.go

@@ -5,13 +5,18 @@ package common
 import (
 	"fmt"
 
+	"github.com/pingidentity/pingcli/internal/errs"
 	"github.com/spf13/cobra"
 )
 
+var (
+	argsErrorPrefix = "failed to execute command"
+)
+
 func ExactArgs(numArgs int) cobra.PositionalArgs {
 	return func(cmd *cobra.Command, args []string) error {
 		if len(args) != numArgs {
-			return fmt.Errorf("failed to execute '%s': command accepts %d arg(s), received %d", cmd.CommandPath(), numArgs, len(args))
+			return &errs.PingCLIError{Prefix: argsErrorPrefix, Err: fmt.Errorf("%w: command accepts %d arg(s), received %d", ErrExactArgs, numArgs, len(args))}
 		}
 
 		return nil
@@ -21,7 +26,7 @@ func ExactArgs(numArgs int) cobra.PositionalArgs {
 func RangeArgs(minArgs, maxArgs int) cobra.PositionalArgs {
 	return func(cmd *cobra.Command, args []string) error {
 		if len(args) < minArgs || len(args) > maxArgs {
-			return fmt.Errorf("failed to execute '%s': command accepts %d to %d arg(s), received %d", cmd.CommandPath(), minArgs, maxArgs, len(args))
+			return &errs.PingCLIError{Prefix: argsErrorPrefix, Err: fmt.Errorf("%w: command accepts %d to %d arg(s), received %d", ErrRangeArgs, minArgs, maxArgs, len(args))}
 		}
 
 		return nil

cmd/common/cobra_utils_test.go

@@ -6,44 +6,132 @@ import (
 	"testing"
 
 	"github.com/pingidentity/pingcli/cmd/common"
-	"github.com/pingidentity/pingcli/internal/testing/testutils"
-	"github.com/spf13/cobra"
+	"github.com/stretchr/testify/require"
 )
 
-// Test ExactArgs returns no error when the number of arguments matches the expected number
-func TestExactArgs_Matches(t *testing.T) {
-	posArgsFunc := common.ExactArgs(2)
-	err := posArgsFunc(nil, []string{"arg1", "arg2"})
-	testutils.CheckExpectedError(t, err, nil)
-}
+func Test_ExactArgs(t *testing.T) {
+	testCases := []struct {
+		name        string
+		numArgs     int
+		args        []string
+		expectedErr error
+	}{
+		{
+			name:        "No arguments, expecting 0",
+			numArgs:     0,
+			args:        []string{},
+			expectedErr: nil,
+		},
+		{
+			name:        "One argument, expecting 1",
+			numArgs:     1,
+			args:        []string{"arg1"},
+			expectedErr: nil,
+		},
+		{
+			name:        "Two arguments, expecting 2",
+			numArgs:     2,
+			args:        []string{"arg1", "arg2"},
+			expectedErr: nil,
+		},
+		{
+			name:        "Three arguments, expecting 2",
+			numArgs:     2,
+			args:        []string{"arg1", "arg2", "arg3"},
+			expectedErr: common.ErrExactArgs,
+		},
+		{
+			name:        "No arguments, expecting 1",
+			numArgs:     1,
+			args:        []string{},
+			expectedErr: common.ErrExactArgs,
+		},
+		{
+			name:        "One argument, expecting 0",
+			numArgs:     0,
+			args:        []string{"arg1"},
+			expectedErr: common.ErrExactArgs,
+		},
+	}
 
-// Test ExactArgs returns an error when the number of arguments does not match the expected number
-func TestExactArgs_DoesNotMatch(t *testing.T) {
-	expectedErrorPattern := `^failed to execute 'test': command accepts 2 arg\(s\), received 3$`
-	posArgsFunc := common.ExactArgs(2)
-	err := posArgsFunc(&cobra.Command{Use: "test"}, []string{"arg1", "arg2", "arg3"})
-	testutils.CheckExpectedError(t, err, &expectedErrorPattern)
-}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			posArgsFunc := common.ExactArgs(tc.numArgs)
+			err := posArgsFunc(nil, tc.args)
 
-// Test RangeArgs returns no error when the number of arguments is within the expected range
-func TestRangeArgs_Matches(t *testing.T) {
-	posArgsFunc := common.RangeArgs(2, 4)
-	err := posArgsFunc(nil, []string{"arg1", "arg2", "arg3"})
-	testutils.CheckExpectedError(t, err, nil)
+			if tc.expectedErr != nil {
+				require.Error(t, err)
+				require.ErrorIs(t, err, tc.expectedErr)
+			} else {
+				require.NoError(t, err)
+			}
+		})
+	}
 }
 
-// Test RangeArgs returns an error when the number of arguments is below the expected range
-func TestRangeArgs_BelowRange(t *testing.T) {
-	expectedErrorPattern := `^failed to execute 'test': command accepts 2 to 4 arg\(s\), received 1$`
-	posArgsFunc := common.RangeArgs(2, 4)
-	err := posArgsFunc(&cobra.Command{Use: "test"}, []string{"arg1"})
-	testutils.CheckExpectedError(t, err, &expectedErrorPattern)
-}
+func Test_RangeArgs(t *testing.T) {
+	testCases := []struct {
+		name        string
+		minArgs     int
+		maxArgs     int
+		args        []string
+		expectedErr error
+	}{
+		{
+			name:        "No arguments, expecting 0 to 2",
+			minArgs:     0,
+			maxArgs:     2,
+			args:        []string{},
+			expectedErr: nil,
+		},
+		{
+			name:        "One argument, expecting 1 to 2",
+			minArgs:     1,
+			maxArgs:     2,
+			args:        []string{"arg1"},
+			expectedErr: nil,
+		},
+		{
+			name:        "Two arguments, expecting 1 to 2",
+			minArgs:     1,
+			maxArgs:     2,
+			args:        []string{"arg1", "arg2"},
+			expectedErr: nil,
+		},
+		{
+			name:        "Three arguments, expecting 1 to 2",
+			minArgs:     1,
+			maxArgs:     2,
+			args:        []string{"arg1", "arg2", "arg3"},
+			expectedErr: common.ErrRangeArgs,
+		},
+		{
+			name:        "No arguments, expecting 1 to 2",
+			minArgs:     1,
+			maxArgs:     2,
+			args:        []string{},
+			expectedErr: common.ErrRangeArgs,
+		},
+		{
+			name:        "One argument, expecting 0 to 0",
+			minArgs:     0,
+			maxArgs:     0,
+			args:        []string{"arg1"},
+			expectedErr: common.ErrRangeArgs,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			posArgsFunc := common.RangeArgs(tc.minArgs, tc.maxArgs)
+			err := posArgsFunc(nil, tc.args)
 
-// Test RangeArgs returns an error when the number of arguments is above the expected range
-func TestRangeArgs_AboveRange(t *testing.T) {
-	expectedErrorPattern := `^failed to execute 'test': command accepts 2 to 4 arg\(s\), received 5$`
-	posArgsFunc := common.RangeArgs(2, 4)
-	err := posArgsFunc(&cobra.Command{Use: "test"}, []string{"arg1", "arg2", "arg3", "arg4", "arg5"})
-	testutils.CheckExpectedError(t, err, &expectedErrorPattern)
+			if tc.expectedErr != nil {
+				require.Error(t, err)
+				require.ErrorIs(t, err, tc.expectedErr)
+			} else {
+				require.NoError(t, err)
+			}
+		})
+	}
 }

cmd/common/errors.go

@@ -0,0 +1,10 @@
+// Copyright © 2025 Ping Identity Corporation
+
+package common
+
+import "errors"
+
+var (
+	ErrExactArgs = errors.New("exact number of arguments not provided")
+	ErrRangeArgs = errors.New("argument count not in valid range")
+)

cmd/completion/cmd.go

@@ -5,6 +5,7 @@ package completion
 import (
 	"fmt"
 
+	"github.com/pingidentity/pingcli/internal/errs"
 	"github.com/spf13/cobra"
 )
 
@@ -53,13 +54,25 @@ PowerShell:
 func completionCmdRunE(cmd *cobra.Command, args []string) error {
 	switch args[0] {
 	case "bash":
-		_ = cmd.Root().GenBashCompletionV2(cmd.OutOrStdout(), true)
+		err := cmd.Root().GenBashCompletionV2(cmd.OutOrStdout(), true)
+		if err != nil {
+			return &errs.PingCLIError{Prefix: "", Err: err}
+		}
 	case "zsh":
-		_ = cmd.Root().GenZshCompletion(cmd.OutOrStdout())
+		err := cmd.Root().GenZshCompletion(cmd.OutOrStdout())
+		if err != nil {
+			return &errs.PingCLIError{Prefix: "", Err: err}
+		}
 	case "fish":
-		_ = cmd.Root().GenFishCompletion(cmd.OutOrStdout(), true)
+		err := cmd.Root().GenFishCompletion(cmd.OutOrStdout(), true)
+		if err != nil {
+			return &errs.PingCLIError{Prefix: "", Err: err}
+		}
 	case "powershell":
-		_ = cmd.Root().GenPowerShellCompletion(cmd.OutOrStdout())
+		err := cmd.Root().GenPowerShellCompletion(cmd.OutOrStdout())
+		if err != nil {
+			return &errs.PingCLIError{Prefix: "", Err: err}
+		}
 	}
 
 	return nil

cmd/completion/cmd_test.go

@@ -5,12 +5,71 @@ package completion_test
 import (
 	"testing"
 
-	"github.com/pingidentity/pingcli/internal/testing/testutils"
 	"github.com/pingidentity/pingcli/internal/testing/testutils_cobra"
+	"github.com/pingidentity/pingcli/internal/testing/testutils_koanf"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
-// Test Completion Command Executes without issue
-func TestCompletionCmd_Execute(t *testing.T) {
-	err := testutils_cobra.ExecutePingcli(t)
-	testutils.CheckExpectedError(t, err, nil)
+func Test_CompletionCommand(t *testing.T) {
+	testutils_koanf.InitKoanfs(t)
+
+	testCases := []struct {
+		name                string
+		args                []string
+		expectErr           bool
+		expectedErrIs       error
+		expectedErrContains string
+	}{
+		{
+			name:                "Too few arguments",
+			args:                []string{},
+			expectErr:           true,
+			expectedErrContains: "accepts 1 arg(s), received 0",
+		},
+		{
+			name:      "Happy Path - bash",
+			args:      []string{"bash"},
+			expectErr: false,
+		},
+		{
+			name:      "Happy Path - help",
+			args:      []string{"--help"},
+			expectErr: false,
+		},
+		{
+			name:                "Too many arguments",
+			args:                []string{"bash", "extra-arg"},
+			expectErr:           true,
+			expectedErrContains: "accepts 1 arg(s), received 2",
+		},
+		{
+			name:                "Invalid flag",
+			args:                []string{"--invalid-flag"},
+			expectErr:           true,
+			expectedErrContains: "unknown flag",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			testutils_koanf.InitKoanfs(t)
+
+			err := testutils_cobra.ExecutePingcli(t, append([]string{"completion"}, tc.args...)...)
+
+			if !tc.expectErr {
+				require.NoError(t, err)
+
+				return
+			}
+
+			assert.Error(t, err)
+			if tc.expectedErrIs != nil {
+				assert.ErrorIs(t, err, tc.expectedErrIs)
+			}
+			if tc.expectedErrContains != "" {
+				assert.ErrorContains(t, err, tc.expectedErrContains)
+			}
+		})
+	}
 }