From 85f242cd451df320d06c1748f4467177a6b572af Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Mon, 21 May 2018 15:26:19 -0700 Subject: [PATCH 1/3] Initialize logging with default info --- ctc_lib/container_tool_command_base.go | 9 +++++++++ ctc_lib/container_tool_command_test.go | 28 ++++++++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/ctc_lib/container_tool_command_base.go b/ctc_lib/container_tool_command_base.go index f18379a2a..44b85db7d 100644 --- a/ctc_lib/container_tool_command_base.go +++ b/ctc_lib/container_tool_command_base.go @@ -24,6 +24,7 @@ import ( "github.com/GoogleCloudPlatform/runtimes-common/ctc_lib/flags" "github.com/GoogleCloudPlatform/runtimes-common/ctc_lib/logging" "github.com/GoogleCloudPlatform/runtimes-common/ctc_lib/types" + log "github.com/sirupsen/logrus" "github.com/spf13/cobra" "github.com/spf13/viper" ) @@ -50,6 +51,14 @@ func (ctb *ContainerToolCommandBase) toolName() string { } func (ctb *ContainerToolCommandBase) Init() { + // Init Logging with info level with colors disabled since initLogging gets called + // only after arguments are parsed correctly. + Log = logging.NewLogger( + viper.GetString(config.LogDirConfigKey), + ctb.Name(), + log.InfoLevel, + false, + ) cobra.OnInitialize(initConfig, ctb.initLogging) ctb.AddFlags() ctb.AddSubCommands() diff --git a/ctc_lib/container_tool_command_test.go b/ctc_lib/container_tool_command_test.go index e607ea567..f20718941 100644 --- a/ctc_lib/container_tool_command_test.go +++ b/ctc_lib/container_tool_command_test.go @@ -263,3 +263,31 @@ func TestContainerToolCommandOutputInJson(t *testing.T) { t.Errorf("Expected to contain: \n %v\nGot:\n %v\n", expectedObj, actualObj) } } + +func TestContainerToolCommandLogNotNull(t *testing.T) { + defer SetExitOnError(true) + testCommand := ContainerToolCommand{ + ContainerToolCommandBase: &ContainerToolCommandBase{ + Command: &cobra.Command{ + Use: "loggingError", + Short: "Logging not nil", + }, + }, + Output: "", + RunO: func(command *cobra.Command, args []string) (interface{}, error) { + return nil, nil + }, + } + SetExitOnError(false) + var OutputBuffer bytes.Buffer + testCommand.Command.SetOutput(&OutputBuffer) + testCommand.SetArgs([]string{"--name=Sparks"}) + err := ExecuteE(&testCommand) + expectedError := ("unknown flag: --name") + if Log == nil { + t.Errorf("Expected Log to be not nil. Got nil") + } + if err.Error() != expectedError { + t.Errorf("Expected Error: \n %q \nGot:\n %q\n", expectedError, err.Error()) + } +} From ec0edaee238dbb8c531d7fae2cc51b0a6cff467e Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Mon, 21 May 2018 16:12:52 -0700 Subject: [PATCH 2/3] Do not display usage on error after flags are parsed --- ctc_lib/container_tool_command_base.go | 12 ++++++----- ctc_lib/container_tool_command_test.go | 30 ++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/ctc_lib/container_tool_command_base.go b/ctc_lib/container_tool_command_base.go index 44b85db7d..0df987142 100644 --- a/ctc_lib/container_tool_command_base.go +++ b/ctc_lib/container_tool_command_base.go @@ -59,11 +59,17 @@ func (ctb *ContainerToolCommandBase) Init() { log.InfoLevel, false, ) - cobra.OnInitialize(initConfig, ctb.initLogging) + cobra.OnInitialize(initConfig, ctb.initLogging, ctb.SetSilenceUsage) ctb.AddFlags() ctb.AddSubCommands() } +func (ctb *ContainerToolCommandBase) SetSilenceUsage() { + // Donot display usage when using RunE after args are parsed. + // See https://github.com/spf13/cobra/issues/340 for more information. + ctb.SilenceUsage = true +} + func (ctb *ContainerToolCommandBase) initLogging() { Log = logging.NewLogger( viper.GetString(config.LogDirConfigKey), @@ -85,10 +91,6 @@ func (ctb *ContainerToolCommandBase) AddSubCommands() { // Set up Root Command ctb.Command.SetHelpTemplate(HelpTemplate) - - // Donot display usage when using RunE. - // See https://github.com/spf13/cobra/issues/340 for more information. - ctb.SilenceUsage = true } func (ctb *ContainerToolCommandBase) AddCommand(command CLIInterface) { diff --git a/ctc_lib/container_tool_command_test.go b/ctc_lib/container_tool_command_test.go index f20718941..43cb86b59 100644 --- a/ctc_lib/container_tool_command_test.go +++ b/ctc_lib/container_tool_command_test.go @@ -23,6 +23,7 @@ import ( "os" "os/exec" "reflect" + "strings" "testing" "github.com/spf13/cobra" @@ -287,6 +288,35 @@ func TestContainerToolCommandLogNotNull(t *testing.T) { if Log == nil { t.Errorf("Expected Log to be not nil. Got nil") } + if !strings.Contains(OutputBuffer.String(), "Usage:") { + t.Error("Expected to contain Usage. However Usage is not displayed") + } + if err.Error() != expectedError { + t.Errorf("Expected Error: \n %q \nGot:\n %q\n", expectedError, err.Error()) + } +} + +func TestContainerToolCommandDoesNotDisplayUsage(t *testing.T) { + defer SetExitOnError(true) + testCommand := ContainerToolCommand{ + ContainerToolCommandBase: &ContainerToolCommandBase{ + Command: &cobra.Command{ + Use: "fail Command", + }, + }, + Output: "", + RunO: func(command *cobra.Command, args []string) (interface{}, error) { + return nil, errors.New("Command failed") + }, + } + SetExitOnError(false) + var OutputBuffer bytes.Buffer + testCommand.Command.SetOutput(&OutputBuffer) + err := ExecuteE(&testCommand) + expectedError := ("Command failed") + if strings.Contains(OutputBuffer.String(), "Usage:") { + t.Error("Expected to not display usage when Command Fails. However Usage is displayed") + } if err.Error() != expectedError { t.Errorf("Expected Error: \n %q \nGot:\n %q\n", expectedError, err.Error()) } From 5c72e0f145011a5ce181eb578ab2db381b26a983 Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Wed, 23 May 2018 11:38:01 -0700 Subject: [PATCH 3/3] fix nit --- ctc_lib/container_tool_command_base.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ctc_lib/container_tool_command_base.go b/ctc_lib/container_tool_command_base.go index 0df987142..5b1f0a9a4 100644 --- a/ctc_lib/container_tool_command_base.go +++ b/ctc_lib/container_tool_command_base.go @@ -65,7 +65,7 @@ func (ctb *ContainerToolCommandBase) Init() { } func (ctb *ContainerToolCommandBase) SetSilenceUsage() { - // Donot display usage when using RunE after args are parsed. + // Do not display usage when using RunE after args are parsed. // See https://github.com/spf13/cobra/issues/340 for more information. ctb.SilenceUsage = true }