From 8cb24562482b01266b3c3992bbd55ce373fd06fb Mon Sep 17 00:00:00 2001 From: Ian Campbell Date: Mon, 13 May 2019 16:22:02 +0100 Subject: [PATCH] context: produce consistent output on `context create`. Refactor `RunCreate` slightly so that all three paths always produce the same output, namely the name of the new context of `stdout` (for scripting) and the success log message on `stderr`. Validate by extending the existing unit tests to always check the output is as expected. Signed-off-by: Ian Campbell (cherry picked from commit ff44305c472c966d9afc6034e29147101f5ca3a1) Signed-off-by: Sebastiaan van Stijn --- cli/command/context/create.go | 18 +++++++++++------- cli/command/context/create_test.go | 22 ++++++++++++++++++++-- internal/test/cli.go | 6 ++++++ 3 files changed, 37 insertions(+), 9 deletions(-) diff --git a/cli/command/context/create.go b/cli/command/context/create.go index 73d8c29f18..9d5865a70a 100644 --- a/cli/command/context/create.go +++ b/cli/command/context/create.go @@ -78,13 +78,19 @@ func RunCreate(cli command.Cli, o *CreateOptions) error { if err != nil { return errors.Wrap(err, "unable to parse default-stack-orchestrator") } - if o.From == "" && o.Docker == nil && o.Kubernetes == nil { - return createFromExistingContext(s, cli.CurrentContext(), stackOrchestrator, o) + switch { + case o.From == "" && o.Docker == nil && o.Kubernetes == nil: + err = createFromExistingContext(s, cli.CurrentContext(), stackOrchestrator, o) + case o.From != "": + err = createFromExistingContext(s, o.From, stackOrchestrator, o) + default: + err = createNewContext(o, stackOrchestrator, cli, s) } - if o.From != "" { - return createFromExistingContext(s, o.From, stackOrchestrator, o) + if err == nil { + fmt.Fprintln(cli.Out(), o.Name) + fmt.Fprintf(cli.Err(), "Successfully created context %q\n", o.Name) } - return createNewContext(o, stackOrchestrator, cli, s) + return err } func createNewContext(o *CreateOptions, stackOrchestrator command.Orchestrator, cli command.Cli, s store.Writer) error { @@ -127,8 +133,6 @@ func createNewContext(o *CreateOptions, stackOrchestrator command.Orchestrator, if err := s.ResetTLSMaterial(o.Name, &contextTLSData); err != nil { return err } - fmt.Fprintln(cli.Out(), o.Name) - fmt.Fprintf(cli.Err(), "Successfully created context %q\n", o.Name) return nil } diff --git a/cli/command/context/create_test.go b/cli/command/context/create_test.go index dba73c5b64..013e82d05e 100644 --- a/cli/command/context/create_test.go +++ b/cli/command/context/create_test.go @@ -1,6 +1,7 @@ package context import ( + "fmt" "io/ioutil" "os" "testing" @@ -131,6 +132,11 @@ func TestCreateInvalids(t *testing.T) { } } +func assertContextCreateLogging(t *testing.T, cli *test.FakeCli, n string) { + assert.Equal(t, n+"\n", cli.OutBuffer().String()) + assert.Equal(t, fmt.Sprintf("Successfully created context %q\n", n), cli.ErrBuffer().String()) +} + func TestCreateOrchestratorSwarm(t *testing.T) { cli, cleanup := makeFakeCli(t) defer cleanup() @@ -141,8 +147,7 @@ func TestCreateOrchestratorSwarm(t *testing.T) { Docker: map[string]string{}, }) assert.NilError(t, err) - assert.Equal(t, "test\n", cli.OutBuffer().String()) - assert.Equal(t, "Successfully created context \"test\"\n", cli.ErrBuffer().String()) + assertContextCreateLogging(t, cli, "test") } func TestCreateOrchestratorEmpty(t *testing.T) { @@ -154,6 +159,7 @@ func TestCreateOrchestratorEmpty(t *testing.T) { Docker: map[string]string{}, }) assert.NilError(t, err) + assertContextCreateLogging(t, cli, "test") } func validateTestKubeEndpoint(t *testing.T, s store.Reader, name string) { @@ -189,6 +195,7 @@ func TestCreateOrchestratorAllKubernetesEndpointFromCurrent(t *testing.T) { cli, cleanup := makeFakeCli(t) defer cleanup() createTestContextWithKube(t, cli) + assertContextCreateLogging(t, cli, "test") validateTestKubeEndpoint(t, cli.ContextStore(), "test") } @@ -225,6 +232,7 @@ func TestCreateFromContext(t *testing.T) { defer cleanup() revert := env.Patch(t, "KUBECONFIG", "./testdata/test-kubeconfig") defer revert() + cli.ResetOutputBuffers() assert.NilError(t, RunCreate(cli, &CreateOptions{ Name: "original", Description: "original description", @@ -236,6 +244,9 @@ func TestCreateFromContext(t *testing.T) { }, DefaultStackOrchestrator: "swarm", })) + assertContextCreateLogging(t, cli, "original") + + cli.ResetOutputBuffers() assert.NilError(t, RunCreate(cli, &CreateOptions{ Name: "dummy", Description: "dummy description", @@ -247,11 +258,13 @@ func TestCreateFromContext(t *testing.T) { }, DefaultStackOrchestrator: "swarm", })) + assertContextCreateLogging(t, cli, "dummy") cli.SetCurrentContext("dummy") for _, c := range cases { t.Run(c.name, func(t *testing.T) { + cli.ResetOutputBuffers() err := RunCreate(cli, &CreateOptions{ From: "original", Name: c.name, @@ -261,6 +274,7 @@ func TestCreateFromContext(t *testing.T) { Kubernetes: c.kubernetes, }) assert.NilError(t, err) + assertContextCreateLogging(t, cli, c.name) newContext, err := cli.ContextStore().GetMetadata(c.name) assert.NilError(t, err) newContextTyped, err := command.GetDockerContext(newContext) @@ -308,6 +322,7 @@ func TestCreateFromCurrent(t *testing.T) { defer cleanup() revert := env.Patch(t, "KUBECONFIG", "./testdata/test-kubeconfig") defer revert() + cli.ResetOutputBuffers() assert.NilError(t, RunCreate(cli, &CreateOptions{ Name: "original", Description: "original description", @@ -319,17 +334,20 @@ func TestCreateFromCurrent(t *testing.T) { }, DefaultStackOrchestrator: "swarm", })) + assertContextCreateLogging(t, cli, "original") cli.SetCurrentContext("original") for _, c := range cases { t.Run(c.name, func(t *testing.T) { + cli.ResetOutputBuffers() err := RunCreate(cli, &CreateOptions{ Name: c.name, Description: c.description, DefaultStackOrchestrator: c.orchestrator, }) assert.NilError(t, err) + assertContextCreateLogging(t, cli, c.name) newContext, err := cli.ContextStore().GetMetadata(c.name) assert.NilError(t, err) newContextTyped, err := command.GetDockerContext(newContext) diff --git a/internal/test/cli.go b/internal/test/cli.go index 2321ca4d6b..6b7a329e26 100644 --- a/internal/test/cli.go +++ b/internal/test/cli.go @@ -169,6 +169,12 @@ func (c *FakeCli) ErrBuffer() *bytes.Buffer { return c.err } +// ResetOutputBuffers resets the .OutBuffer() and.ErrBuffer() back to empty +func (c *FakeCli) ResetOutputBuffers() { + c.outBuffer.Reset() + c.err.Reset() +} + // SetNotaryClient sets the internal getter for retrieving a NotaryClient func (c *FakeCli) SetNotaryClient(notaryClientFunc NotaryClientFuncType) { c.notaryClientFunc = notaryClientFunc