refactor: adopt idiomatic cobra patterns across all commands

Makefile

@@ -8,6 +8,10 @@ WATCHER_NAME=watcher
 HOST_OS=$(shell go env GOOS)
 HOST_ARCH=$(shell go env GOARCH)
 
+.PHONY: vet
+vet:
+	go vet ./...
+
 .PHONY: build-local
 build-local:
 	go build -o ${DIST_DIR}/${BIN_NAME}

cmd/cmd.go

@@ -27,9 +27,10 @@ func NewCommad() *cobra.Command {
 	var clientOpts connectors.ClientOptions
 
 	command := &cobra.Command{
-		Use:          "microcks",
-		Short:        "A CLI tool for Microcks",
-		SilenceUsage: true,
+		Use:           "microcks",
+		Short:         "A CLI tool for Microcks",
+		SilenceUsage:  true,
+		SilenceErrors: true,
 		Run: func(cmd *cobra.Command, args []string) {
 			cmd.HelpFunc()(cmd, args)
 		},

cmd/cmd_test.go

@@ -0,0 +1,88 @@
+package cmd
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/spf13/cobra"
+	"github.com/stretchr/testify/require"
+)
+
+func TestStartCommandPortValidation(t *testing.T) {
+	tests := []struct {
+		name    string
+		port    string
+		wantErr bool
+	}{
+		{name: "lowest valid port", port: "1", wantErr: false},
+		{name: "default port", port: "8585", wantErr: false},
+		{name: "highest valid port", port: "65535", wantErr: false},
+		{name: "non numeric port", port: "abc", wantErr: true},
+		{name: "zero port", port: "0", wantErr: true},
+		{name: "negative port", port: "-1", wantErr: true},
+		{name: "port above range", port: "65536", wantErr: true},
+		{name: "empty port", port: "", wantErr: true},
+		{name: "port with spaces", port: " 8080", wantErr: true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			cmd := NewStartCommand(nil)
+			cmd.SetArgs([]string{"--port", tt.port})
+
+			executed := false
+			cmd.RunE = func(cmd *cobra.Command, args []string) error {
+				executed = true
+				return nil
+			}
+
+			err := cmd.Execute()
+			if tt.wantErr {
+				require.Error(t, err)
+				require.False(t, executed, "RunE should not execute when validation fails")
+			} else {
+				require.NoError(t, err)
+				require.True(t, executed, "RunE should execute when validation passes")
+			}
+		})
+	}
+}
+
+func TestLoginCommandSsoPortValidation(t *testing.T) {
+	tests := []struct {
+		name    string
+		ssoPort int
+		wantErr bool
+	}{
+		{name: "lowest valid port", ssoPort: 1, wantErr: false},
+		{name: "default port", ssoPort: 58085, wantErr: false},
+		{name: "highest valid port", ssoPort: 65535, wantErr: false},
+		{name: "zero port", ssoPort: 0, wantErr: true},
+		{name: "negative port", ssoPort: -1, wantErr: true},
+		{name: "port above range", ssoPort: 65536, wantErr: true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			cmd := NewLoginCommand(nil)
+			cmd.SetArgs([]string{"--sso-port", fmt.Sprintf("%d", tt.ssoPort), "http://localhost:8080"})
+
+			executed := false
+			originalRun := cmd.Run
+			cmd.RunE = func(cmd *cobra.Command, args []string) error {
+				executed = true
+				return nil
+			}
+
+			err := cmd.Execute()
+			if tt.wantErr {
+				require.Error(t, err)
+				require.False(t, executed, "RunE should not execute when validation fails")
+			} else {
+				require.NoError(t, err)
+				require.True(t, executed, "RunE should execute when validation passes")
+			}
+			_ = originalRun
+		})
+	}
+}

cmd/context.go

@@ -27,29 +27,28 @@ microcks context http://localhost:8080
 
 # Delete Microcks context
 microcks context http://localhost:8080 --delete`,
-		Run: func(cmd *cobra.Command, args []string) {
+		RunE: func(cmd *cobra.Command, args []string) error {
 			configPath := globalClientOpts.ConfigPath
 			localCfg, err := config.ReadLocalConfig(configPath)
 			errors.CheckError(err)
 			if delete {
 				if len(args) == 0 {
-					cmd.HelpFunc()(cmd, args)
-					os.Exit(1)
+					return fmt.Errorf("context name is required when using --delete")
 				}
 				err := deleteContext(args[0], configPath)
 				errors.CheckError(err)
-				return
+				return nil
 			}
 
 			if len(args) == 0 {
 				printMicrocksContexts(configPath)
-				return
+				return nil
 			}
 
 			ctxName := args[0]
 			if localCfg.CurrentContext == ctxName {
 				fmt.Printf("Already at context '%s'\n", localCfg.CurrentContext)
-				return
+				return nil
 			}
 			if _, err = localCfg.ResolveContext(ctxName); err != nil {
 				log.Fatal(err)
@@ -58,6 +57,7 @@ microcks context http://localhost:8080 --delete`,
 			err = config.WriteLocalConfig(*localCfg, configPath)
 			errors.CheckError(err)
 			fmt.Printf("Switched to context '%s'\n", localCfg.CurrentContext)
+			return nil
 		},
 	}
 

cmd/import.go

@@ -1,23 +1,9 @@
-/*
- * Copyright The Microcks Authors.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *  http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
 package cmd
 
 import (
 	"fmt"
-	"os"
+	"path/filepath"
+	"runtime"
 	"strconv"
 	"strings"
 
@@ -28,67 +14,67 @@ import (
 	"github.com/spf13/cobra"
 )
 
+func parsePathSuffix(arg string) (string, bool) {
+	colonIdx := strings.LastIndex(arg, ":")
+	if colonIdx < 0 {
+		return arg, true
+	}
+	if runtime.GOOS == "windows" && colonIdx == 1 && len(arg) > 2 {
+		if c := arg[0]; (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') {
+			return arg, true
+		}
+	}
+	pathPart := arg[:colonIdx]
+	suffixPart := arg[colonIdx+1:]
+	val, err := strconv.ParseBool(suffixPart)
+	if err != nil {
+		return arg, true
+	}
+	return pathPart, val
+}
+
 func NewImportCommand(globalClientOpts *connectors.ClientOptions) *cobra.Command {
 	var watch bool
 
 	var importCmd = &cobra.Command{
-		Use:   "import",
+		Use:   "import <specificationFile1[:primary],specificationFile2[:primary]>",
 		Short: "import API artifacts on Microcks server",
 		Long:  `import API artifacts on Microcks server`,
-		Args:  cobra.MaximumNArgs(1),
-		Run: func(cmd *cobra.Command, args []string) {
-			// Parse subcommand args first.
-			if len(args) == 0 {
-				fmt.Println("import command require <specificationFile1[:primary],specificationFile2[:primary]> args")
-				cmd.HelpFunc()(cmd, args)
-				os.Exit(1)
-			}
-
+		Args:  cobra.MinimumNArgs(1),
+		RunE: func(cmd *cobra.Command, args []string) error {
 			specificationFiles := args[0]
 
-			// Initialize config from command options.
 			config.InsecureTLS = globalClientOpts.InsecureTLS
 			config.CaCertPaths = globalClientOpts.CaCertPaths
 			config.Verbose = globalClientOpts.Verbose
 
-			// Read local config file in case we need some context info.
 			localConfig, err := config.ReadLocalConfig(globalClientOpts.ConfigPath)
 			if err != nil {
-				fmt.Println(err)
-				return
+				return err
 			}
 
-			// Prepare Microcks client.
 			var mc connectors.MicrocksClient
 
 			if globalClientOpts.ServerAddr != "" && globalClientOpts.ClientId != "" && globalClientOpts.ClientSecret != "" {
-				// Create client with server address.
 				mc = connectors.NewMicrocksClient(globalClientOpts.ServerAddr)
 
 				keycloakURL, err := mc.GetKeycloakURL()
 				if err != nil {
-					fmt.Printf("Got error when invoking Microcks client retrieving config: %s", err)
-					os.Exit(1)
+					return fmt.Errorf("got error when invoking Microcks client retrieving config: %s", err)
 				}
 
 				var oauthToken string = "unauthenticated-token"
 				if keycloakURL != "null" {
-					// If Keycloak is enabled, retrieve an OAuth token using Keycloak Client.
 					kc := connectors.NewKeycloakClient(keycloakURL, globalClientOpts.ClientId, globalClientOpts.ClientSecret)
 
 					oauthToken, err = kc.ConnectAndGetToken()
 					if err != nil {
-						fmt.Printf("Got error when invoking Keycloack client: %s", err)
-						os.Exit(1)
+						return fmt.Errorf("got error when invoking Keycloak client: %s", err)
 					}
-					//fmt.Printf("Retrieve OAuthToken: %s", oauthToken)
 				}
 
-				// Set Auth token.
 				mc.SetOAuthToken(oauthToken)
 
-				// If no context provided use current one from config file or client server address.
-				// So that watch config can be updated properly, referencing the right context.
 				if globalClientOpts.Context == "" {
 					if (localConfig != nil) && (localConfig.CurrentContext != "") {
 						globalClientOpts.Context = localConfig.CurrentContext
@@ -98,10 +84,8 @@ func NewImportCommand(globalClientOpts *connectors.ClientOptions) *cobra.Command
 				}
 
 			} else {
-				// Create client from config file and using the current or provided context.
 				if localConfig == nil {
-					fmt.Println("Please login to perform operation...")
-					return
+					return fmt.Errorf("please login to perform operation")
 				}
 
 				if globalClientOpts.Context == "" {
@@ -110,40 +94,27 @@ func NewImportCommand(globalClientOpts *connectors.ClientOptions) *cobra.Command
 
 				mc, err = connectors.NewClient(*globalClientOpts)
 				if err != nil {
-					fmt.Printf("error %v", err)
-					return
+					return err
 				}
 			}
 
-			// Handle multiple specification files separated by comma.
 			sepSpecificationFiles := strings.Split(specificationFiles, ",")
 			for _, f := range sepSpecificationFiles {
 				mainArtifact := true
 				var err error
 
-				// Check if mainArtifact flag is provided.
-				if strings.Contains(f, ":") {
-					pathAndMainArtifact := strings.Split(f, ":")
-					f = pathAndMainArtifact[0]
-					mainArtifact, err = strconv.ParseBool(pathAndMainArtifact[1])
-					if err != nil {
-						fmt.Printf("Cannot parse '%s' as Bool, default to true\n", pathAndMainArtifact[1])
-					}
-				}
+				f, mainArtifact = parsePathSuffix(f)
 
-				// Try uploading this artifact.
 				msg, err := mc.UploadArtifact(f, mainArtifact)
 				if err != nil {
-					fmt.Printf("Got error when invoking Microcks client importing Artifact: %s", err)
-					os.Exit(1)
+					return fmt.Errorf("got error when invoking Microcks client importing Artifact: %s", err)
 				}
 				action := "discovered"
 				if !mainArtifact {
 					action = "completed"
 				}
 				fmt.Printf("Microcks has %s '%s'\n", action, msg)
 
-				// If watch flag is provided, update watch config.
 				if watch {
 					watchFile, err := config.DefaultLocalWatchPath()
 					errors.CheckError(err)
@@ -154,25 +125,21 @@ func NewImportCommand(globalClientOpts *connectors.ClientOptions) *cobra.Command
 						watchCfg = &config.WatchConfig{}
 					}
 
-					// Normalize file path to match the watcher fsnotify events format.
-					if strings.HasPrefix(f, "./") {
-						f = strings.TrimPrefix(f, "./")
-					}
+					f = strings.TrimPrefix(f, "./")
+					f = strings.TrimPrefix(f, ".\\")
+					f = filepath.Clean(f)
 
-					// Upsert entry.
 					watchCfg.UpsertEntry(config.WatchEntry{
 						FilePath:     f,
 						Context:      []string{globalClientOpts.Context},
 						MainArtifact: mainArtifact,
 					})
 
-					// Write watch file.
 					err = config.WriteLocalWatchConfig(*watchCfg, watchFile)
 					errors.CheckError(err)
 				}
 			}
 
-			// Start watcher if --watch flag is provided.
 			if watch {
 				watchFile, err := config.DefaultLocalWatchPath()
 				errors.CheckError(err)
@@ -183,6 +150,7 @@ func NewImportCommand(globalClientOpts *connectors.ClientOptions) *cobra.Command
 				fmt.Println("Watch mode enabled - microcks-watcher started...")
 				wm.Run()
 			}
+			return nil
 		},
 	}