From 9242f476838d5668459f7b549180769c2e547897 Mon Sep 17 00:00:00 2001 From: Danny Olson Date: Mon, 25 Mar 2024 16:22:41 -0700 Subject: [PATCH] Refactor flag cmd tests --- cmd/cmdtest.go | 30 ++++++++++++--- cmd/flags/create.go | 51 ++++++++++++------------- cmd/flags/create_test.go | 75 +++++++++++++++++++++++++++++++++++++ cmd/flags/flags.go | 14 ++++--- cmd/flags/update.go | 50 +++++++++++++------------ cmd/projects/create_test.go | 23 +++--------- cmd/projects/list_test.go | 10 ++--- cmd/root.go | 11 +++--- internal/flags/flags.go | 53 +++++++++++++++++--------- internal/flags/mock.go | 41 ++++++++++++++++++++ internal/projects/mock.go | 2 - 11 files changed, 254 insertions(+), 106 deletions(-) create mode 100644 cmd/flags/create_test.go create mode 100644 internal/flags/mock.go diff --git a/cmd/cmdtest.go b/cmd/cmdtest.go index 5bd54720..ea56fee7 100644 --- a/cmd/cmdtest.go +++ b/cmd/cmdtest.go @@ -7,13 +7,21 @@ import ( "github.com/stretchr/testify/require" + "ldcli/internal/flags" "ldcli/internal/projects" ) var ValidResponse = `{"valid": true}` -func ArgsValidCreate() []string { - args := append(ArgsCreateCommand(), ArgsAccess()...) +func ArgsValidFlagsCreate() []string { + args := append(ArgsFlagsCreateCommand(), ArgsAccess()...) + args = append(args, ArgsData()...) + + return args +} + +func ArgsValidProjectsCreate() []string { + args := append(ArgsProjectsCreateCommand(), ArgsAccess()...) args = append(args, ArgsData()...) return args @@ -39,7 +47,14 @@ func ArgsAccess() []string { } } -func ArgsCreateCommand() []string { +func ArgsFlagsCreateCommand() []string { + return []string{ + "flags", + "create", + } +} + +func ArgsProjectsCreateCommand() []string { return []string{ "projects", "create", @@ -53,8 +68,13 @@ func ArgsListCommand() []string { } } -func CallCmd(t *testing.T, client *projects.MockClient, args []string) ([]byte, error) { - rootCmd, err := NewRootCommand(client) +func CallCmd( + t *testing.T, + flagsClient *flags.MockClient, + projectsClient *projects.MockClient, + args []string, +) ([]byte, error) { + rootCmd, err := NewRootCommand(flagsClient, projectsClient) require.NoError(t, err) b := bytes.NewBufferString("") rootCmd.SetOut(b) diff --git a/cmd/flags/create.go b/cmd/flags/create.go index a56eb233..84f7d448 100644 --- a/cmd/flags/create.go +++ b/cmd/flags/create.go @@ -13,13 +13,13 @@ import ( "ldcli/internal/flags" ) -func NewCreateCmd() (*cobra.Command, error) { +func NewCreateCmd(client flags.Client) (*cobra.Command, error) { cmd := &cobra.Command{ Use: "create", Short: "Create a new flag", Long: "Create a new flag", PreRunE: validate, - RunE: runCreate, + RunE: runCreate(client), } cmd.Flags().StringP("data", "d", "", "Input data in JSON") @@ -50,33 +50,34 @@ type inputData struct { Key string `json:"key"` } -func runCreate(cmd *cobra.Command, args []string) error { - client := flags.NewClient( - viper.GetString("accessToken"), - viper.GetString("baseUri"), - ) +func runCreate(client flags.Client) func(*cobra.Command, []string) error { + return func(cmd *cobra.Command, args []string) error { + // rebind flags used in other subcommands + _ = viper.BindPFlag("data", cmd.Flags().Lookup("data")) + _ = viper.BindPFlag("projKey", cmd.Flags().Lookup("projKey")) - var data inputData - err := json.Unmarshal([]byte(cmd.Flags().Lookup("data").Value.String()), &data) - // err := json.Unmarshal([]byte(viper.GetString("data")), &data) - if err != nil { - return err - } - projKey := viper.GetString("projKey") + var data inputData + err := json.Unmarshal([]byte(viper.GetString("data")), &data) + if err != nil { + return err + } - response, err := client.Create( - context.Background(), - data.Name, - data.Key, - projKey, - ) - if err != nil { - return err - } + response, err := client.Create( + context.Background(), + viper.GetString("accessToken"), + viper.GetString("baseUri"), + data.Name, + data.Key, + viper.GetString("projKey"), + ) + if err != nil { + return err + } - fmt.Fprintf(cmd.OutOrStdout(), string(response)+"\n") + fmt.Fprintf(cmd.OutOrStdout(), string(response)+"\n") - return nil + return nil + } } // validate ensures the flags are valid before using them. diff --git a/cmd/flags/create_test.go b/cmd/flags/create_test.go new file mode 100644 index 00000000..7c73f739 --- /dev/null +++ b/cmd/flags/create_test.go @@ -0,0 +1,75 @@ +package flags_test + +import ( + "ldcli/cmd" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "ldcli/internal/errors" + "ldcli/internal/flags" +) + +func TestCreate(t *testing.T) { + t.Run("with valid flags calls projects API", func(t *testing.T) { + client := flags.MockClient{} + client. + On("Create", "testAccessToken", "http://test.com", "test-name", "test-key", "test-proj-key"). + Return([]byte(cmd.ValidResponse), nil) + args := []string{ + "flags", "create", + "-t", "testAccessToken", + "-u", "http://test.com", + "-d", `{"key": "test-key", "name": "test-name"}`, + "--projKey", "test-proj-key", + } + + output, err := cmd.CallCmd(t, &client, nil, args) + + require.NoError(t, err) + assert.JSONEq(t, `{"valid": true}`, string(output)) + }) + + t.Run("with an error response is an error", func(t *testing.T) { + client := flags.MockClient{} + client. + On("Create", "testAccessToken", "http://test.com", "test-name", "test-key", "test-proj-key"). + Return([]byte(`{}`), errors.NewError("An error")) + args := []string{ + "flags", "create", + "-t", "testAccessToken", + "-u", "http://test.com", + "-d", `{"key": "test-key", "name": "test-name"}`, + "--projKey", "test-proj-key", + } + + _, err := cmd.CallCmd(t, &client, nil, args) + + require.EqualError(t, err, "An error") + }) + + t.Run("with missing required flags is an error", func(t *testing.T) { + args := []string{ + "flags", "create", + } + + _, err := cmd.CallCmd(t, &flags.MockClient{}, nil, args) + + assert.EqualError(t, err, `required flag(s) "accessToken", "data", "projKey" not set`) + }) + + t.Run("with invalid baseUri is an error", func(t *testing.T) { + args := []string{ + "flags", "create", + "-t", "testAccessToken", + "-u", "invalid", + "-d", `{"key": "test-key", "name": "test-name"}`, + "--projKey", "test-proj-key", + } + + _, err := cmd.CallCmd(t, &flags.MockClient{}, nil, args) + + assert.EqualError(t, err, "baseUri is invalid") + }) +} diff --git a/cmd/flags/flags.go b/cmd/flags/flags.go index 596d7583..a32772ac 100644 --- a/cmd/flags/flags.go +++ b/cmd/flags/flags.go @@ -1,25 +1,29 @@ package flags -import "github.com/spf13/cobra" +import ( + "github.com/spf13/cobra" -func NewFlagsCmd() (*cobra.Command, error) { + "ldcli/internal/flags" +) + +func NewFlagsCmd(client flags.Client) (*cobra.Command, error) { cmd := &cobra.Command{ Use: "flags", Short: "Make requests (list, create, etc.) on flags", Long: "Make requests (list, create, etc.) on flags", } - updateCmd, err := NewUpdateCmd() + createCmd, err := NewCreateCmd(client) if err != nil { return nil, err } - createCmd, err := NewCreateCmd() + updateCmd, err := NewUpdateCmd(client) if err != nil { return nil, err } - cmd.AddCommand(updateCmd) cmd.AddCommand(createCmd) + cmd.AddCommand(updateCmd) return cmd, nil } diff --git a/cmd/flags/update.go b/cmd/flags/update.go index 06fa702e..fcbed697 100644 --- a/cmd/flags/update.go +++ b/cmd/flags/update.go @@ -12,13 +12,13 @@ import ( "ldcli/internal/flags" ) -func NewUpdateCmd() (*cobra.Command, error) { +func NewUpdateCmd(client flags.Client) (*cobra.Command, error) { cmd := &cobra.Command{ Use: "update", Short: "Update a flag", Long: "Update a flag", PreRunE: validate, - RunE: runUpdate, + RunE: runUpdate(client), } var data string @@ -61,30 +61,32 @@ func NewUpdateCmd() (*cobra.Command, error) { return cmd, nil } -func runUpdate(cmd *cobra.Command, args []string) error { - client := flags.NewClient( - viper.GetString("accessToken"), - viper.GetString("baseUri"), - ) +func runUpdate(client flags.Client) func(*cobra.Command, []string) error { + return func(cmd *cobra.Command, args []string) error { + // rebind flags used in other subcommands + _ = viper.BindPFlag("data", cmd.Flags().Lookup("data")) + _ = viper.BindPFlag("projKey", cmd.Flags().Lookup("projKey")) - var patch []ldapi.PatchOperation - // err := json.Unmarshal([]byte(viper.GetString("data")), &patch) - err := json.Unmarshal([]byte(cmd.Flags().Lookup("data").Value.String()), &patch) - if err != nil { - return err - } + var patch []ldapi.PatchOperation + err := json.Unmarshal([]byte(viper.GetString("data")), &patch) + if err != nil { + return err + } - response, err := client.Update( - context.Background(), - viper.GetString("key"), - viper.GetString("projKey"), - patch, - ) - if err != nil { - return err - } + response, err := client.Update( + context.Background(), + viper.GetString("accessToken"), + viper.GetString("baseUri"), + viper.GetString("key"), + viper.GetString("projKey"), + patch, + ) + if err != nil { + return err + } - fmt.Fprintf(cmd.OutOrStdout(), string(response)+"\n") + fmt.Fprintf(cmd.OutOrStdout(), string(response)+"\n") - return nil + return nil + } } diff --git a/cmd/projects/create_test.go b/cmd/projects/create_test.go index 284ef41a..fe95a294 100644 --- a/cmd/projects/create_test.go +++ b/cmd/projects/create_test.go @@ -18,7 +18,7 @@ func TestCreate(t *testing.T) { On("Create", "testAccessToken", "http://test.com", "test-name", "test-key"). Return([]byte(cmd.ValidResponse), nil) - output, err := cmd.CallCmd(t, &client, cmd.ArgsValidCreate()) + output, err := cmd.CallCmd(t, nil, &client, cmd.ArgsValidProjectsCreate()) require.NoError(t, err) assert.JSONEq(t, `{"valid": true}`, string(output)) @@ -28,32 +28,21 @@ func TestCreate(t *testing.T) { client := projects.MockClient{} client. On("Create", "testAccessToken", "http://test.com", "test-name", "test-key"). - Return([]byte(`{}`), errors.NewError("You are not authorized to make this request")) + Return([]byte(`{}`), errors.NewError("An error")) - _, err := cmd.CallCmd(t, &client, cmd.ArgsValidCreate()) + _, err := cmd.CallCmd(t, nil, &client, cmd.ArgsValidProjectsCreate()) - require.EqualError(t, err, "You are not authorized to make this request") - }) - - t.Run("with a forbidden response is an error", func(t *testing.T) { - client := projects.MockClient{} - client. - On("Create", "testAccessToken", "http://test.com", "test-name", "test-key"). - Return([]byte(`{}`), errors.NewError("You do not have permission to make this request")) - - _, err := cmd.CallCmd(t, &client, cmd.ArgsValidCreate()) - - require.EqualError(t, err, "You do not have permission to make this request") + require.EqualError(t, err, "An error") }) t.Run("with missing required flags is an error", func(t *testing.T) { - _, err := cmd.CallCmd(t, &projects.MockClient{}, cmd.ArgsCreateCommand()) + _, err := cmd.CallCmd(t, nil, &projects.MockClient{}, cmd.ArgsProjectsCreateCommand()) assert.EqualError(t, err, `required flag(s) "accessToken", "data" not set`) }) t.Run("with invalid baseUri is an error", func(t *testing.T) { - _, err := cmd.CallCmd(t, &projects.MockClient{}, append(cmd.ArgsCreateCommand(), "--baseUri", "invalid")) + _, err := cmd.CallCmd(t, nil, &projects.MockClient{}, append(cmd.ArgsProjectsCreateCommand(), "--baseUri", "invalid")) assert.EqualError(t, err, "baseUri is invalid") }) diff --git a/cmd/projects/list_test.go b/cmd/projects/list_test.go index 32b3d50e..6997c6ce 100644 --- a/cmd/projects/list_test.go +++ b/cmd/projects/list_test.go @@ -18,7 +18,7 @@ func TestList(t *testing.T) { On("List", "testAccessToken", "http://test.com"). Return([]byte(cmd.ValidResponse), nil) - output, err := cmd.CallCmd(t, &client, cmd.ArgsValidList()) + output, err := cmd.CallCmd(t, nil, &client, cmd.ArgsValidList()) require.NoError(t, err) assert.JSONEq(t, `{"valid": true}`, string(output)) @@ -30,7 +30,7 @@ func TestList(t *testing.T) { On("List", "testAccessToken", "http://test.com"). Return([]byte(`{}`), errors.NewError("You are not authorized to make this request")) - _, err := cmd.CallCmd(t, &client, cmd.ArgsValidList()) + _, err := cmd.CallCmd(t, nil, &client, cmd.ArgsValidList()) require.EqualError(t, err, "You are not authorized to make this request") }) @@ -41,19 +41,19 @@ func TestList(t *testing.T) { On("List", "testAccessToken", "http://test.com"). Return([]byte(`{}`), errors.NewError("You do not have permission to make this request")) - _, err := cmd.CallCmd(t, &client, cmd.ArgsValidList()) + _, err := cmd.CallCmd(t, nil, &client, cmd.ArgsValidList()) require.EqualError(t, err, "You do not have permission to make this request") }) t.Run("with missing required flags is an error", func(t *testing.T) { - _, err := cmd.CallCmd(t, &projects.MockClient{}, cmd.ArgsListCommand()) + _, err := cmd.CallCmd(t, nil, &projects.MockClient{}, cmd.ArgsListCommand()) assert.EqualError(t, err, `required flag(s) "accessToken" not set`) }) t.Run("with invalid baseUri is an error", func(t *testing.T) { - _, err := cmd.CallCmd(t, &projects.MockClient{}, append(cmd.ArgsListCommand(), "--baseUri", "invalid")) + _, err := cmd.CallCmd(t, nil, &projects.MockClient{}, append(cmd.ArgsListCommand(), "--baseUri", "invalid")) assert.EqualError(t, err, "baseUri is invalid") }) diff --git a/cmd/root.go b/cmd/root.go index c81101e1..0d00dbae 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -9,13 +9,14 @@ import ( "github.com/spf13/cobra" "github.com/spf13/viper" - "ldcli/cmd/flags" + flagscmd "ldcli/cmd/flags" projcmd "ldcli/cmd/projects" errs "ldcli/internal/errors" + "ldcli/internal/flags" "ldcli/internal/projects" ) -func NewRootCommand(client projects.Client) (*cobra.Command, error) { +func NewRootCommand(flagsClient flags.Client, projectsClient projects.Client) (*cobra.Command, error) { cmd := &cobra.Command{ Use: "ldcli", Short: "LaunchDarkly CLI", @@ -55,11 +56,11 @@ func NewRootCommand(client projects.Client) (*cobra.Command, error) { return nil, err } - projectsCmd, err := projcmd.NewProjectsCmd(client) + projectsCmd, err := projcmd.NewProjectsCmd(projectsClient) if err != nil { return nil, err } - flagsCmd, err := flags.NewFlagsCmd() + flagsCmd, err := flagscmd.NewFlagsCmd(flagsClient) if err != nil { return nil, err } @@ -72,7 +73,7 @@ func NewRootCommand(client projects.Client) (*cobra.Command, error) { } func Execute() { - rootCmd, err := NewRootCommand(projects.NewClient()) + rootCmd, err := NewRootCommand(flags.NewClient(), projects.NewClient()) if err != nil { log.Fatal(err) } diff --git a/internal/flags/flags.go b/internal/flags/flags.go index 5a44b526..b9f8587d 100644 --- a/internal/flags/flags.go +++ b/internal/flags/flags.go @@ -10,33 +10,36 @@ import ( ) type Client interface { - Create(ctx context.Context, name string, key string, projectKey string) ([]byte, error) - Update(ctx context.Context, key string, projKey string) ([]byte, error) + Create(ctx context.Context, accessToken, baseURI, name, key, projKey string) ([]byte, error) + Update( + ctx context.Context, + accessToken, + baseURI, + key, + projKey string, + patch []ldapi.PatchOperation, + ) ([]byte, error) } -type FlagsClient struct { - client *ldapi.APIClient -} +type FlagsClient struct{} -func NewClient(accessToken string, baseURI string) FlagsClient { - config := ldapi.NewConfiguration() - config.AddDefaultHeader("Authorization", accessToken) - config.Servers[0].URL = baseURI - client := ldapi.NewAPIClient(config) +var _ Client = FlagsClient{} - return FlagsClient{ - client: client, - } +func NewClient() FlagsClient { + return FlagsClient{} } func (c FlagsClient) Create( ctx context.Context, - name string, - key string, + accessToken, + baseURI, + name, + key, projectKey string, ) ([]byte, error) { + client := c.client(accessToken, baseURI) post := ldapi.NewFeatureFlagBody(name, key) - flag, _, err := c.client.FeatureFlagsApi.PostFeatureFlag(ctx, projectKey).FeatureFlagBody(*post).Execute() + flag, _, err := client.FeatureFlagsApi.PostFeatureFlag(ctx, projectKey).FeatureFlagBody(*post).Execute() if err != nil { return nil, errors.NewAPIError(err) @@ -52,11 +55,14 @@ func (c FlagsClient) Create( func (c FlagsClient) Update( ctx context.Context, - key string, + accessToken, + baseURI, + key, projKey string, patch []ldapi.PatchOperation, ) ([]byte, error) { - flag, _, err := c.client.FeatureFlagsApi. + client := c.client(accessToken, baseURI) + flag, _, err := client.FeatureFlagsApi. PatchFeatureFlag(ctx, projKey, key). PatchWithComment(*ldapi.NewPatchWithComment(patch)). Execute() @@ -71,3 +77,14 @@ func (c FlagsClient) Update( return responseJSON, nil } + +// client creates an LD API client. It's not set as a field on the struct because the CLI flags +// are evaluated when running the command, not when executing the program. That means we don't have +// the flag values until the command's RunE method is called. +func (c FlagsClient) client(accessToken string, baseURI string) *ldapi.APIClient { + config := ldapi.NewConfiguration() + config.AddDefaultHeader("Authorization", accessToken) + config.Servers[0].URL = baseURI + + return ldapi.NewAPIClient(config) +} diff --git a/internal/flags/mock.go b/internal/flags/mock.go new file mode 100644 index 00000000..af330cb1 --- /dev/null +++ b/internal/flags/mock.go @@ -0,0 +1,41 @@ +package flags + +import ( + "context" + + ldapi "github.com/launchdarkly/api-client-go/v14" + + "github.com/stretchr/testify/mock" +) + +type MockClient struct { + mock.Mock +} + +var _ Client = &MockClient{} + +func (c *MockClient) Create( + ctx context.Context, + accessToken, + baseURI, + name, + key, + projKey string, +) ([]byte, error) { + args := c.Called(accessToken, baseURI, name, key, projKey) + + return args.Get(0).([]byte), args.Error(1) +} + +func (c *MockClient) Update( + ctx context.Context, + accessToken, + baseURI, + key, + projKey string, + patch []ldapi.PatchOperation, +) ([]byte, error) { + args := c.Called(accessToken, baseURI, projKey, key, patch) + + return args.Get(0).([]byte), args.Error(1) +} diff --git a/internal/projects/mock.go b/internal/projects/mock.go index 3f50b251..c8fa59a5 100644 --- a/internal/projects/mock.go +++ b/internal/projects/mock.go @@ -8,8 +8,6 @@ import ( type MockClient struct { mock.Mock - AccessToken string - BaseURI string } var _ Client = &MockClient{}