From 412ed17cc66d876c46ee8df3767066ca0e676f28 Mon Sep 17 00:00:00 2001 From: Ash McKenzie Date: Wed, 11 Sep 2019 23:00:48 +1000 Subject: New console package for writing to the console --- internal/console/console.go | 73 ++++++++++++++ internal/console/console_test.go | 201 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 274 insertions(+) create mode 100644 internal/console/console.go create mode 100644 internal/console/console_test.go diff --git a/internal/console/console.go b/internal/console/console.go new file mode 100644 index 0000000..72f80cc --- /dev/null +++ b/internal/console/console.go @@ -0,0 +1,73 @@ +package console + +import ( + "fmt" + "io" + "strings" +) + +func DisplayWarningMessage(message string, out io.Writer) { + DisplayWarningMessages([]string{message}, out) +} + +func DisplayInfoMessage(message string, out io.Writer) { + DisplayInfoMessages([]string{message}, out) +} + +func DisplayWarningMessages(messages []string, out io.Writer) { + DisplayMessages(messages, out, true) +} + +func DisplayInfoMessages(messages []string, out io.Writer) { + DisplayMessages(messages, out, false) +} + +func DisplayMessages(messages []string, out io.Writer, displayDivider bool) { + if noMessages(messages) { + return + } + + displayBlankLineOrDivider(out, displayDivider) + + for _, msg := range messages { + fmt.Fprintf(out, formatLine(msg)) + } + + displayBlankLineOrDivider(out, displayDivider) +} + +func noMessages(messages []string) bool { + if len(messages) == 0 { + return true + } + + for _, msg := range messages { + if len(strings.TrimSpace(msg)) > 0 { + return false + } + } + + return true +} + +func formatLine(message string) string { + return fmt.Sprintf("remote: %v\n", message) +} + +func displayBlankLineOrDivider(out io.Writer, displayDivider bool) { + if displayDivider { + fmt.Fprintf(out, divider()) + } else { + fmt.Fprintf(out, blankLine()) + } +} + +func blankLine() string { + return formatLine("") +} + +func divider() string { + ruler := strings.Repeat("=", 72) + + return fmt.Sprintf("%v%v%v", blankLine(), formatLine(ruler), blankLine()) +} diff --git a/internal/console/console_test.go b/internal/console/console_test.go new file mode 100644 index 0000000..bcd8444 --- /dev/null +++ b/internal/console/console_test.go @@ -0,0 +1,201 @@ +package console + +import ( + "bytes" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestDisplayWarningMessage(t *testing.T) { + tests := []struct { + name string + message string + wantOut string + }{ + { + name: "empty", + message: "", + wantOut: "", + }, + { + name: "basically empty", + message: " ", + wantOut: "", + }, + { + name: "something", + message: "something", + wantOut: `remote: +remote: ======================================================================== +remote: +remote: something +remote: +remote: ======================================================================== +remote: +`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + out := &bytes.Buffer{} + DisplayWarningMessage(tt.message, out) + + require.Equal(t, tt.wantOut, out.String()) + }) + } +} + +func TestDisplayWarningMessages(t *testing.T) { + tests := []struct { + name string + messages []string + wantOut string + }{ + { + name: "empty", + messages: []string{""}, + wantOut: "", + }, + { + name: "basically empty", + messages: []string{" "}, + wantOut: "", + }, + { + name: "something", + messages: []string{"something", "here"}, + wantOut: `remote: +remote: ======================================================================== +remote: +remote: something +remote: here +remote: +remote: ======================================================================== +remote: +`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + out := &bytes.Buffer{} + DisplayWarningMessages(tt.messages, out) + + require.Equal(t, tt.wantOut, out.String()) + }) + } +} + +func TestDisplayInfoMessage(t *testing.T) { + tests := []struct { + name string + message string + wantOut string + }{ + { + name: "empty", + message: "", + wantOut: "", + }, + { + name: "basically empty", + message: " ", + wantOut: "", + }, + { + name: "something", + message: "something", + wantOut: `remote: +remote: something +remote: +`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + out := &bytes.Buffer{} + DisplayInfoMessage(tt.message, out) + + require.Equal(t, tt.wantOut, out.String()) + }) + } +} + +func TestDisplayInfoMessages(t *testing.T) { + tests := []struct { + name string + messages []string + wantOut string + }{ + { + name: "empty", + messages: []string{""}, + wantOut: "", + }, + { + name: "basically empty", + messages: []string{" "}, + wantOut: "", + }, + { + name: "something", + messages: []string{"something", "here"}, + wantOut: "remote: \nremote: something\nremote: here\nremote: \n", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + out := &bytes.Buffer{} + DisplayInfoMessages(tt.messages, out) + + require.Equal(t, tt.wantOut, out.String()) + }) + } +} + +func Test_noMessages(t *testing.T) { + tests := []struct { + name string + messages []string + want bool + }{ + { + name: "empty", + messages: []string{""}, + want: true, + }, + { + name: "basically empty", + messages: []string{" "}, + want: true, + }, + { + name: "something", + messages: []string{"something", "here"}, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.want, noMessages(tt.messages)) + }) + } +} + +func Test_formatLine(t *testing.T) { + require.Equal(t, "remote: something\n", formatLine("something")) +} + +func Test_divider(t *testing.T) { + want := `remote: +remote: ======================================================================== +remote: +` + + require.Equal(t, want, divider()) +} -- cgit v1.2.1 From 1bade9e198f08437ad150e63dc751edb862a6f51 Mon Sep 17 00:00:00 2001 From: Ash McKenzie Date: Wed, 11 Sep 2019 23:01:40 +1000 Subject: More consistent console messages (golang) --- cmd/gitlab-shell-authorized-keys-check/main.go | 3 ++- cmd/gitlab-shell-authorized-principals-check/main.go | 3 ++- cmd/gitlab-shell/main.go | 3 ++- internal/command/lfsauthenticate/lfsauthenticate_test.go | 4 ++-- internal/command/receivepack/customaction.go | 13 +++---------- internal/command/receivepack/customaction_test.go | 2 +- internal/command/shared/accessverifier/accessverifier.go | 6 ++---- .../command/shared/accessverifier/accessverifier_test.go | 2 +- .../command/shared/disallowedcommand/disallowedcommand.go | 2 +- 9 files changed, 16 insertions(+), 22 deletions(-) diff --git a/cmd/gitlab-shell-authorized-keys-check/main.go b/cmd/gitlab-shell-authorized-keys-check/main.go index 6b52181..8cc0bc8 100644 --- a/cmd/gitlab-shell-authorized-keys-check/main.go +++ b/cmd/gitlab-shell-authorized-keys-check/main.go @@ -7,6 +7,7 @@ import ( "gitlab.com/gitlab-org/gitlab-shell/internal/command" "gitlab.com/gitlab-org/gitlab-shell/internal/command/readwriter" "gitlab.com/gitlab-org/gitlab-shell/internal/config" + "gitlab.com/gitlab-org/gitlab-shell/internal/console" "gitlab.com/gitlab-org/gitlab-shell/internal/executable" ) @@ -38,7 +39,7 @@ func main() { } if err = cmd.Execute(); err != nil { - fmt.Fprintf(readWriter.ErrOut, "%v\n", err) + console.DisplayWarningMessage(err.Error(), readWriter.ErrOut) os.Exit(1) } } diff --git a/cmd/gitlab-shell-authorized-principals-check/main.go b/cmd/gitlab-shell-authorized-principals-check/main.go index 645ccf0..328e11f 100644 --- a/cmd/gitlab-shell-authorized-principals-check/main.go +++ b/cmd/gitlab-shell-authorized-principals-check/main.go @@ -7,6 +7,7 @@ import ( "gitlab.com/gitlab-org/gitlab-shell/internal/command" "gitlab.com/gitlab-org/gitlab-shell/internal/command/readwriter" "gitlab.com/gitlab-org/gitlab-shell/internal/config" + "gitlab.com/gitlab-org/gitlab-shell/internal/console" "gitlab.com/gitlab-org/gitlab-shell/internal/executable" ) @@ -38,7 +39,7 @@ func main() { } if err = cmd.Execute(); err != nil { - fmt.Fprintf(readWriter.ErrOut, "%v\n", err) + console.DisplayWarningMessage(err.Error(), readWriter.ErrOut) os.Exit(1) } } diff --git a/cmd/gitlab-shell/main.go b/cmd/gitlab-shell/main.go index 148c652..7751e4d 100644 --- a/cmd/gitlab-shell/main.go +++ b/cmd/gitlab-shell/main.go @@ -7,6 +7,7 @@ import ( "gitlab.com/gitlab-org/gitlab-shell/internal/command" "gitlab.com/gitlab-org/gitlab-shell/internal/command/readwriter" "gitlab.com/gitlab-org/gitlab-shell/internal/config" + "gitlab.com/gitlab-org/gitlab-shell/internal/console" "gitlab.com/gitlab-org/gitlab-shell/internal/executable" ) @@ -38,7 +39,7 @@ func main() { } if err = cmd.Execute(); err != nil { - fmt.Fprintf(readWriter.ErrOut, "%v\n", err) + console.DisplayWarningMessage(err.Error(), readWriter.ErrOut) os.Exit(1) } } diff --git a/internal/command/lfsauthenticate/lfsauthenticate_test.go b/internal/command/lfsauthenticate/lfsauthenticate_test.go index f2ccc20..22e151a 100644 --- a/internal/command/lfsauthenticate/lfsauthenticate_test.go +++ b/internal/command/lfsauthenticate/lfsauthenticate_test.go @@ -31,12 +31,12 @@ func TestFailedRequests(t *testing.T) { { desc: "With missing arguments", arguments: &commandargs.Shell{}, - expectedOutput: "> GitLab: Disallowed command", + expectedOutput: "Disallowed command", }, { desc: "With disallowed command", arguments: &commandargs.Shell{GitlabKeyId: "1", SshArgs: []string{"git-lfs-authenticate", "group/repo", "unknown"}}, - expectedOutput: "> GitLab: Disallowed command", + expectedOutput: "Disallowed command", }, { desc: "With disallowed user", diff --git a/internal/command/receivepack/customaction.go b/internal/command/receivepack/customaction.go index c94ae4c..7575ee9 100644 --- a/internal/command/receivepack/customaction.go +++ b/internal/command/receivepack/customaction.go @@ -3,12 +3,13 @@ package receivepack import ( "bytes" "errors" - "fmt" + "io" "io/ioutil" "net/http" "strings" + "gitlab.com/gitlab-org/gitlab-shell/internal/console" "gitlab.com/gitlab-org/gitlab-shell/internal/gitlabnet" "gitlab.com/gitlab-org/gitlab-shell/internal/gitlabnet/accessverifier" ) @@ -32,19 +33,11 @@ func (c *Command) processCustomAction(response *accessverifier.Response) error { return errors.New("Custom action error: Empty API endpoints") } - c.displayInfoMessage(data.InfoMessage) + console.DisplayInfoMessages(strings.Split(data.InfoMessage, "\n"), c.ReadWriter.ErrOut) return c.processApiEndpoints(response) } -func (c *Command) displayInfoMessage(infoMessage string) { - messages := strings.Split(infoMessage, "\n") - - for _, msg := range messages { - fmt.Fprintf(c.ReadWriter.ErrOut, "> GitLab: %v\n", msg) - } -} - func (c *Command) processApiEndpoints(response *accessverifier.Response) error { client, err := gitlabnet.GetClient(c.Config) diff --git a/internal/command/receivepack/customaction_test.go b/internal/command/receivepack/customaction_test.go index 2a4a718..11e7dce 100644 --- a/internal/command/receivepack/customaction_test.go +++ b/internal/command/receivepack/customaction_test.go @@ -100,6 +100,6 @@ func TestCustomReceivePack(t *testing.T) { // expect printing of info message, "custom" string from the first request // and "output" string from the second request - require.Equal(t, "> GitLab: info_message\n> GitLab: one more message\n", errBuf.String()) + require.Equal(t, "remote: \nremote: info_message\nremote: one more message\nremote: \n", errBuf.String()) require.Equal(t, "customoutput", outBuf.String()) } diff --git a/internal/command/shared/accessverifier/accessverifier.go b/internal/command/shared/accessverifier/accessverifier.go index 3aaf98d..5d2d709 100644 --- a/internal/command/shared/accessverifier/accessverifier.go +++ b/internal/command/shared/accessverifier/accessverifier.go @@ -2,11 +2,11 @@ package accessverifier import ( "errors" - "fmt" "gitlab.com/gitlab-org/gitlab-shell/internal/command/commandargs" "gitlab.com/gitlab-org/gitlab-shell/internal/command/readwriter" "gitlab.com/gitlab-org/gitlab-shell/internal/config" + "gitlab.com/gitlab-org/gitlab-shell/internal/console" "gitlab.com/gitlab-org/gitlab-shell/internal/gitlabnet/accessverifier" ) @@ -39,7 +39,5 @@ func (c *Command) Verify(action commandargs.CommandType, repo string) (*Response } func (c *Command) displayConsoleMessages(messages []string) { - for _, msg := range messages { - fmt.Fprintf(c.ReadWriter.ErrOut, "> GitLab: %v\n", msg) - } + console.DisplayInfoMessages(messages, c.ReadWriter.ErrOut) } diff --git a/internal/command/shared/accessverifier/accessverifier_test.go b/internal/command/shared/accessverifier/accessverifier_test.go index 39c2a66..cfcf4a8 100644 --- a/internal/command/shared/accessverifier/accessverifier_test.go +++ b/internal/command/shared/accessverifier/accessverifier_test.go @@ -77,6 +77,6 @@ func TestConsoleMessages(t *testing.T) { cmd.Args = &commandargs.Shell{GitlabKeyId: "1"} cmd.Verify(action, repo) - require.Equal(t, "> GitLab: console\n> GitLab: message\n", errBuf.String()) + require.Equal(t, "remote: \nremote: console\nremote: message\nremote: \n", errBuf.String()) require.Empty(t, outBuf.String()) } diff --git a/internal/command/shared/disallowedcommand/disallowedcommand.go b/internal/command/shared/disallowedcommand/disallowedcommand.go index 3c98bcc..794944f 100644 --- a/internal/command/shared/disallowedcommand/disallowedcommand.go +++ b/internal/command/shared/disallowedcommand/disallowedcommand.go @@ -3,5 +3,5 @@ package disallowedcommand import "errors" var ( - Error = errors.New("> GitLab: Disallowed command") + Error = errors.New("Disallowed command") ) -- cgit v1.2.1 From aa82832f7ba0a6ca7017f019be17810579f9a688 Mon Sep 17 00:00:00 2001 From: Ash McKenzie Date: Fri, 18 Oct 2019 14:30:51 +1100 Subject: More consistent console messages (Ruby) --- spec/gitlab_shell_custom_git_receive_pack_spec.rb | 26 ++++++++++++++++------- spec/gitlab_shell_lfs_authentication_spec.rb | 3 ++- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/spec/gitlab_shell_custom_git_receive_pack_spec.rb b/spec/gitlab_shell_custom_git_receive_pack_spec.rb index 0a57903..4b0fec4 100644 --- a/spec/gitlab_shell_custom_git_receive_pack_spec.rb +++ b/spec/gitlab_shell_custom_git_receive_pack_spec.rb @@ -8,6 +8,7 @@ describe 'Custom bin/gitlab-shell git-receive-pack' do include_context 'gitlab shell' let(:env) { {'SSH_CONNECTION' => 'fake', 'SSH_ORIGINAL_COMMAND' => 'git-receive-pack group/repo' } } + let(:divider) { "remote: ========================================================================\n" } before(:context) do write_config("gitlab_url" => "http+unix://#{CGI.escape(tmp_socket_path)}") @@ -65,21 +66,24 @@ describe 'Custom bin/gitlab-shell git-receive-pack' do end describe 'dialog for performing a custom action' do - let(:inaccessible_error) { "Internal API error (403)\n" } - context 'when API calls perform successfully' do + let(:remote_blank_line) { "remote: \n" } def verify_successful_call!(cmd) Open3.popen3(env, cmd) do |stdin, stdout, stderr| - expect(stderr.gets).to eq("> GitLab: console\n") - expect(stderr.gets).to eq("> GitLab: message\n") + expect(stderr.gets).to eq(remote_blank_line) + expect(stderr.gets).to eq("remote: console\n") + expect(stderr.gets).to eq("remote: message\n") + expect(stderr.gets).to eq(remote_blank_line) - expect(stderr.gets).to eq("> GitLab: info_message\n") - expect(stderr.gets).to eq("> GitLab: another_message\n") - expect(stdout.gets(6)).to eq("custom") + expect(stderr.gets).to eq(remote_blank_line) + expect(stderr.gets).to eq("remote: info_message\n") + expect(stderr.gets).to eq("remote: another_message\n") + expect(stderr.gets).to eq(remote_blank_line) stdin.puts("input") stdin.close + expect(stdout.gets(6)).to eq("custom") expect(stdout.flush.read).to eq("input\n") end end @@ -106,7 +110,13 @@ describe 'Custom bin/gitlab-shell git-receive-pack' do it 'custom action is not performed' do Open3.popen2e(env, cmd) do |stdin, stdout| - expect(stdout.gets).to eq(inaccessible_error) + expect(stdout.gets).to eq("remote: \n") + expect(stdout.gets).to eq(divider) + expect(stdout.gets).to eq("remote: \n") + expect(stdout.gets).to eq("remote: Internal API error (403)\n") + expect(stdout.gets).to eq("remote: \n") + expect(stdout.gets).to eq(divider) + expect(stdout.gets).to eq("remote: \n") end end end diff --git a/spec/gitlab_shell_lfs_authentication_spec.rb b/spec/gitlab_shell_lfs_authentication_spec.rb index d27f50a..437005c 100644 --- a/spec/gitlab_shell_lfs_authentication_spec.rb +++ b/spec/gitlab_shell_lfs_authentication_spec.rb @@ -117,9 +117,10 @@ describe 'bin/gitlab-shell git-lfs-authentication' do let(:env) { {'SSH_CONNECTION' => 'fake', 'SSH_ORIGINAL_COMMAND' => 'git-lfs-authenticate project/repo unknown' } } it 'the command is disallowed' do + divider = "remote: \nremote: ========================================================================\nremote: \n" _, stderr, status = Open3.capture3(env, cmd) - expect(stderr).to eq("> GitLab: Disallowed command\n") + expect(stderr).to eq("#{divider}remote: Disallowed command\n#{divider}") expect(status).not_to be_success end end -- cgit v1.2.1