Repository: reviewdog/reviewdog
Pull request #405 (2020-01-03 13:31:14)

Add -tee option to debug tools' output
ref: #354


Patch:
diff --git a/.github/workflows/reviewdog.yml b/.github/workflows/reviewdog.yml
index a7c9ca9f..23d0ee37 100644
--- a/.github/workflows/reviewdog.yml
+++ b/.github/workflows/reviewdog.yml
@@ -56,11 +56,11 @@ jobs:
         run: |
           reviewdog -reporter=github-pr-check -runners=golint,govet
 
-      - name: Run reviewdog (github-pr-review)
+      - name: Run reviewdog (github-pr-review with tee)
         env:
           REVIEWDOG_GITHUB_API_TOKEN: ${{ secrets.GITHUB_TOKEN }}
         run: |
-          golint ./... | reviewdog -f=golint -name=golint-pr-review -reporter=github-pr-review
+          golint ./... | reviewdog -f=golint -name=golint-pr-review -reporter=github-pr-review -tee
 
   golangci-lint:
     name: runner / golangci-lint
diff --git a/cmd/reviewdog/doghouse.go b/cmd/reviewdog/doghouse.go
index 8dc02042..59b7e1c9 100644
--- a/cmd/reviewdog/doghouse.go
+++ b/cmd/reviewdog/doghouse.go
@@ -87,7 +87,7 @@ func checkResultSet(ctx context.Context, r io.Reader, opt *option, isProject boo
 		if err != nil {
 			return nil, err
 		}
-		resultSet, err = projectRunAndParse(ctx, conf, buildRunnersMap(opt.runners), opt.level)
+		resultSet, err = projectRunAndParse(ctx, conf, buildRunnersMap(opt.runners), opt.level, opt.tee)
 		if err != nil {
 			return nil, err
 		}
diff --git a/cmd/reviewdog/main.go b/cmd/reviewdog/main.go
index 4fece05b..761e02cf 100644
--- a/cmd/reviewdog/main.go
+++ b/cmd/reviewdog/main.go
@@ -53,6 +53,7 @@ type option struct {
 	reporter         string
 	level            string
 	guessPullRequest bool
+	tee              bool
 }
 
 // flags doc
@@ -68,6 +69,7 @@ const (
 	runnersDoc          = `comma separated runners name to run in config file. default: run all runners`
 	levelDoc            = `report level currently used for github-pr-check reporter ("info","warning","error").`
 	guessPullRequestDoc = `guess Pull Request ID by branch name and commit SHA`
+	teeDoc              = `enable "tee"-like mode which outputs tools's output as is while reporting results to -reporter. Useful for debugging as well.`
 	reporterDoc         = `reporter of reviewdog results. (local, github-check, github-pr-check, github-pr-review, gitlab-mr-discussion, gitlab-mr-commit)
 	"local" (default)
 		Report results to stdout.
@@ -153,6 +155,7 @@ func init() {
 	flag.StringVar(&opt.reporter, "reporter", "local", reporterDoc)
 	flag.StringVar(&opt.level, "level", "error", levelDoc)
 	flag.BoolVar(&opt.guessPullRequest, "guess", false, guessPullRequestDoc)
+	flag.BoolVar(&opt.tee, "tee", false, teeDoc)
 }
 
 func usage() {
@@ -191,6 +194,10 @@ func run(r io.Reader, w io.Writer, opt *option) error {
 See -reporter flag for migration and set -reporter="github-pr-review" or -reporter="github-pr-check" or -reporter="gitlab-mr-commit"`)
 	}
 
+	if opt.tee {
+		r = io.TeeReader(r, w)
+	}
+
 	// assume it's project based run when both -efm and -f are not specified
 	isProject := len(opt.efms) == 0 && opt.f == ""
 
@@ -293,7 +300,7 @@ github-pr-check reporter as a fallback.
 		if err != nil {
 			return err
 		}
-		return project.Run(ctx, conf, buildRunnersMap(opt.runners), cs, ds)
+		return project.Run(ctx, conf, buildRunnersMap(opt.runners), cs, ds, opt.tee)
 	}
 
 	p, err := newParserFromOpt(opt)
diff --git a/project/cmdbuilder.go b/project/cmdbuilder.go
new file mode 100644
index 00000000..0a112dd0
--- /dev/null
+++ b/project/cmdbuilder.go
@@ -0,0 +1,51 @@
+package project
+
+import (
+	"context"
+	"io"
+	"os"
+	"os/exec"
+)
+
+var (
+	defaultTeeStdout io.Writer = os.Stdout
+	defaultTeeStderr io.Writer = os.Stderr
+)
+
+type cmdBuilder struct {
+	envs      []string
+	teeStdout io.Writer
+	teeStderr io.Writer
+	enableTee bool
+}
+
+func newCmdBuilder(envs []string, enableTee bool) *cmdBuilder {
+	return &cmdBuilder{
+		envs:      envs,
+		teeStdout: defaultTeeStdout,
+		teeStderr: defaultTeeStderr,
+		enableTee: enableTee,
+	}
+}
+
+func (cb *cmdBuilder) build(ctx context.Context, command string) (*exec.Cmd, io.Reader, io.Reader, error) {
+	cmd := exec.CommandContext(ctx, "sh", "-c", command)
+	cmd.Env = cb.envs
+	stdout, err := cmd.StdoutPipe()
+	if err != nil {
+		return nil, nil, nil, err
+	}
+	stderr, err := cmd.StderrPipe()
+	if err != nil {
+		return nil, nil, nil, err
+	}
+	var (
+		teeOut io.Reader = stdout
+		teeErr io.Reader = stderr
+	)
+	if cb.enableTee {
+		teeOut = io.TeeReader(stdout, cb.teeStdout)
+		teeErr = io.TeeReader(stderr, cb.teeStderr)
+	}
+	return cmd, teeOut, teeErr, nil
+}
diff --git a/project/run.go b/project/run.go
index 2d892bbf..1610cd92 100644
--- a/project/run.go
+++ b/project/run.go
@@ -7,7 +7,6 @@ import (
 	"io"
 	"log"
 	"os"
-	"os/exec"
 	"runtime"
 	"strings"
 
@@ -18,13 +17,18 @@ import (
 )
 
 // RunAndParse runs commands and parse results. Returns map of tool name to check results.
-func RunAndParse(ctx context.Context, conf *Config, runners map[string]bool, defaultLevel string) (*reviewdog.ResultMap, error) {
+func RunAndParse(ctx context.Context, conf *Config, runners map[string]bool, defaultLevel string, teeMode bool) (*reviewdog.ResultMap, error) {
 	var results reviewdog.ResultMap
 	// environment variables for each commands
 	envs := filteredEnviron()
+	cmdBuilder := newCmdBuilder(envs, teeMode)
 	var usedRunners []string
 	var g errgroup.Group
-	semaphore := make(chan int, runtime.NumCPU())
+	semaphoreNum := runtime.NumCPU()
+	if teeMode {
+		semaphoreNum = 1
+	}
+	semaphore := make(chan int, semaphoreNum)
 	for _, runner := range conf.Runner {
 		runner := runner
 		if len(runners) != 0 && !runners[runner.Name] {
@@ -42,10 +46,7 @@ func RunAndParse(ctx context.Context, conf *Config, runners map[string]bool, def
 		if err != nil {
 			return nil, err
 		}
-		cmd := exec.CommandContext(ctx, "sh", "-c", runner.Cmd)
-		cmd.Env = envs
-		stdout, err := cmd.StdoutPipe()
-		stderr, err := cmd.StderrPipe()
+		cmd, stdout, stderr, err := cmdBuilder.build(ctx, runner.Cmd)
 		if err != nil {
 			return nil, err
 		}
@@ -77,8 +78,8 @@ func RunAndParse(ctx context.Context, conf *Config, runners map[string]bool, def
 }
 
 // Run runs reviewdog tasks based on Config.
-func Run(ctx context.Context, conf *Config, runners map[string]bool, c reviewdog.CommentService, d reviewdog.DiffService) error {
-	results, err := RunAndParse(ctx, conf, runners, "") // Level is not used.
+func Run(ctx context.Context, conf *Config, runners map[string]bool, c reviewdog.CommentService, d reviewdog.DiffService, teeMode bool) error {
+	results, err := RunAndParse(ctx, conf, runners, "", teeMode) // Level is not used.
 	if err != nil {
 		return err
 	}


Test patch:
diff --git a/cmd/reviewdog/doghouse_test.go b/cmd/reviewdog/doghouse_test.go
index ca50d5d3..679b7b01 100644
--- a/cmd/reviewdog/doghouse_test.go
+++ b/cmd/reviewdog/doghouse_test.go
@@ -111,7 +111,7 @@ func TestNewDoghouseServerCli(t *testing.T) {
 }
 
 func TestCheckResultSet_Project(t *testing.T) {
-	defer func(f func(ctx context.Context, conf *project.Config, runners map[string]bool, level string) (*reviewdog.ResultMap, error)) {
+	defer func(f func(ctx context.Context, conf *project.Config, runners map[string]bool, level string, tee bool) (*reviewdog.ResultMap, error)) {
 		projectRunAndParse = f
 	}(projectRunAndParse)
 
@@ -125,7 +125,7 @@ func TestCheckResultSet_Project(t *testing.T) {
 		},
 	}})
 
-	projectRunAndParse = func(ctx context.Context, conf *project.Config, runners map[string]bool, level string) (*reviewdog.ResultMap, error) {
+	projectRunAndParse = func(ctx context.Context, conf *project.Config, runners map[string]bool, level string, tee bool) (*reviewdog.ResultMap, error) {
 		return &wantCheckResult, nil
 	}
 
diff --git a/cmd/reviewdog/main_test.go b/cmd/reviewdog/main_test.go
index b0372ad9..d0925b81 100644
--- a/cmd/reviewdog/main_test.go
+++ b/cmd/reviewdog/main_test.go
@@ -71,6 +71,26 @@ line3
 
 }
 
+func TestRun_local_tee(t *testing.T) {
+	stdin := "tee test"
+	opt := &option{
+		diffCmd:   "git diff",
+		efms:      strslice([]string{`%f(%l,%c): %m`}),
+		diffStrip: 0,
+		reporter:  "local",
+		tee:       true,
+	}
+
+	stdout := new(bytes.Buffer)
+	if err := run(strings.NewReader(stdin), stdout, opt); err != nil {
+		t.Error(err)
+	}
+
+	if got := strings.Trim(stdout.String(), "\n"); got != stdin {
+		t.Errorf("raw: got %v, want %v", got, stdin)
+	}
+}
+
 func TestRun_project(t *testing.T) {
 	t.Run("diff command is empty", func(t *testing.T) {
 		opt := &option{
diff --git a/project/run_test.go b/project/run_test.go
index eaf51063..ba72c3e0 100644
--- a/project/run_test.go
+++ b/project/run_test.go
@@ -1,6 +1,7 @@
 package project
 
 import (
+	"bytes"
 	"context"
 	"errors"
 	"os"
@@ -37,7 +38,7 @@ func TestRun(t *testing.T) {
 
 	t.Run("empty", func(t *testing.T) {
 		conf := &Config{}
-		if err := Run(ctx, conf, nil, nil, nil); err != nil {
+		if err := Run(ctx, conf, nil, nil, nil, false); err != nil {
 			t.Error(err)
 		}
 	})
@@ -48,7 +49,7 @@ func TestRun(t *testing.T) {
 				"test": {},
 			},
 		}
-		if err := Run(ctx, conf, nil, nil, nil); err == nil {
+		if err := Run(ctx, conf, nil, nil, nil, false); err == nil {
 			t.Error("want error, got nil")
 		} else {
 			t.Log(err)
@@ -69,14 +70,16 @@ func TestRun(t *testing.T) {
 				},
 			},
 		}
-		if err := Run(ctx, conf, nil, nil, ds); err == nil {
+		if err := Run(ctx, conf, nil, nil, ds, false); err == nil {
 			t.Error("want error, got nil")
 		} else {
 			t.Log(err)
 		}
 	})
 
-	t.Run("no cmd error (not for reviewdog to exit with error)", func(t *testing.T) {
+	t.Run("cmd error (not for reviewdog to exit with error)", func(t *testing.T) {
+		buf := new(bytes.Buffer)
+		defaultTeeStderr = buf
 		ds := &fakeDiffService{
 			FakeDiff: func() ([]byte, error) {
 				return []byte(""), nil
@@ -95,9 +98,43 @@ func TestRun(t *testing.T) {
 				},
 			},
 		}
-		if err := Run(ctx, conf, nil, cs, ds); err != nil {
+		if err := Run(ctx, conf, nil, cs, ds, false); err != nil {
 			t.Error(err)
 		}
+		want := ""
+		if got := buf.String(); got != want {
+			t.Errorf("got stderr %q, want %q", got, want)
+		}
+	})
+
+	t.Run("cmd error with tee", func(t *testing.T) {
+		buf := new(bytes.Buffer)
+		defaultTeeStderr = buf
+		ds := &fakeDiffService{
+			FakeDiff: func() ([]byte, error) {
+				return []byte(""), nil
+			},
+		}
+		cs := &fakeCommentService{
+			FakePost: func(c *reviewdog.Comment) error {
+				return nil
+			},
+		}
+		conf := &Config{
+			Runner: map[string]*Runner{
+				"test": {
+					Cmd:         "not found",
+					Errorformat: []string{`%f:%l:%c:%m`},
+				},
+			},
+		}
+		if err := Run(ctx, conf, nil, cs, ds, true); err != nil {
+			t.Error(err)
+		}
+		want := "sh: 1: not: not found\n"
+		if got := buf.String(); got != want {
+			t.Errorf("got stderr %q, want %q", got, want)
+		}
 	})
 
 	t.Run("success", func(t *testing.T) {
@@ -119,11 +156,41 @@ func TestRun(t *testing.T) {
 				},
 			},
 		}
-		if err := Run(ctx, conf, nil, cs, ds); err != nil {
+		if err := Run(ctx, conf, nil, cs, ds, false); err != nil {
 			t.Error(err)
 		}
 	})
 
+	t.Run("success with tee", func(t *testing.T) {
+		buf := new(bytes.Buffer)
+		defaultTeeStdout = buf
+		ds := &fakeDiffService{
+			FakeDiff: func() ([]byte, error) {
+				return []byte(""), nil
+			},
+		}
+		cs := &fakeCommentService{
+			FakePost: func(c *reviewdog.Comment) error {
+				return nil
+			},
+		}
+		conf := &Config{
+			Runner: map[string]*Runner{
+				"test": {
+					Cmd:         "echo 'hi'",
+					Errorformat: []string{`%f:%l:%c:%m`},
+				},
+			},
+		}
+		if err := Run(ctx, conf, nil, cs, ds, true); err != nil {
+			t.Error(err)
+		}
+		want := "hi\n"
+		if got := buf.String(); got != want {
+			t.Errorf("got stdout %q, want %q", got, want)
+		}
+	})
+
 	t.Run("runners", func(t *testing.T) {
 		called := 0
 		ds := &fakeDiffService{
@@ -151,7 +218,7 @@ func TestRun(t *testing.T) {
 				},
 			},
 		}
-		if err := Run(ctx, conf, map[string]bool{"test2": true}, cs, ds); err != nil {
+		if err := Run(ctx, conf, map[string]bool{"test2": true}, cs, ds, false); err != nil {
 			t.Error(err)
 		}
 		if called != 1 {
@@ -184,7 +251,7 @@ func TestRun(t *testing.T) {
 				},
 			},
 		}
-		if err := Run(ctx, conf, map[string]bool{"hoge": true}, cs, ds); err == nil {
+		if err := Run(ctx, conf, map[string]bool{"hoge": true}, cs, ds, false); err == nil {
 			t.Error("got no error but want runner not found error")
 		}
 	})