From d9f0a9615ea2e8d6f86eb8f2fd405782dbf1b9b6 Mon Sep 17 00:00:00 2001 From: Efe Karakus Date: Mon, 23 May 2022 14:27:59 -0700 Subject: [PATCH 1/6] chore: update "env show" validation for opt flags --- internal/pkg/cli/env_show.go | 48 +++++++++----- internal/pkg/cli/env_show_test.go | 107 ++++++++++++------------------ internal/pkg/cli/flag.go | 4 ++ 3 files changed, 76 insertions(+), 83 deletions(-) diff --git a/internal/pkg/cli/env_show.go b/internal/pkg/cli/env_show.go index bd7bb827990..a01046ed5b8 100644 --- a/internal/pkg/cli/env_show.go +++ b/internal/pkg/cli/env_show.go @@ -34,6 +34,7 @@ type showEnvVars struct { name string shouldOutputJSON bool shouldOutputResources bool + shouldOutputManifest bool } type showEnvOpts struct { @@ -82,28 +83,23 @@ func newShowEnvOpts(vars showEnvVars) (*showEnvOpts, error) { return opts, nil } -// Validate returns an error if the values provided by the user are invalid. +// Validate returns an error if any optional flags are invalid. func (o *showEnvOpts) Validate() error { - if o.appName == "" { - return nil + if o.shouldOutputManifest && o.shouldOutputResources { + return fmt.Errorf("--%s and --%s cannot be specified together", manifestFlag, resourcesFlag) } - if _, err := o.store.GetApplication(o.appName); err != nil { - return err - } - if o.name != "" { - if _, err := o.store.GetEnvironment(o.appName, o.name); err != nil { - return err - } + if o.shouldOutputManifest && o.shouldOutputJSON { + return fmt.Errorf("--%s and --%s cannot be specified together", manifestFlag, jsonFlag) } return nil } -// Ask asks for fields that are required but not passed in. +// Ask validates required fields that users passed in, otherwise it prompts for them. func (o *showEnvOpts) Ask() error { - if err := o.askApp(); err != nil { + if err := o.validateOrAskApp(); err != nil { return err } - return o.askEnvName() + return o.validateOrAskEnv() } // Execute shows the environments through the prompt. @@ -128,9 +124,9 @@ func (o *showEnvOpts) Execute() error { return nil } -func (o *showEnvOpts) askApp() error { +func (o *showEnvOpts) validateOrAskApp() error { if o.appName != "" { - return nil + return o.validateApp() } app, err := o.sel.Application(envShowAppNamePrompt, envShowAppNameHelpPrompt) if err != nil { @@ -140,17 +136,31 @@ func (o *showEnvOpts) askApp() error { return nil } -func (o *showEnvOpts) askEnvName() error { - //return if env name is set by flag +func (o *showEnvOpts) validateApp() error { + _, err := o.store.GetApplication(o.appName) + if err != nil { + return fmt.Errorf("validate application name '%s': %v", o.appName, err) + } + return nil +} + +func (o *showEnvOpts) validateOrAskEnv() error { if o.name != "" { - return nil + return o.validateEnv() } env, err := o.sel.Environment(fmt.Sprintf(envShowNamePrompt, color.HighlightUserInput(o.appName)), envShowHelpPrompt, o.appName) if err != nil { return fmt.Errorf("select environment for application %s: %w", o.appName, err) } o.name = env + return nil +} +func (o *showEnvOpts) validateEnv() error { + _, err := o.store.GetEnvironment(o.appName, o.name) + if err != nil { + return fmt.Errorf("validate environment name '%s' in application '%s': %v", o.name, o.appName, err) + } return nil } @@ -177,5 +187,7 @@ func buildEnvShowCmd() *cobra.Command { cmd.Flags().StringVarP(&vars.name, nameFlag, nameFlagShort, "", envFlagDescription) cmd.Flags().BoolVar(&vars.shouldOutputJSON, jsonFlag, false, jsonFlagDescription) cmd.Flags().BoolVar(&vars.shouldOutputResources, resourcesFlag, false, envResourcesFlagDescription) + cmd.Flags().BoolVar(&vars.shouldOutputManifest, manifestFlag, false, manifestFlagDescription) + _ = cmd.Flags().MarkHidden(manifestFlag) return cmd } diff --git a/internal/pkg/cli/env_show_test.go b/internal/pkg/cli/env_show_test.go index 009af208c17..213419669f6 100644 --- a/internal/pkg/cli/env_show_test.go +++ b/internal/pkg/cli/env_show_test.go @@ -28,80 +28,32 @@ type showEnvMocks struct { func TestEnvShow_Validate(t *testing.T) { testCases := map[string]struct { - inputApp string - inputEnvironment string - setupMocks func(mocks showEnvMocks) + inVars showEnvVars wantedError error }{ - "skip validation is app flag is not set": { - inputEnvironment: "my-env", - - setupMocks: func(m showEnvMocks) {}, - }, - "valid app name and environment name": { - inputApp: "my-app", - inputEnvironment: "my-env", - - setupMocks: func(m showEnvMocks) { - gomock.InOrder( - m.storeSvc.EXPECT().GetApplication("my-app").Return(&config.Application{ - Name: "my-app", - }, nil), - m.storeSvc.EXPECT().GetEnvironment("my-app", "my-env").Return(&config.Environment{ - Name: "my-env", - }, nil), - ) + "should error when --manifest and --json are used together": { + inVars: showEnvVars{ + shouldOutputManifest: true, + shouldOutputJSON: true, }, - - wantedError: nil, + wantedError: errors.New("--manifest and --json cannot be specified together"), }, - "invalid app name": { - inputApp: "my-app", - inputEnvironment: "my-env", - - setupMocks: func(m showEnvMocks) { - m.storeSvc.EXPECT().GetApplication("my-app").Return(nil, errors.New("some error")) + "should error when --manifest and --resources are used together": { + inVars: showEnvVars{ + shouldOutputManifest: true, + shouldOutputResources: true, }, - - wantedError: fmt.Errorf("some error"), - }, - "invalid environment name": { - inputApp: "my-app", - inputEnvironment: "my-env", - - setupMocks: func(m showEnvMocks) { - gomock.InOrder( - m.storeSvc.EXPECT().GetApplication("my-app").Return(&config.Application{ - Name: "my-app", - }, nil), - m.storeSvc.EXPECT().GetEnvironment("my-app", "my-env").Return(nil, errors.New("some error")), - ) - }, - - wantedError: fmt.Errorf("some error"), + wantedError: errors.New("--manifest and --resources cannot be specified together"), }, + "should not error by default": {}, } for name, tc := range testCases { t.Run(name, func(t *testing.T) { - ctrl := gomock.NewController(t) - defer ctrl.Finish() - - mockStoreReader := mocks.NewMockstore(ctrl) - - mocks := showEnvMocks{ - storeSvc: mockStoreReader, - } - - tc.setupMocks(mocks) showEnvs := &showEnvOpts{ - showEnvVars: showEnvVars{ - name: tc.inputEnvironment, - appName: tc.inputApp, - }, - store: mockStoreReader, + showEnvVars: tc.inVars, } // WHEN @@ -129,15 +81,36 @@ func TestEnvShow_Ask(t *testing.T) { wantedEnv string wantedError error }{ - "with all flags": { + "ensure resources are registered in SSM if users passes flags": { inputApp: "my-app", inputEnv: "my-env", - setupMocks: func(mocks showEnvMocks) {}, + setupMocks: func(m showEnvMocks) { + m.storeSvc.EXPECT().GetApplication("my-app").Return(nil, nil) + m.storeSvc.EXPECT().GetEnvironment("my-app", "my-env").Return(nil, nil) + }, wantedApp: "my-app", wantedEnv: "my-env", }, + "should wrap error if validation of app name fails": { + inputApp: "my-app", + + setupMocks: func(m showEnvMocks) { + m.storeSvc.EXPECT().GetApplication("my-app").Return(nil, mockErr) + }, + wantedError: errors.New("validate application name 'my-app': some error"), + }, + "should wrap error if validation of env name fails": { + inputApp: "my-app", + inputEnv: "my-env", + + setupMocks: func(m showEnvMocks) { + m.storeSvc.EXPECT().GetApplication("my-app").Return(nil, nil) + m.storeSvc.EXPECT().GetEnvironment("my-app", "my-env").Return(nil, mockErr) + }, + wantedError: errors.New("validate environment name 'my-env' in application 'my-app': some error"), + }, "returns error when fail to select app": { inputApp: "", inputEnv: "", @@ -153,6 +126,7 @@ func TestEnvShow_Ask(t *testing.T) { inputEnv: "", setupMocks: func(m showEnvMocks) { + m.storeSvc.EXPECT().GetApplication("my-app").Return(nil, nil) m.sel.EXPECT().Environment(fmt.Sprintf(envShowNamePrompt, color.HighlightUserInput("my-app")), envShowHelpPrompt, "my-app").Return("", mockErr) }, @@ -180,9 +154,11 @@ func TestEnvShow_Ask(t *testing.T) { defer ctrl.Finish() mockSelector := mocks.NewMockconfigSelector(ctrl) + mockStore := mocks.NewMockstore(ctrl) mocks := showEnvMocks{ - sel: mockSelector, + sel: mockSelector, + storeSvc: mockStore, } tc.setupMocks(mocks) @@ -192,7 +168,8 @@ func TestEnvShow_Ask(t *testing.T) { name: tc.inputEnv, appName: tc.inputApp, }, - sel: mockSelector, + sel: mockSelector, + store: mockStore, } // WHEN err := showEnvs.Ask() diff --git a/internal/pkg/cli/flag.go b/internal/pkg/cli/flag.go index 99d7d422dcb..7ff931f94f3 100644 --- a/internal/pkg/cli/flag.go +++ b/internal/pkg/cli/flag.go @@ -8,6 +8,7 @@ import ( "strings" "github.com/aws/copilot-cli/internal/pkg/manifest" + "github.com/aws/copilot-cli/internal/pkg/template" ) @@ -27,6 +28,8 @@ const ( allFlag = "all" forceFlag = "force" noRollbackFlag = "no-rollback" + manifestFlag = "manifest" + // Command specific flags. dockerFileFlag = "dockerfile" dockerFileContextFlag = "build-context" @@ -204,6 +207,7 @@ const ( rollback in case of deployment failure. We do not recommend using this flag for a production environment.` + manifestFlagDescription = "Optional. Output the manifest file used for the deployment." imageTagFlagDescription = `Optional. The container image tag.` resourceTagsFlagDescription = `Optional. Labels with a key and value separated by commas. From 2c6d8ec76cd0a5be0464d655d9c32a6db36060d4 Mon Sep 17 00:00:00 2001 From: Efe Karakus Date: Mon, 23 May 2022 15:27:12 -0700 Subject: [PATCH 2/6] chore: add hidden --manifest flag to env show --- internal/pkg/cli/env_show.go | 20 ++++- internal/pkg/cli/env_show_test.go | 132 +++++++++++++++--------------- 2 files changed, 80 insertions(+), 72 deletions(-) diff --git a/internal/pkg/cli/env_show.go b/internal/pkg/cli/env_show.go index a01046ed5b8..03f8e413fc7 100644 --- a/internal/pkg/cli/env_show.go +++ b/internal/pkg/cli/env_show.go @@ -107,20 +107,23 @@ func (o *showEnvOpts) Execute() error { if err := o.initEnvDescriber(); err != nil { return err } + if o.shouldOutputManifest { + return o.writeManifest() + } + env, err := o.describer.Describe() if err != nil { return fmt.Errorf("describe environment %s: %w", o.name, err) } + content := env.HumanString() if o.shouldOutputJSON { data, err := env.JSONString() if err != nil { return err } - fmt.Fprint(o.w, data) - } else { - fmt.Fprint(o.w, env.HumanString()) + content = data } - + fmt.Fprint(o.w, content) return nil } @@ -164,6 +167,15 @@ func (o *showEnvOpts) validateEnv() error { return nil } +func (o *showEnvOpts) writeManifest() error { + out, err := o.describer.Manifest() + if err != nil { + return fmt.Errorf("fetch manifest for environment %s: %v", o.name, err) + } + fmt.Fprintln(o.w, string(out)) + return nil +} + // buildEnvShowCmd builds the command for showing environments in an application. func buildEnvShowCmd() *cobra.Command { vars := showEnvVars{} diff --git a/internal/pkg/cli/env_show_test.go b/internal/pkg/cli/env_show_test.go index 213419669f6..46c1035b756 100644 --- a/internal/pkg/cli/env_show_test.go +++ b/internal/pkg/cli/env_show_test.go @@ -187,63 +187,57 @@ func TestEnvShow_Ask(t *testing.T) { func TestEnvShow_Execute(t *testing.T) { mockError := errors.New("some error") - testEnv := &config.Environment{ - App: "testApp", - Name: "testEnv", - Region: "us-west-2", - AccountID: "123456789012", - Prod: false, - RegistryURL: "", - ExecutionRoleARN: "", - ManagerRoleARN: "", - } - testSvc1 := &config.Workload{ - App: "testApp", - Name: "testSvc1", - Type: "load-balanced", - } - testSvc2 := &config.Workload{ - App: "testApp", - Name: "testSvc2", - Type: "load-balanced", - } - testSvc3 := &config.Workload{ - App: "testApp", - Name: "testSvc3", - Type: "load-balanced", - } - testJob1 := &config.Workload{ - App: "testApp", - Name: "testJob1", - Type: "Scheduled Job", - } - testJob2 := &config.Workload{ - App: "testApp", - Name: "testJob2", - Type: "Scheduled Job", - } - var wantedResources = []*stack.Resource{ - { - Type: "AWS::IAM::Role", - PhysicalID: "testApp-testEnv-CFNExecutionRole", + mockEnvDescription := describe.EnvDescription{ + Environment: &config.Environment{ + App: "testApp", + Name: "testEnv", + Region: "us-west-2", + AccountID: "123456789012", + Prod: false, + RegistryURL: "", + ExecutionRoleARN: "", + ManagerRoleARN: "", }, - { - Type: "testApp-testEnv-Cluster", - PhysicalID: "AWS::ECS::Cluster-jI63pYBWU6BZ", + Services: []*config.Workload{ + { + App: "testApp", + Name: "testSvc1", + Type: "load-balanced", + }, { + App: "testApp", + Name: "testSvc2", + Type: "load-balanced", + }, { + App: "testApp", + Name: "testSvc3", + Type: "load-balanced", + }}, + Jobs: []*config.Workload{{ + App: "testApp", + Name: "testJob1", + Type: "Scheduled Job", + }, { + App: "testApp", + Name: "testJob2", + Type: "Scheduled Job", + }}, + Tags: map[string]string{"copilot-application": "testApp", "copilot-environment": "testEnv", "key1": "value1", "key2": "value2"}, + Resources: []*stack.Resource{ + { + Type: "AWS::IAM::Role", + PhysicalID: "testApp-testEnv-CFNExecutionRole", + }, + { + Type: "testApp-testEnv-Cluster", + PhysicalID: "AWS::ECS::Cluster-jI63pYBWU6BZ", + }, }, } - mockTags := map[string]string{"copilot-application": "testApp", "copilot-environment": "testEnv", "key1": "value1", "key2": "value2"} - mockEnvDescription := describe.EnvDescription{ - Environment: testEnv, - Services: []*config.Workload{testSvc1, testSvc2, testSvc3}, - Jobs: []*config.Workload{testJob1, testJob2}, - Tags: mockTags, - Resources: wantedResources, - } testCases := map[string]struct { - inputEnv string - shouldOutputJSON bool + inputEnv string + shouldOutputJSON bool + shouldOutputManifest bool setupMocks func(mocks showEnvMocks) @@ -253,9 +247,7 @@ func TestEnvShow_Execute(t *testing.T) { "return error if fail to describe the env": { inputEnv: "testEnv", setupMocks: func(m showEnvMocks) { - gomock.InOrder( - m.describer.EXPECT().Describe().Return(nil, mockError), - ) + m.describer.EXPECT().Describe().Return(nil, mockError) }, wantedError: fmt.Errorf("describe environment testEnv: some error"), @@ -264,19 +256,15 @@ func TestEnvShow_Execute(t *testing.T) { inputEnv: "testEnv", shouldOutputJSON: true, setupMocks: func(m showEnvMocks) { - gomock.InOrder( - m.describer.EXPECT().Describe().Return(&mockEnvDescription, mockError), - ) + m.describer.EXPECT().Describe().Return(&mockEnvDescription, mockError) }, wantedError: fmt.Errorf("describe environment testEnv: some error"), }, - "success in human format": { + "should print human format": { inputEnv: "testEnv", setupMocks: func(m showEnvMocks) { - gomock.InOrder( - m.describer.EXPECT().Describe().Return(&mockEnvDescription, nil), - ) + m.describer.EXPECT().Describe().Return(&mockEnvDescription, nil) }, wantedContent: `About @@ -311,17 +299,24 @@ Resources testApp-testEnv-Cluster AWS::ECS::Cluster-jI63pYBWU6BZ `, }, - "success in JSON format": { + "should print JSON format": { inputEnv: "testEnv", shouldOutputJSON: true, setupMocks: func(m showEnvMocks) { - gomock.InOrder( - m.describer.EXPECT().Describe().Return(&mockEnvDescription, nil), - ) + m.describer.EXPECT().Describe().Return(&mockEnvDescription, nil) }, wantedContent: "{\"environment\":{\"app\":\"testApp\",\"name\":\"testEnv\",\"region\":\"us-west-2\",\"accountID\":\"123456789012\",\"prod\":false,\"registryURL\":\"\",\"executionRoleARN\":\"\",\"managerRoleARN\":\"\"},\"services\":[{\"app\":\"testApp\",\"name\":\"testSvc1\",\"type\":\"load-balanced\"},{\"app\":\"testApp\",\"name\":\"testSvc2\",\"type\":\"load-balanced\"},{\"app\":\"testApp\",\"name\":\"testSvc3\",\"type\":\"load-balanced\"}],\"jobs\":[{\"app\":\"testApp\",\"name\":\"testJob1\",\"type\":\"Scheduled Job\"},{\"app\":\"testApp\",\"name\":\"testJob2\",\"type\":\"Scheduled Job\"}],\"tags\":{\"copilot-application\":\"testApp\",\"copilot-environment\":\"testEnv\",\"key1\":\"value1\",\"key2\":\"value2\"},\"resources\":[{\"type\":\"AWS::IAM::Role\",\"physicalID\":\"testApp-testEnv-CFNExecutionRole\"},{\"type\":\"testApp-testEnv-Cluster\",\"physicalID\":\"AWS::ECS::Cluster-jI63pYBWU6BZ\"}],\"environmentVPC\":{\"id\":\"\",\"publicSubnetIDs\":null,\"privateSubnetIDs\":null}}\n", }, + "should print manifest file": { + inputEnv: "testEnv", + shouldOutputManifest: true, + setupMocks: func(m showEnvMocks) { + m.describer.EXPECT().Manifest().Return([]byte("hello"), nil) + }, + + wantedContent: "hello\n", + }, } for name, tc := range testCases { @@ -341,8 +336,9 @@ Resources showEnvs := &showEnvOpts{ showEnvVars: showEnvVars{ - name: tc.inputEnv, - shouldOutputJSON: tc.shouldOutputJSON, + name: tc.inputEnv, + shouldOutputJSON: tc.shouldOutputJSON, + shouldOutputManifest: tc.shouldOutputManifest, }, store: mockStoreReader, describer: mockEnvDescriber, From 35ebe40e69865f15f2bf744e5d5a633959650b3a Mon Sep 17 00:00:00 2001 From: Efe Karakus Date: Mon, 23 May 2022 16:04:11 -0700 Subject: [PATCH 3/6] test: fix unit tests for new selector mocks that were forgotten --- internal/pkg/term/selector/selector_test.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/internal/pkg/term/selector/selector_test.go b/internal/pkg/term/selector/selector_test.go index 283073c8fb9..562d2b281a7 100644 --- a/internal/pkg/term/selector/selector_test.go +++ b/internal/pkg/term/selector/selector_test.go @@ -1399,10 +1399,10 @@ func TestConfigSelect_Service(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - MockconfigLister := mocks.NewMockconfigLister(ctrl) + mockconfigLister := mocks.NewMockconfigLister(ctrl) mockprompt := mocks.NewMockprompter(ctrl) mocks := configSelectMocks{ - workloadLister: MockconfigLister, + workloadLister: mockconfigLister, prompt: mockprompt, } tc.setupMocks(mocks) @@ -1411,7 +1411,7 @@ func TestConfigSelect_Service(t *testing.T) { AppEnvSelector: &AppEnvSelector{ prompt: mockprompt, }, - workloadLister: MockconfigLister, + workloadLister: mockconfigLister, } got, err := sel.Service("Select a service", "Help text", appName) @@ -1530,10 +1530,10 @@ func TestConfigSelect_Job(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - MockconfigLister := mocks.NewMockconfigLister(ctrl) + mockconfigLister := mocks.NewMockconfigLister(ctrl) mockprompt := mocks.NewMockprompter(ctrl) mocks := configSelectMocks{ - workloadLister: MockconfigLister, + workloadLister: mockconfigLister, prompt: mockprompt, } tc.setupMocks(mocks) @@ -1542,7 +1542,7 @@ func TestConfigSelect_Job(t *testing.T) { AppEnvSelector: &AppEnvSelector{ prompt: mockprompt, }, - workloadLister: MockconfigLister, + workloadLister: mockconfigLister, } got, err := sel.Job("Select a job", "Help text", appName) @@ -2393,17 +2393,17 @@ func TestWorkspaceSelect_WsPipeline(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - MockwsPipelinesLister := mocks.NewMockwsPipelinesLister(ctrl) + mockwsPipelinesLister := mocks.NewMockwsPipelinesLister(ctrl) mockprompt := mocks.NewMockprompter(ctrl) mocks := wsPipelineSelectMocks{ prompt: mockprompt, - ws: MockwsPipelinesLister, + ws: mockwsPipelinesLister, } tc.setupMocks(mocks) sel := WsPipelineSelector{ prompt: mockprompt, - ws: MockwsPipelinesLister, + ws: mockwsPipelinesLister, } got, err := sel.WsPipeline("Select a pipeline", "Help text") if tc.wantedErr != nil { From c484db04a3dbf66b2e3460322cb4b58d0eb9ad8f Mon Sep 17 00:00:00 2001 From: Efe Karakus Date: Mon, 23 May 2022 16:20:10 -0700 Subject: [PATCH 4/6] chore: fix mocks --- internal/pkg/cli/env_deploy_test.go | 6 +++--- internal/pkg/cli/interfaces.go | 1 + internal/pkg/cli/mocks/mock_interfaces.go | 21 ++++++++++++++++++--- 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/internal/pkg/cli/env_deploy_test.go b/internal/pkg/cli/env_deploy_test.go index 1f6ea5c0747..007ef55b4b3 100644 --- a/internal/pkg/cli/env_deploy_test.go +++ b/internal/pkg/cli/env_deploy_test.go @@ -60,7 +60,7 @@ func TestDeployEnvOpts_Ask(t *testing.T) { setUpMocks: func(m *deployEnvAskMocks) { m.store.EXPECT().GetApplication("mockApp").Return(&config.Application{}, nil) m.store.EXPECT().GetEnvironment("mockApp", "mockEnv").AnyTimes() - m.sel.EXPECT().WSEnvironment(gomock.Any(), gomock.Any()).Return("", errors.New("some error")) + m.sel.EXPECT().LocalEnvironment(gomock.Any(), gomock.Any()).Return("", errors.New("some error")) }, wantedError: errors.New("select environment: some error"), }, @@ -70,7 +70,7 @@ func TestDeployEnvOpts_Ask(t *testing.T) { setUpMocks: func(m *deployEnvAskMocks) { m.store.EXPECT().GetApplication("mockApp").Return(&config.Application{}, nil) m.store.EXPECT().GetEnvironment("mockApp", "mockEnv").Return(&config.Environment{}, nil) - m.sel.EXPECT().WSEnvironment(gomock.Any(), gomock.Any()).Times(0) + m.sel.EXPECT().LocalEnvironment(gomock.Any(), gomock.Any()).Times(0) }, wantedEnvName: "mockEnv", }, @@ -79,7 +79,7 @@ func TestDeployEnvOpts_Ask(t *testing.T) { setUpMocks: func(m *deployEnvAskMocks) { m.store.EXPECT().GetApplication("mockApp").Return(&config.Application{}, nil) m.store.EXPECT().GetEnvironment(gomock.Any(), gomock.Any()).Times(0) - m.sel.EXPECT().WSEnvironment(gomock.Any(), gomock.Any()).Return("mockEnv", nil) + m.sel.EXPECT().LocalEnvironment(gomock.Any(), gomock.Any()).Return("mockEnv", nil) }, wantedEnvName: "mockEnv", }, diff --git a/internal/pkg/cli/interfaces.go b/internal/pkg/cli/interfaces.go index 7d080fcdca2..b2bbd85c68a 100644 --- a/internal/pkg/cli/interfaces.go +++ b/internal/pkg/cli/interfaces.go @@ -417,6 +417,7 @@ type statusDescriber interface { type envDescriber interface { Describe() (*describe.EnvDescription, error) PublicCIDRBlocks() ([]string, error) + Manifest() ([]byte, error) } type versionGetter interface { diff --git a/internal/pkg/cli/mocks/mock_interfaces.go b/internal/pkg/cli/mocks/mock_interfaces.go index a4e97a60e43..95f4b0a0f1f 100644 --- a/internal/pkg/cli/mocks/mock_interfaces.go +++ b/internal/pkg/cli/mocks/mock_interfaces.go @@ -4429,6 +4429,21 @@ func (mr *MockenvDescriberMockRecorder) Describe() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Describe", reflect.TypeOf((*MockenvDescriber)(nil).Describe)) } +// Manifest mocks base method. +func (m *MockenvDescriber) Manifest() ([]byte, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Manifest") + ret0, _ := ret[0].([]byte) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Manifest indicates an expected call of Manifest. +func (mr *MockenvDescriberMockRecorder) Manifest() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Manifest", reflect.TypeOf((*MockenvDescriber)(nil).Manifest)) +} + // PublicCIDRBlocks mocks base method. func (m *MockenvDescriber) PublicCIDRBlocks() ([]string, error) { m.ctrl.T.Helper() @@ -5247,7 +5262,7 @@ func (m *MockwsEnvironmentSelector) EXPECT() *MockwsEnvironmentSelectorMockRecor return m.recorder } -// WSEnvironment mocks base method. +// LocalEnvironment mocks base method. func (m *MockwsEnvironmentSelector) LocalEnvironment(msg, help string) (string, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "LocalEnvironment", msg, help) @@ -5256,8 +5271,8 @@ func (m *MockwsEnvironmentSelector) LocalEnvironment(msg, help string) (string, return ret0, ret1 } -// WSEnvironment indicates an expected call of WSEnvironment. -func (mr *MockwsEnvironmentSelectorMockRecorder) WSEnvironment(msg, help interface{}) *gomock.Call { +// LocalEnvironment indicates an expected call of LocalEnvironment. +func (mr *MockwsEnvironmentSelectorMockRecorder) LocalEnvironment(msg, help interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "LocalEnvironment", reflect.TypeOf((*MockwsEnvironmentSelector)(nil).LocalEnvironment), msg, help) } From db7890c24d53148ec9a2c10285f623a63644e06c Mon Sep 17 00:00:00 2001 From: Efe Karakus Date: Wed, 25 May 2022 09:59:46 -0700 Subject: [PATCH 5/6] refactor: simplify if-statements --- internal/pkg/cli/env_show.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/internal/pkg/cli/env_show.go b/internal/pkg/cli/env_show.go index 03f8e413fc7..934df22e3ca 100644 --- a/internal/pkg/cli/env_show.go +++ b/internal/pkg/cli/env_show.go @@ -140,8 +140,7 @@ func (o *showEnvOpts) validateOrAskApp() error { } func (o *showEnvOpts) validateApp() error { - _, err := o.store.GetApplication(o.appName) - if err != nil { + if _, err := o.store.GetApplication(o.appName); err != nil { return fmt.Errorf("validate application name '%s': %v", o.appName, err) } return nil @@ -160,8 +159,7 @@ func (o *showEnvOpts) validateOrAskEnv() error { } func (o *showEnvOpts) validateEnv() error { - _, err := o.store.GetEnvironment(o.appName, o.name) - if err != nil { + if _, err := o.store.GetEnvironment(o.appName, o.name); err != nil { return fmt.Errorf("validate environment name '%s' in application '%s': %v", o.name, o.appName, err) } return nil From 9ef8616269c990cde092e850f09babbf1817f77d Mon Sep 17 00:00:00 2001 From: Efe Karakus Date: Wed, 25 May 2022 11:49:39 -0700 Subject: [PATCH 6/6] refactor: use fancy %q fmt verb --- internal/pkg/cli/env_show.go | 4 ++-- internal/pkg/cli/env_show_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/pkg/cli/env_show.go b/internal/pkg/cli/env_show.go index 934df22e3ca..4765ae495be 100644 --- a/internal/pkg/cli/env_show.go +++ b/internal/pkg/cli/env_show.go @@ -141,7 +141,7 @@ func (o *showEnvOpts) validateOrAskApp() error { func (o *showEnvOpts) validateApp() error { if _, err := o.store.GetApplication(o.appName); err != nil { - return fmt.Errorf("validate application name '%s': %v", o.appName, err) + return fmt.Errorf("validate application name %q: %v", o.appName, err) } return nil } @@ -160,7 +160,7 @@ func (o *showEnvOpts) validateOrAskEnv() error { func (o *showEnvOpts) validateEnv() error { if _, err := o.store.GetEnvironment(o.appName, o.name); err != nil { - return fmt.Errorf("validate environment name '%s' in application '%s': %v", o.name, o.appName, err) + return fmt.Errorf("validate environment name %q in application %q: %v", o.name, o.appName, err) } return nil } diff --git a/internal/pkg/cli/env_show_test.go b/internal/pkg/cli/env_show_test.go index 46c1035b756..ac7c22e9dac 100644 --- a/internal/pkg/cli/env_show_test.go +++ b/internal/pkg/cli/env_show_test.go @@ -99,7 +99,7 @@ func TestEnvShow_Ask(t *testing.T) { setupMocks: func(m showEnvMocks) { m.storeSvc.EXPECT().GetApplication("my-app").Return(nil, mockErr) }, - wantedError: errors.New("validate application name 'my-app': some error"), + wantedError: errors.New(`validate application name "my-app": some error`), }, "should wrap error if validation of env name fails": { inputApp: "my-app", @@ -109,7 +109,7 @@ func TestEnvShow_Ask(t *testing.T) { m.storeSvc.EXPECT().GetApplication("my-app").Return(nil, nil) m.storeSvc.EXPECT().GetEnvironment("my-app", "my-env").Return(nil, mockErr) }, - wantedError: errors.New("validate environment name 'my-env' in application 'my-app': some error"), + wantedError: errors.New(`validate environment name "my-env" in application "my-app": some error`), }, "returns error when fail to select app": { inputApp: "",