Merge pull request #2016 from cli/filter-git-push-output

Filter out unwanted stderr output during git push commands

git/git.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"errors"
 	"fmt"
+	"io"
 	"net/url"
 	"os"
 	"os/exec"
@@ -162,10 +163,10 @@ func CommitBody(sha string) (string, error) {
 }
 
 // Push publishes a git ref to a remote and sets up upstream configuration
-func Push(remote string, ref string) error {
+func Push(remote string, ref string, cmdOut, cmdErr io.Writer) error {
 	pushCmd := GitCommand("push", "--set-upstream", remote, ref)
-	pushCmd.Stdout = os.Stdout
-	pushCmd.Stderr = os.Stderr
+	pushCmd.Stdout = cmdOut
+	pushCmd.Stderr = cmdErr
 	return run.PrepareCmd(pushCmd).Run()
 }
 

pkg/cmd/pr/create/create.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"net/http"
 	"net/url"
+	"regexp"
 	"strings"
 	"time"
 
@@ -426,21 +427,33 @@ func createRun(opts *CreateOptions) error {
 
 	// automatically push the branch if it hasn't been pushed anywhere yet
 	if isPushEnabled {
-		pushTries := 0
-		maxPushTries := 3
-		for {
-			if err := git.Push(headRemote.Name, fmt.Sprintf("HEAD:%s", headBranch)); err != nil {
-				if didForkRepo && pushTries < maxPushTries {
-					pushTries++
-					// first wait 2 seconds after forking, then 4s, then 6s
-					waitSeconds := 2 * pushTries
-					fmt.Fprintf(opts.IO.ErrOut, "waiting %s before retrying...\n", utils.Pluralize(waitSeconds, "second"))
-					time.Sleep(time.Duration(waitSeconds) * time.Second)
-					continue
+		pushBranch := func() error {
+			pushTries := 0
+			maxPushTries := 3
+			for {
+				r := NewRegexpWriter(opts.IO.ErrOut, gitPushRegexp, "")
+				defer r.Flush()
+				cmdErr := r
+				cmdOut := opts.IO.Out
+				if err := git.Push(headRemote.Name, fmt.Sprintf("HEAD:%s", headBranch), cmdOut, cmdErr); err != nil {
+					if didForkRepo && pushTries < maxPushTries {
+						pushTries++
+						// first wait 2 seconds after forking, then 4s, then 6s
+						waitSeconds := 2 * pushTries
+						fmt.Fprintf(opts.IO.ErrOut, "waiting %s before retrying...\n", utils.Pluralize(waitSeconds, "second"))
+						time.Sleep(time.Duration(waitSeconds) * time.Second)
+						continue
+					}
+					return err
 				}
-				return err
+				break
 			}
-			break
+			return nil
+		}
+
+		err := pushBranch()
+		if err != nil {
+			return err
 		}
 	}
 
@@ -561,3 +574,5 @@ func generateCompareURL(r ghrepo.Interface, base, head, title, body string, assi
 	}
 	return url, nil
 }
+
+var gitPushRegexp = regexp.MustCompile("^remote: (Create a pull request.*by visiting|[[:space:]]*https://.*/pull/new/).*\n?$")

pkg/cmd/pr/create/regexp_writer.go

@@ -0,0 +1,64 @@
+package create
+
+import (
+	"bytes"
+	"io"
+	"regexp"
+)
+
+func NewRegexpWriter(out io.Writer, re *regexp.Regexp, repl string) *RegexpWriter {
+	return &RegexpWriter{out: out, re: *re, repl: repl}
+}
+
+type RegexpWriter struct {
+	out  io.Writer
+	re   regexp.Regexp
+	repl string
+	buf  []byte
+}
+
+func (s *RegexpWriter) Write(data []byte) (int, error) {
+	if len(data) == 0 {
+		return 0, nil
+	}
+
+	filtered := []byte{}
+	repl := []byte(s.repl)
+	lines := bytes.SplitAfter(data, []byte("\n"))
+
+	if len(s.buf) > 0 {
+		lines[0] = append(s.buf, lines[0]...)
+	}
+
+	for i, line := range lines {
+		if i == len(lines) {
+			s.buf = line
+		} else {
+			f := s.re.ReplaceAll(line, repl)
+			if len(f) > 0 {
+				filtered = append(filtered, f...)
+			}
+		}
+	}
+
+	if len(filtered) != 0 {
+		_, err := s.out.Write(filtered)
+		if err != nil {
+			return 0, err
+		}
+	}
+
+	return len(data), nil
+}
+
+func (s *RegexpWriter) Flush() (int, error) {
+	if len(s.buf) > 0 {
+		repl := []byte(s.repl)
+		filtered := s.re.ReplaceAll(s.buf, repl)
+		if len(filtered) > 0 {
+			return s.out.Write(filtered)
+		}
+	}
+
+	return 0, nil
+}

pkg/cmd/pr/create/regexp_writer_test.go

@@ -0,0 +1,160 @@
+package create
+
+import (
+	"bytes"
+	"regexp"
+	"testing"
+
+	"github.com/MakeNowJust/heredoc"
+	"github.com/stretchr/testify/assert"
+)
+
+func Test_Write(t *testing.T) {
+	type input struct {
+		in   []string
+		re   *regexp.Regexp
+		repl string
+	}
+	type output struct {
+		wantsErr bool
+		out      string
+		length   int
+	}
+	tests := []struct {
+		name   string
+		input  input
+		output output
+	}{
+		{
+			name: "single line input",
+			input: input{
+				in:   []string{"some input line that has wrong information"},
+				re:   regexp.MustCompile("wrong"),
+				repl: "right",
+			},
+			output: output{
+				wantsErr: false,
+				out:      "some input line that has right information",
+				length:   42,
+			},
+		},
+		{
+			name: "multiple line input",
+			input: input{
+				in:   []string{"multiple lines\nin this\ninput lines"},
+				re:   regexp.MustCompile("lines"),
+				repl: "tests",
+			},
+			output: output{
+				wantsErr: false,
+				out:      "multiple tests\nin this\ninput tests",
+				length:   34,
+			},
+		},
+		{
+			name: "no matches",
+			input: input{
+				in:   []string{"this line has no matches"},
+				re:   regexp.MustCompile("wrong"),
+				repl: "right",
+			},
+			output: output{
+				wantsErr: false,
+				out:      "this line has no matches",
+				length:   24,
+			},
+		},
+		{
+			name: "no output",
+			input: input{
+				in:   []string{"remove this whole line"},
+				re:   regexp.MustCompile("^remove.*$"),
+				repl: "",
+			},
+			output: output{
+				wantsErr: false,
+				out:      "",
+				length:   22,
+			},
+		},
+		{
+			name: "no input",
+			input: input{
+				in:   []string{""},
+				re:   regexp.MustCompile("remove"),
+				repl: "",
+			},
+			output: output{
+				wantsErr: false,
+				out:      "",
+				length:   0,
+			},
+		},
+		{
+			name: "multiple lines removed",
+			input: input{
+				in:   []string{"begining line\nremove this whole line\nremove this one also\nnot this one"},
+				re:   regexp.MustCompile("(?s)^remove.*$"),
+				repl: "",
+			},
+			output: output{
+				wantsErr: false,
+				out:      "begining line\nnot this one",
+				length:   70,
+			},
+		},
+		{
+			name: "removes remote from git push output",
+			input: input{
+				in: []string{heredoc.Doc(`
+					output: some information
+					remote:
+					remote: Create a pull request for 'regex' on GitHub by visiting:
+					remote:      https://github.com/owner/repo/pull/new/regex
+					remote:
+					output: more information
+			 `)},
+				re:   regexp.MustCompile("^remote: (Create a pull request.*by visiting|[[:space:]]*https://.*/pull/new/).*\n?$"),
+				repl: "",
+			},
+			output: output{
+				wantsErr: false,
+				out:      "output: some information\nremote:\nremote:\noutput: more information\n",
+				length:   189,
+			},
+		},
+		{
+			name: "multiple writes",
+			input: input{
+				in:   []string{"first write\n", "second write ", "third write"},
+				re:   regexp.MustCompile("write"),
+				repl: "read",
+			},
+			output: output{
+				wantsErr: false,
+				out:      "first read\nsecond read third read",
+				length:   36,
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		out := &bytes.Buffer{}
+		writer := NewRegexpWriter(out, tt.input.re, tt.input.repl)
+		t.Run(tt.name, func(t *testing.T) {
+			length := 0
+			for _, in := range tt.input.in {
+				l, err := writer.Write([]byte(in))
+				length = length + l
+				if tt.output.wantsErr {
+					assert.Error(t, err)
+					return
+				}
+				assert.NoError(t, err)
+			}
+			writer.Flush()
+			assert.Equal(t, tt.output.out, out.String())
+			assert.Equal(t, tt.output.length, length)
+		})
+	}
+}