Repository: reviewdog/reviewdog
Pull request #283 (2019-09-14 13:36:15)

Implement -runners flag to specify which runners to run in conf file


Patch:
diff --git a/.github/workflows/reviewdog.yml b/.github/workflows/reviewdog.yml
index 8b0cbeec..d07dcf87 100644
--- a/.github/workflows/reviewdog.yml
+++ b/.github/workflows/reviewdog.yml
@@ -37,4 +37,4 @@ jobs:
           REVIEWDOG_GITHUB_API_TOKEN: ${{ secrets.GITHUB_TOKEN }}
         run: |
           PATH=$(go env GOPATH)/bin:$PATH
-          golint ./... | reviewdog -f=golint -name=golint-via-github-actions -reporter=github-pr-check
+          reviewdog -reporter=github-pr-check -runners=golint,govet
diff --git a/README.md b/README.md
index 229b6d10..518592e9 100644
--- a/README.md
+++ b/README.md
@@ -202,6 +202,9 @@ $ reviewdog -diff="git diff master"
 project/run_test.go:61:28: [golint] error strings should not end with punctuation
 project/run.go:57:18: [errcheck]        defer os.Setenv(name, os.Getenv(name))
 project/run.go:58:12: [errcheck]        os.Setenv(name, "")
+# You can use -runners to run only specified runners.
+$ reviewdog -diff="git diff master" -runners=golint,govet
+project/run_test.go:61:28: [golint] error strings should not end with punctuation
 # You can use -conf to specify config file path.
 $ reviewdog -conf=./.reviewdog.yml -reporter=github-pr-check
 ```
diff --git a/cmd/reviewdog/doghouse.go b/cmd/reviewdog/doghouse.go
index 845a9552..c6f7cb6f 100644
--- a/cmd/reviewdog/doghouse.go
+++ b/cmd/reviewdog/doghouse.go
@@ -85,7 +85,7 @@ func checkResultSet(ctx context.Context, r io.Reader, opt *option, isProject boo
 		if err != nil {
 			return nil, err
 		}
-		resultSet, err = projectRunAndParse(ctx, conf)
+		resultSet, err = projectRunAndParse(ctx, conf, buildRunnersMap(opt.runners))
 		if err != nil {
 			return nil, err
 		}
diff --git a/cmd/reviewdog/main.go b/cmd/reviewdog/main.go
index 10423b97..d55dc369 100644
--- a/cmd/reviewdog/main.go
+++ b/cmd/reviewdog/main.go
@@ -48,6 +48,7 @@ type option struct {
 	name             string // tool name which is used in comment
 	ci               string
 	conf             string
+	runners          string
 	reporter         string
 	guessPullRequest bool
 }
@@ -62,6 +63,7 @@ const (
 	nameDoc             = `tool name in review comment. -f is used as tool name if -name is empty`
 	ciDoc               = `[deprecated] reviewdog automatically get necessary data. See also -reporter for migration`
 	confDoc             = `config file path`
+	runnersDoc          = `comma separated runners name to run in config file. default: run all runners`
 	guessPullRequestDoc = `guess Pull Request ID by branch name and commit SHA`
 	reporterDoc         = `reporter of reviewdog results. (local, github-pr-check, github-pr-review, gitlab-mr-discussion, gitlab-mr-commit)
 	"local" (default)
@@ -137,6 +139,7 @@ func init() {
 	flag.StringVar(&opt.name, "name", "", nameDoc)
 	flag.StringVar(&opt.ci, "ci", "", ciDoc)
 	flag.StringVar(&opt.conf, "conf", "", confDoc)
+	flag.StringVar(&opt.runners, "runners", "", runnersDoc)
 	flag.StringVar(&opt.reporter, "reporter", "local", reporterDoc)
 	flag.BoolVar(&opt.guessPullRequest, "guess", false, guessPullRequestDoc)
 }
@@ -262,7 +265,7 @@ See -reporter flag for migration and set -reporter="github-pr-review" or -report
 		if err != nil {
 			return err
 		}
-		return project.Run(ctx, conf, cs, ds)
+		return project.Run(ctx, conf, buildRunnersMap(opt.runners), cs, ds)
 	}
 
 	p, err := newParserFromOpt(opt)
@@ -559,3 +562,13 @@ func toolName(opt *option) string {
 	}
 	return name
 }
+
+func buildRunnersMap(runners string) map[string]bool {
+	m := make(map[string]bool)
+	for _, r := range strings.Split(runners, ",") {
+		if name := strings.TrimSpace(r); name != "" {
+			m[name] = true
+		}
+	}
+	return m
+}
diff --git a/project/run.go b/project/run.go
index c8b728d3..ca930f5a 100644
--- a/project/run.go
+++ b/project/run.go
@@ -5,9 +5,11 @@ import (
 	"context"
 	"fmt"
 	"io"
+	"log"
 	"os"
 	"os/exec"
 	"runtime"
+	"strings"
 
 	"github.com/reviewdog/reviewdog"
 	"github.com/reviewdog/reviewdog/diff"
@@ -15,15 +17,21 @@ import (
 )
 
 // RunAndParse runs commands and parse results. Returns map of tool name to check results.
-func RunAndParse(ctx context.Context, conf *Config) (*reviewdog.ResultMap, error) {
+func RunAndParse(ctx context.Context, conf *Config, runners map[string]bool) (*reviewdog.ResultMap, error) {
 	var results reviewdog.ResultMap
 	// environment variables for each commands
 	envs := filteredEnviron()
+	var usedRunners []string
 	var g errgroup.Group
 	semaphore := make(chan int, runtime.NumCPU())
 	for _, runner := range conf.Runner {
 		runner := runner
+		if len(runners) != 0 && !runners[runner.Name] {
+			continue // Skip this runner.
+		}
+		usedRunners = append(usedRunners, runner.Name)
 		semaphore <- 1
+		log.Printf("reviewdog: [start]\trunner=%s", runner.Name)
 		fname := runner.Format
 		if fname == "" && len(runner.Errorformat) == 0 {
 			fname = runner.Name
@@ -49,6 +57,7 @@ func RunAndParse(ctx context.Context, conf *Config) (*reviewdog.ResultMap, error
 			if err != nil {
 				return err
 			}
+			log.Printf("reviewdog: [finish]\trunner=%s", runner.Name)
 			results.Store(runner.Name, rs)
 			return nil
 		})
@@ -56,12 +65,15 @@ func RunAndParse(ctx context.Context, conf *Config) (*reviewdog.ResultMap, error
 	if err := g.Wait(); err != nil {
 		return nil, fmt.Errorf("fail to run reviewdog: %v", err)
 	}
+	if err := checkUnknownRunner(runners, usedRunners); err != nil {
+		return nil, err
+	}
 	return &results, nil
 }
 
 // Run runs reviewdog tasks based on Config.
-func Run(ctx context.Context, conf *Config, c reviewdog.CommentService, d reviewdog.DiffService) error {
-	results, err := RunAndParse(ctx, conf)
+func Run(ctx context.Context, conf *Config, runners map[string]bool, c reviewdog.CommentService, d reviewdog.DiffService) error {
+	results, err := RunAndParse(ctx, conf, runners)
 	if err != nil {
 		return err
 	}
@@ -102,3 +114,20 @@ func filteredEnviron() []string {
 	}
 	return os.Environ()
 }
+
+func checkUnknownRunner(specifiedRunners map[string]bool, usedRunners []string) error {
+	if len(specifiedRunners) == 0 {
+		return nil
+	}
+	for _, r := range usedRunners {
+		delete(specifiedRunners, r)
+	}
+	var rs []string
+	for r := range specifiedRunners {
+		rs = append(rs, r)
+	}
+	if len(specifiedRunners) != 0 {
+		return fmt.Errorf("runner not found: [%s]", strings.Join(rs, ","))
+	}
+	return nil
+}


Test patch:
diff --git a/cmd/reviewdog/doghouse_test.go b/cmd/reviewdog/doghouse_test.go
index b81e1ad6..568d1a34 100644
--- a/cmd/reviewdog/doghouse_test.go
+++ b/cmd/reviewdog/doghouse_test.go
@@ -110,7 +110,7 @@ func TestNewDoghouseServerCli(t *testing.T) {
 }
 
 func TestCheckResultSet_Project(t *testing.T) {
-	defer func(f func(ctx context.Context, conf *project.Config) (*reviewdog.ResultMap, error)) {
+	defer func(f func(ctx context.Context, conf *project.Config, runners map[string]bool) (*reviewdog.ResultMap, error)) {
 		projectRunAndParse = f
 	}(projectRunAndParse)
 
@@ -124,7 +124,7 @@ func TestCheckResultSet_Project(t *testing.T) {
 		},
 	})
 
-	projectRunAndParse = func(ctx context.Context, conf *project.Config) (*reviewdog.ResultMap, error) {
+	projectRunAndParse = func(ctx context.Context, conf *project.Config, runners map[string]bool) (*reviewdog.ResultMap, error) {
 		return &wantCheckResult, nil
 	}
 
diff --git a/project/run_test.go b/project/run_test.go
index 03fc6175..9e2b20c2 100644
--- a/project/run_test.go
+++ b/project/run_test.go
@@ -37,7 +37,7 @@ func TestRun(t *testing.T) {
 
 	t.Run("empty", func(t *testing.T) {
 		conf := &Config{}
-		if err := Run(ctx, conf, nil, nil); err != nil {
+		if err := Run(ctx, conf, nil, nil, nil); err != nil {
 			t.Error(err)
 		}
 	})
@@ -48,7 +48,7 @@ func TestRun(t *testing.T) {
 				"test": {},
 			},
 		}
-		if err := Run(ctx, conf, nil, nil); err == nil {
+		if err := Run(ctx, conf, nil, nil, nil); err == nil {
 			t.Error("want error, got nil")
 		} else {
 			t.Log(err)
@@ -69,7 +69,7 @@ func TestRun(t *testing.T) {
 				},
 			},
 		}
-		if err := Run(ctx, conf, nil, ds); err == nil {
+		if err := Run(ctx, conf, nil, nil, ds); err == nil {
 			t.Error("want error, got nil")
 		} else {
 			t.Log(err)
@@ -95,7 +95,7 @@ func TestRun(t *testing.T) {
 				},
 			},
 		}
-		if err := Run(ctx, conf, cs, ds); err != nil {
+		if err := Run(ctx, conf, nil, cs, ds); err != nil {
 			t.Error(err)
 		}
 	})
@@ -119,11 +119,76 @@ func TestRun(t *testing.T) {
 				},
 			},
 		}
-		if err := Run(ctx, conf, cs, ds); err != nil {
+		if err := Run(ctx, conf, nil, cs, ds); err != nil {
 			t.Error(err)
 		}
 	})
 
+	t.Run("runners", func(t *testing.T) {
+		called := 0
+		ds := &fakeDiffService{
+			FakeDiff: func() ([]byte, error) {
+				called++
+				return []byte(""), nil
+			},
+		}
+		cs := &fakeCommentService{
+			FakePost: func(c *reviewdog.Comment) error {
+				return nil
+			},
+		}
+		conf := &Config{
+			Runner: map[string]*Runner{
+				"test1": {
+					Name:        "test1",
+					Cmd:         "echo 'test1'",
+					Errorformat: []string{`%f:%l:%c:%m`},
+				},
+				"test2": {
+					Name:        "test2",
+					Cmd:         "echo 'test2'",
+					Errorformat: []string{`%f:%l:%c:%m`},
+				},
+			},
+		}
+		if err := Run(ctx, conf, map[string]bool{"test2": true}, cs, ds); err != nil {
+			t.Error(err)
+		}
+		if called != 1 {
+			t.Errorf("Diff service called %d times, want 1 time", called)
+		}
+	})
+
+	t.Run("unknown runners", func(t *testing.T) {
+		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{
+				"test1": {
+					Name:        "test1",
+					Cmd:         "echo 'test1'",
+					Errorformat: []string{`%f:%l:%c:%m`},
+				},
+				"test2": {
+					Name:        "test2",
+					Cmd:         "echo 'test2'",
+					Errorformat: []string{`%f:%l:%c:%m`},
+				},
+			},
+		}
+		if err := Run(ctx, conf, map[string]bool{"hoge": true}, cs, ds); err == nil {
+			t.Error("got no error but want runner not found error")
+		}
+	})
+
 }
 
 func TestFilteredEnviron(t *testing.T) {