From 022e7a3b108e4b5bbefc77c29659eddc5b165763 Mon Sep 17 00:00:00 2001 From: syedowais312 Date: Sun, 10 May 2026 02:09:45 +0530 Subject: [PATCH] refactor: extract test command validation helpers Signed-off-by: syedowais312 --- cmd/test.go | 110 ++++++++++++++++-------------- cmd/test_test.go | 170 ++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 200 insertions(+), 80 deletions(-) diff --git a/cmd/test.go b/cmd/test.go index 2d76db54..2259c60d 100644 --- a/cmd/test.go +++ b/cmd/test.go @@ -32,6 +32,10 @@ var ( runnerChoices = map[string]bool{"HTTP": true, "SOAP_HTTP": true, "SOAP_UI": true, "POSTMAN": true, "OPEN_API_SCHEMA": true, "ASYNC_API_SCHEMA": true, "GRPC_PROTOBUF": true, "GRAPHQL_SCHEMA": true} ) +const testCommandUsageError = "test command require args" +const testCommandRunnerError = " should be one of: HTTP, SOAP, SOAP_UI, POSTMAN, OPEN_API_SCHEMA, ASYNC_API_SCHEMA, GRPC_PROTOBUF, GRAPHQL_SCHEMA" +const testCommandWaitForFormatError = "--waitFor format is wrong. Accepted units are: milli, sec, min (e.g. 500milli, 30sec, 5min)" + func NewTestCommand(globalClientOpts *connectors.ClientOptions) *cobra.Command { var ( waitFor string @@ -47,32 +51,9 @@ func NewTestCommand(globalClientOpts *connectors.ClientOptions) *cobra.Command { Long: `Run tests on Microcks`, Args: cobra.ExactArgs(3), Run: func(cmd *cobra.Command, args []string) { - - serviceRef := args[0] - testEndpoint := args[1] - runnerType := args[2] - - // Validate presence and values of args. - if len(serviceRef) == 0 || strings.HasPrefix(serviceRef, "-") { - fmt.Println("test command require args") - os.Exit(1) - } - if len(testEndpoint) == 0 || strings.HasPrefix(testEndpoint, "-") { - fmt.Println("test command require args") - os.Exit(1) - } - if len(runnerType) == 0 || strings.HasPrefix(runnerType, "-") { - fmt.Println("test command require args") - os.Exit(1) - } - if _, validChoice := runnerChoices[runnerType]; !validChoice { - fmt.Println(" should be one of: HTTP, SOAP_HTTP, SOAP_UI, POSTMAN, OPEN_API_SCHEMA, ASYNC_API_SCHEMA, GRPC_PROTOBUF, GRAPHQL_SCHEMA") - os.Exit(1) - } - - // Validate presence and values of flags. - if !strings.HasSuffix(waitFor, "milli") && !strings.HasSuffix(waitFor, "sec") && !strings.HasSuffix(waitFor, "min") { - fmt.Println("--waitFor format is wrong. Accepted units are: milli, sec, min (e.g. 500milli, 30sec, 5min)") + serviceRef, testEndpoint, runnerType, err := validateTestCommandArgs(args) + if err != nil { + fmt.Println(err.Error()) os.Exit(1) } @@ -81,29 +62,10 @@ func NewTestCommand(globalClientOpts *connectors.ClientOptions) *cobra.Command { config.CaCertPaths = globalClientOpts.CaCertPaths config.Verbose = globalClientOpts.Verbose - // Compute time to wait in milliseconds. - var waitForMilliseconds int64 - if strings.HasSuffix(waitFor, "milli") { - n, err := strconv.ParseInt(waitFor[:len(waitFor)-5], 0, 64) - if err != nil { - fmt.Printf("--waitFor value %q is not a valid number\n", waitFor) - os.Exit(1) - } - waitForMilliseconds = n - } else if strings.HasSuffix(waitFor, "sec") { - n, err := strconv.ParseInt(waitFor[:len(waitFor)-3], 0, 64) - if err != nil { - fmt.Printf("--waitFor value %q is not a valid number\n", waitFor) - os.Exit(1) - } - waitForMilliseconds = n * 1000 - } else if strings.HasSuffix(waitFor, "min") { - n, err := strconv.ParseInt(waitFor[:len(waitFor)-3], 0, 64) - if err != nil { - fmt.Printf("--waitFor value %q is not a valid number\n", waitFor) - os.Exit(1) - } - waitForMilliseconds = n * 60 * 1000 + waitForMilliseconds, err := parseWaitForMilliseconds(waitFor) + if err != nil { + fmt.Println(err.Error()) + os.Exit(1) } var mc connectors.MicrocksClient @@ -165,7 +127,8 @@ func NewTestCommand(globalClientOpts *connectors.ClientOptions) *cobra.Command { serverAddr = ctx.Server.Server } - testResultID, err := mc.CreateTestResult(serviceRef, testEndpoint, runnerType, secretName, waitForMilliseconds, filteredOperations, operationsHeaders, oAuth2Context) + var testResultID string + testResultID, err = mc.CreateTestResult(serviceRef, testEndpoint, runnerType, secretName, waitForMilliseconds, filteredOperations, operationsHeaders, oAuth2Context) if err != nil { fmt.Printf("Got error when invoking Microcks client creating Test: %s", err) os.Exit(1) @@ -215,6 +178,53 @@ func NewTestCommand(globalClientOpts *connectors.ClientOptions) *cobra.Command { return testCmd } +func validateTestCommandArgs(args []string) (string, string, string, error) { + if len(args) < 3 { + return "", "", "", fmt.Errorf(testCommandUsageError) + } + + serviceRef := args[0] + testEndpoint := args[1] + runnerType := args[2] + + if len(serviceRef) == 0 || strings.HasPrefix(serviceRef, "-") { + return "", "", "", fmt.Errorf(testCommandUsageError) + } + if len(testEndpoint) == 0 || strings.HasPrefix(testEndpoint, "-") { + return "", "", "", fmt.Errorf(testCommandUsageError) + } + if len(runnerType) == 0 || strings.HasPrefix(runnerType, "-") { + return "", "", "", fmt.Errorf(testCommandUsageError) + } + if _, validChoice := runnerChoices[runnerType]; !validChoice { + return "", "", "", fmt.Errorf(testCommandRunnerError) + } + + return serviceRef, testEndpoint, runnerType, nil +} + +func parseWaitForMilliseconds(waitFor string) (int64, error) { + if !strings.HasSuffix(waitFor, "milli") && !strings.HasSuffix(waitFor, "sec") && !strings.HasSuffix(waitFor, "min") { + return 0, fmt.Errorf(testCommandWaitForFormatError) + } + + if strings.HasSuffix(waitFor, "milli") { + return parseWaitForValue(waitFor, "milli", 1) + } + if strings.HasSuffix(waitFor, "sec") { + return parseWaitForValue(waitFor, "sec", 1000) + } + return parseWaitForValue(waitFor, "min", 60*1000) +} + +func parseWaitForValue(waitFor string, suffix string, multiplier int64) (int64, error) { + n, err := strconv.ParseInt(waitFor[:len(waitFor)-len(suffix)], 0, 64) + if err != nil { + return 0, fmt.Errorf("--waitFor value %q is not a valid number", waitFor) + } + return n * multiplier, nil +} + func nowInMilliseconds() int64 { return time.Now().UnixNano() / int64(time.Millisecond) } diff --git a/cmd/test_test.go b/cmd/test_test.go index 5cff0962..e4c9d51a 100644 --- a/cmd/test_test.go +++ b/cmd/test_test.go @@ -16,44 +16,154 @@ package cmd import ( - "os" - "os/exec" "testing" + "github.com/microcks/microcks-cli/pkg/connectors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -const missingTestArgsMessage = "accepts 3 arg(s), received 2" - -func TestTestCommandMissingRunnerWithGlobalFlagsDoesNotPanic(t *testing.T) { - if os.Getenv("MICROCKS_CLI_TEST_MISSING_RUNNER") == "1" { - os.Args = []string{ - os.Args[0], - "test", - "--microcksURL=http://localhost:8080", - "--keycloakClientId=foo", - "--keycloakClientSecret=bar", - "MyAPI:1.0", - "http://localhost:3000", - } - - err := NewCommand().Execute() - if err != nil { - t.Fatal(err) - } - return +func TestValidateTestCommandArgs(t *testing.T) { + tests := []struct { + name string + args []string + wantErr bool + errContains string + }{ + // valid + { + name: "valid HTTP runner", + args: []string{"API:1.0", "http://localhost:8080", "HTTP"}, + wantErr: false, + }, + { + name: "valid POSTMAN runner", + args: []string{"Petstore:1.0", "http://localhost:9090", "POSTMAN"}, + wantErr: false, + }, + { + name: "valid OPEN_API_SCHEMA runner", + args: []string{"API:2.0", "http://endpoint", "OPEN_API_SCHEMA"}, + wantErr: false, + }, + + // missing args + { + name: "no args", + args: []string{}, + wantErr: true, + errContains: testCommandUsageError, + }, + { + name: "missing runner", + args: []string{"API:1.0", "http://endpoint"}, + wantErr: true, + errContains: testCommandUsageError, + }, + + // args look like flags + { + name: "serviceRef starts with dash", + args: []string{"-flag", "http://endpoint", "HTTP"}, + wantErr: true, + errContains: testCommandUsageError, + }, + { + name: "testEndpoint starts with dash", + args: []string{"API:1.0", "--endpoint", "HTTP"}, + wantErr: true, + errContains: testCommandUsageError, + }, + + // invalid runner + { + name: "invalid runner INVALID", + args: []string{"API:1.0", "http://endpoint", "INVALID"}, + wantErr: true, + errContains: testCommandRunnerError, + }, + { + name: "runner lowercase http", + args: []string{"API:1.0", "http://endpoint", "http"}, + wantErr: true, + errContains: testCommandRunnerError, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, _, _, err := validateTestCommandArgs(tt.args) + if (err != nil) != tt.wantErr { + t.Errorf("validateTestCommandArgs() error = %v, wantErr %v", err, tt.wantErr) + } + if tt.wantErr && tt.errContains != "" && err != nil { + if err.Error() != tt.errContains { + t.Errorf("error message = %q, want %q", err.Error(), tt.errContains) + } + } + }) } +} + +func TestParseWaitForMilliseconds(t *testing.T) { + tests := []struct { + input string + expected int64 + wantErr bool + }{ + // valid + {"500milli", 500, false}, + {"1sec", 1000, false}, + {"2min", 120000, false}, + {"0milli", 0, false}, + {"30sec", 30000, false}, + + // wrong suffix + {"5seconds", 0, true}, + {"5mins", 0, true}, + {"5ms", 0, true}, + {"", 0, true}, + + // bad numeric prefix + {"abcsec", 0, true}, + {"sec", 0, true}, // missing number + {"milli", 0, true}, // missing number + {"1.5sec", 0, true}, // float not accepted + } + + for _, tt := range tests { + t.Run(tt.input, func(t *testing.T) { + got, err := parseWaitForMilliseconds(tt.input) + if (err != nil) != tt.wantErr { + t.Errorf("parseWaitForMilliseconds(%q) error = %v, wantErr %v", tt.input, err, tt.wantErr) + } + if !tt.wantErr && got != tt.expected { + t.Errorf("parseWaitForMilliseconds(%q) = %d, want %d", tt.input, got, tt.expected) + } + }) + } +} + +func TestNewTestCommand(t *testing.T) { + clientOpts := &connectors.ClientOptions{} + cmd := NewTestCommand(clientOpts) + + assert.Equal(t, "test ", cmd.Use) + assert.Equal(t, "Run tests on Microcks", cmd.Short) + + waitForFlag := cmd.Flags().Lookup("waitFor") + require.NotNil(t, waitForFlag) + assert.Equal(t, "5sec", waitForFlag.DefValue) + + secretFlag := cmd.Flags().Lookup("secretName") + require.NotNil(t, secretFlag) - cmd := exec.Command(os.Args[0], "-test.run=TestTestCommandMissingRunnerWithGlobalFlagsDoesNotPanic") - cmd.Env = append(os.Environ(), "MICROCKS_CLI_TEST_MISSING_RUNNER=1") + filteredOperationsFlag := cmd.Flags().Lookup("filteredOperations") + require.NotNil(t, filteredOperationsFlag) - output, err := cmd.CombinedOutput() - require.Error(t, err) + operationsHeadersFlag := cmd.Flags().Lookup("operationsHeaders") + require.NotNil(t, operationsHeadersFlag) - exitError, ok := err.(*exec.ExitError) - require.True(t, ok) - assert.Equal(t, 1, exitError.ExitCode()) - assert.Contains(t, string(output), missingTestArgsMessage) - assert.NotContains(t, string(output), "panic:") // core regression guard: must never crash with a stack trace + oauth2ContextFlag := cmd.Flags().Lookup("oAuth2Context") + require.NotNil(t, oauth2ContextFlag) }