Changing one of cli tests to be more explicit (#7211)

In order to migrate to urfave v3 tests should call .Run explicitely -
with command line arguments. There is no cli.NewContext() anymore.

I think it is even better to have a whole command-line instead of []string -
then one can easily take this usage.

Author: dkrotx

tools/cli/admin_async_queue_commands_test.go

@@ -23,16 +23,15 @@
 package cli
 
 import (
-	"flag"
 	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
-	"github.com/urfave/cli/v2"
 	"go.uber.org/mock/gomock"
 
 	"github.com/uber/cadence/client/admin"
 	"github.com/uber/cadence/common/types"
+	"github.com/uber/cadence/tools/cli/clitest"
 )
 
 func TestAdminGetAsyncWFConfig(t *testing.T) {
@@ -44,7 +43,7 @@ func TestAdminGetAsyncWFConfig(t *testing.T) {
 		setupMocks       func(*admin.MockClient)
 		expectedError    string
 		expectedStr      string
-		flagDomain       string
+		cmdline          string
 		mockDepsError    error
 		mockContextError error
 	}{
@@ -64,15 +63,15 @@ func TestAdminGetAsyncWFConfig(t *testing.T) {
 			},
 			expectedError: "",
 			expectedStr:   "PredefinedQueueName",
-			flagDomain:    "test-domain",
+			cmdline:       "cadence --domain test-domain admin async-wf-queue get",
 		},
 		{
 			name: "Required flag not present",
 			setupMocks: func(client *admin.MockClient) {
 				// No call to the mock admin client is expected
 			},
 			expectedError: "Required flag not present:",
-			flagDomain:    "",
+			cmdline:       "cadence admin async-wf-queue get", // --domain is missing
 		},
 		{
 			name: "Config not found (resp.Configuration == nil)",
@@ -85,7 +84,7 @@ func TestAdminGetAsyncWFConfig(t *testing.T) {
 					Times(1)
 			},
 			expectedError: "",
-			flagDomain:    "test-domain",
+			cmdline:       "cadence --domain test-domain admin async-wf-queue get",
 		},
 		{
 			name: "Failed to get async wf config",
@@ -96,7 +95,7 @@ func TestAdminGetAsyncWFConfig(t *testing.T) {
 					Times(1)
 			},
 			expectedError: "Failed to get async wf queue config",
-			flagDomain:    "test-domain",
+			cmdline:       "cadence --domain test-domain admin async-wf-queue get",
 		},
 	}
 
@@ -114,15 +113,7 @@ func TestAdminGetAsyncWFConfig(t *testing.T) {
 				serverAdminClient: adminClient,
 			}, WithIOHandler(ioHandler))
 
-			// Set up CLI context with flags
-			set := flag.NewFlagSet("test", 0)
-			set.String(FlagDomain, tt.flagDomain, "Domain flag")
-			c := cli.NewContext(app, set, nil)
-
-			// Call the function under test
-			err := AdminGetAsyncWFConfig(c)
-
-			// Check the expected outcome
+			err := clitest.RunCommandLine(t, app, tt.cmdline)
 			if tt.expectedError != "" {
 				assert.Error(t, err)
 				assert.Contains(t, err.Error(), tt.expectedError)
@@ -143,8 +134,7 @@ func TestAdminUpdateAsyncWFConfig(t *testing.T) {
 		name             string
 		setupMocks       func(*admin.MockClient)
 		expectedError    string
-		flagDomain       string
-		flagJSON         string
+		cmdline          string
 		mockContextError error
 		unmarshalError   error
 	}{
@@ -157,35 +147,31 @@ func TestAdminUpdateAsyncWFConfig(t *testing.T) {
 					Times(1)
 			},
 			expectedError: "",
-			flagDomain:    "test-domain",
-			flagJSON:      `{"Enabled": true}`,
+			cmdline:       `cadence --domain test-domain admin async-wf-queue update --json '{"Enabled": true}'`,
 		},
 		{
 			name: "Required flag not present for domain",
 			setupMocks: func(client *admin.MockClient) {
 				// No call to the mock admin client is expected
 			},
 			expectedError: "Required flag not present:",
-			flagDomain:    "",
-			flagJSON:      `{"Enabled": true}`,
+			cmdline:       `cadence admin async-wf-queue update --json '{"Enabled": true}'`, // --domain is missing
 		},
 		{
 			name: "Required flag not present for JSON",
 			setupMocks: func(client *admin.MockClient) {
 				// No call to the mock admin client is expected
 			},
 			expectedError: "Required flag not present:",
-			flagDomain:    "test-domain",
-			flagJSON:      "",
+			cmdline:       `cadence --domain test-domain admin async-wf-queue update --json ""`, // empty --json flag
 		},
 		{
 			name: "Failed to parse async workflow config",
 			setupMocks: func(client *admin.MockClient) {
 				// No call setup for this test case as JSON parsing fails
 			},
 			expectedError:  "Failed to parse async workflow config",
-			flagDomain:     "test-domain",
-			flagJSON:       `invalid-json`,
+			cmdline:        `cadence --domain test-domain admin async-wf-queue update --json invalid-json`,
 			unmarshalError: fmt.Errorf("unmarshal error"),
 		},
 		{
@@ -197,8 +183,7 @@ func TestAdminUpdateAsyncWFConfig(t *testing.T) {
 					Times(1)
 			},
 			expectedError: "Failed to update async workflow queue config",
-			flagDomain:    "test-domain",
-			flagJSON:      `{"Enabled": true}`,
+			cmdline:       `cadence --domain test-domain admin async-wf-queue update --json '{"Enabled": true}'`,
 		},
 	}
 
@@ -215,23 +200,13 @@ func TestAdminUpdateAsyncWFConfig(t *testing.T) {
 				serverAdminClient: adminClient,
 			})
 
-			// Set up CLI context with flags
-			set := flag.NewFlagSet("test", 0)
-			set.String(FlagDomain, tt.flagDomain, "Domain flag")
-			set.String(FlagJSON, tt.flagJSON, "JSON flag")
-			c := cli.NewContext(app, set, nil)
-
-			// Call the function under test
-			err := AdminUpdateAsyncWFConfig(c)
-
-			// Check the expected outcome
+			err := clitest.RunCommandLine(t, app, tt.cmdline)
 			if tt.expectedError != "" {
 				assert.Error(t, err)
 				assert.Contains(t, err.Error(), tt.expectedError)
 			} else {
 				assert.NoError(t, err)
 			}
-
 		})
 	}
 }

tools/cli/app_test.go

@@ -29,12 +29,10 @@ import (
 	"testing"
 	"time"
 
-	"github.com/flynn/go-shlex"
 	"github.com/olekukonko/tablewriter"
 	"github.com/olivere/elastic"
 	"github.com/pborman/uuid"
 	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 	"github.com/urfave/cli/v2"
 	"go.uber.org/mock/gomock"
@@ -44,6 +42,7 @@ import (
 	"github.com/uber/cadence/common"
 	"github.com/uber/cadence/common/config"
 	"github.com/uber/cadence/common/types"
+	"github.com/uber/cadence/tools/cli/clitest"
 )
 
 type (
@@ -159,10 +158,7 @@ func (s *cliAppSuite) runTestCase(tt testcase) {
 		tt.mock()
 	}
 
-	args, err := shlex.Split(tt.command)
-	require.NoError(s.T(), err)
-	err = s.app.Run(args)
-
+	err := clitest.RunCommandLine(s.T(), s.app, tt.command)
 	if tt.err != "" {
 		assert.ErrorContains(s.T(), err, tt.err)
 	} else {

tools/cli/clitest/runner.go

@@ -0,0 +1,18 @@
+package clitest
+
+import (
+	"testing"
+
+	"github.com/flynn/go-shlex"
+	"github.com/stretchr/testify/require"
+	"github.com/urfave/cli/v2"
+)
+
+// RunCommandLine runs cmdline (a whole line like you'd type it in Shell)
+func RunCommandLine(t *testing.T, app *cli.App, cmdline string) error {
+	t.Helper()
+
+	args, err := shlex.Split(cmdline)
+	require.NoError(t, err)
+	return app.Run(args)
+}

tools/cli/clitest/runner_test.go

@@ -0,0 +1,48 @@
+package clitest
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"github.com/urfave/cli/v2"
+)
+
+func TestRunCommandLine(t *testing.T) {
+	var output string
+
+	testApp := &cli.App{
+		Name: "devmngr",
+		Commands: []*cli.Command{
+			{
+				Name: "upgrade",
+				Flags: []cli.Flag{
+					&cli.StringFlag{Name: "device"},
+					&cli.BoolFlag{Name: "enabled"},
+					&cli.StringFlag{Name: "reason"},
+				},
+				Action: func(c *cli.Context) error {
+					output = fmt.Sprintf(
+						"upgrading device: \"%s\"; enabled: %v; reason: \"%s\"",
+						c.String("device"),
+						c.Bool("enabled"),
+						c.String("reason"),
+					)
+					return nil
+				},
+			},
+		},
+	}
+
+	err := RunCommandLine(
+		t,
+		testApp,
+		"devmngr upgrade --device video-card --enabled --reason \"Doom: The Dark Ages is released\"",
+	)
+	require.NoError(t, err)
+	assert.Equal(t,
+		"upgrading device: \"video-card\"; enabled: true; reason: \"Doom: The Dark Ages is released\"",
+		output,
+	)
+}