Skip to content

Commit

Permalink
Use Diagnostic.original_output internally and remove CheckResult type
Browse files Browse the repository at this point in the history 
follow up of #688
  • Loading branch information
@haya14busa
haya14busa committed Jul 25, 2020
1 parent b21c241 commit 4432b86
Show file tree
Hide file tree
Showing 23 changed files with 383 additions and 453 deletions.
31 changes: 14 additions & 17 deletions cmd/reviewdog/doghouse.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"os"
"path/filepath"
"sort"
"strings"

"golang.org/x/oauth2"
"golang.org/x/sync/errgroup"
Expand All @@ -20,6 +19,7 @@ import (
"github.com/reviewdog/reviewdog/doghouse"
"github.com/reviewdog/reviewdog/doghouse/client"
"github.com/reviewdog/reviewdog/project"
"github.com/reviewdog/reviewdog/proto/rdf"
"github.com/reviewdog/reviewdog/service/github/githubutils"
"github.com/reviewdog/reviewdog/service/serviceutil"
)
Expand Down Expand Up @@ -99,13 +99,13 @@ func checkResultSet(ctx context.Context, r io.Reader, opt *option, isProject boo
if err != nil {
return nil, err
}
rs, err := p.Parse(r)
diagnostics, err := p.Parse(r)
if err != nil {
return nil, err
}
resultSet.Store(toolName(opt), &reviewdog.Result{
Level: opt.level,
CheckResults: rs,
Level: opt.level,
Diagnostics: diagnostics,
})
}
return resultSet, nil
Expand All @@ -121,10 +121,10 @@ func postResultSet(ctx context.Context, resultSet *reviewdog.ResultMap,
}
filteredResultSet := new(reviewdog.FilteredResultMap)
resultSet.Range(func(name string, result *reviewdog.Result) {
checkResults := result.CheckResults
as := make([]*doghouse.Annotation, 0, len(checkResults))
for _, r := range checkResults {
as = append(as, checkResultToAnnotation(r, wd, gitRelWd))
diagnostics := result.Diagnostics
as := make([]*doghouse.Annotation, 0, len(diagnostics))
for _, d := range diagnostics {
as = append(as, checkResultToAnnotation(d, wd, gitRelWd))
}
req := &doghouse.CheckRequest{
Name: name,
Expand Down Expand Up @@ -178,12 +178,11 @@ func postResultSet(ctx context.Context, resultSet *reviewdog.ResultMap,
return filteredResultSet, g.Wait()
}

func checkResultToAnnotation(c *reviewdog.CheckResult, wd, gitRelWd string) *doghouse.Annotation {
c.Diagnostic.GetLocation().Path = filepath.ToSlash(filepath.Join(
gitRelWd, reviewdog.CleanPath(c.Diagnostic.GetLocation().GetPath(), wd)))
func checkResultToAnnotation(d *rdf.Diagnostic, wd, gitRelWd string) *doghouse.Annotation {
d.GetLocation().Path = filepath.ToSlash(filepath.Join(
gitRelWd, reviewdog.CleanPath(d.GetLocation().GetPath(), wd)))
return &doghouse.Annotation{
Diagnostic: c.Diagnostic,
RawMessage: strings.Join(c.Lines, "\n"),
Diagnostic: d,
}
}

Expand Down Expand Up @@ -237,12 +236,10 @@ report results via logging command [1].

foundResultPerName = true
if cienv.IsInGitHubAction() {
githubutils.ReportAsGitHubActionsLog(name, results.Level, result.CheckResult)
githubutils.ReportAsGitHubActionsLog(name, results.Level, result.Diagnostic)
} else {
// Output original lines.
for _, line := range result.Lines {
fmt.Fprintln(w, line)
}
fmt.Fprintln(w, result.Diagnostic.GetOriginalOutput())
}
}
if !foundResultPerName {
Expand Down
116 changes: 53 additions & 63 deletions cmd/reviewdog/doghouse_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,18 +121,16 @@ func TestCheckResultSet_Project(t *testing.T) {
}(projectRunAndParse)

var wantCheckResult reviewdog.ResultMap
wantCheckResult.Store("name1", &reviewdog.Result{CheckResults: []*reviewdog.CheckResult{
wantCheckResult.Store("name1", &reviewdog.Result{Diagnostics: []*rdf.Diagnostic{
{
Diagnostic: &rdf.Diagnostic{
Location: &rdf.Location{
Range: &rdf.Range{Start: &rdf.Position{
Line: 1,
Column: 14,
}},
Path: "reviewdog.go",
},
Message: "msg",
Location: &rdf.Location{
Range: &rdf.Range{Start: &rdf.Position{
Line: 1,
Column: 14,
}},
Path: "reviewdog.go",
},
Message: "msg",
},
}})

Expand Down Expand Up @@ -172,19 +170,17 @@ func TestCheckResultSet_NonProject(t *testing.T) {
t.Fatal(err)
}
var want reviewdog.ResultMap
want.Store("golint", &reviewdog.Result{CheckResults: []*reviewdog.CheckResult{
want.Store("golint", &reviewdog.Result{Diagnostics: []*rdf.Diagnostic{
{
Diagnostic: &rdf.Diagnostic{
Location: &rdf.Location{
Range: &rdf.Range{Start: &rdf.Position{
Line: 14,
Column: 14,
}},
Path: "reviewdog.go",
},
Message: "test message",
Location: &rdf.Location{
Range: &rdf.Range{Start: &rdf.Position{
Line: 14,
Column: 14,
}},
Path: "reviewdog.go",
},
Lines: []string{input},
Message: "test message",
OriginalOutput: input,
},
}})

Expand Down Expand Up @@ -242,8 +238,8 @@ func TestPostResultSet_withReportURL(t *testing.T) {
Start: &rdf.Position{Line: 14},
},
},
OriginalOutput: "L1\nL2",
},
RawMessage: "L1\nL2",
},
{
Diagnostic: &rdf.Diagnostic{
Expand Down Expand Up @@ -280,39 +276,33 @@ func TestPostResultSet_withReportURL(t *testing.T) {

// It assumes the current dir is ./cmd/reviewdog/
var resultSet reviewdog.ResultMap
resultSet.Store("name1", &reviewdog.Result{CheckResults: []*reviewdog.CheckResult{
resultSet.Store("name1", &reviewdog.Result{Diagnostics: []*rdf.Diagnostic{
{
Diagnostic: &rdf.Diagnostic{
Location: &rdf.Location{
Range: &rdf.Range{Start: &rdf.Position{
Line: 14,
}},
Path: "reviewdog.go", // test relative path
},
Message: "name1: test 1",
Location: &rdf.Location{
Range: &rdf.Range{Start: &rdf.Position{
Line: 14,
}},
Path: "reviewdog.go", // test relative path
},
Lines: []string{"L1", "L2"},
Message: "name1: test 1",
OriginalOutput: "L1\nL2",
},
{
Diagnostic: &rdf.Diagnostic{
Location: &rdf.Location{
Path: absPath(t, "reviewdog.go"), // test abs path
},
Message: "name1: test 2",
Location: &rdf.Location{
Path: absPath(t, "reviewdog.go"), // test abs path
},
Message: "name1: test 2",
},
}})
resultSet.Store("name2", &reviewdog.Result{CheckResults: []*reviewdog.CheckResult{
resultSet.Store("name2", &reviewdog.Result{Diagnostics: []*rdf.Diagnostic{
{
Diagnostic: &rdf.Diagnostic{
Location: &rdf.Location{
Range: &rdf.Range{Start: &rdf.Position{
Line: 14,
}},
Path: "doghouse.go",
},
Message: "name2: test 1",
Location: &rdf.Location{
Range: &rdf.Range{Start: &rdf.Position{
Line: 14,
}},
Path: "doghouse.go",
},
Message: "name2: test 1",
},
}})

Expand Down Expand Up @@ -344,7 +334,7 @@ func TestPostResultSet_withoutReportURL(t *testing.T) {
}

var resultSet reviewdog.ResultMap
resultSet.Store("name1", &reviewdog.Result{CheckResults: []*reviewdog.CheckResult{}})
resultSet.Store("name1", &reviewdog.Result{Diagnostics: []*rdf.Diagnostic{}})

ghInfo := &cienv.BuildInfo{Owner: owner, Repo: repo, PullRequest: prNum, SHA: sha}

Expand Down Expand Up @@ -375,7 +365,7 @@ func TestPostResultSet_conclusion(t *testing.T) {

fakeCli := &fakeDoghouseServerCli{}
var resultSet reviewdog.ResultMap
resultSet.Store("name1", &reviewdog.Result{CheckResults: []*reviewdog.CheckResult{}})
resultSet.Store("name1", &reviewdog.Result{Diagnostics: []*rdf.Diagnostic{}})
ghInfo := &cienv.BuildInfo{Owner: owner, Repo: repo, PullRequest: prNum, SHA: sha}

tests := []struct {
Expand Down Expand Up @@ -420,7 +410,7 @@ func TestPostResultSet_withEmptyResponse(t *testing.T) {
}

var resultSet reviewdog.ResultMap
resultSet.Store("name1", &reviewdog.Result{CheckResults: []*reviewdog.CheckResult{}})
resultSet.Store("name1", &reviewdog.Result{Diagnostics: []*rdf.Diagnostic{}})

ghInfo := &cienv.BuildInfo{Owner: owner, Repo: repo, PullRequest: prNum, SHA: sha}

Expand All @@ -440,14 +430,14 @@ func TestReportResults(t *testing.T) {
filteredResultSet.Store("name1", &reviewdog.FilteredResult{
FilteredCheck: []*reviewdog.FilteredCheck{
{
CheckResult: &reviewdog.CheckResult{
Lines: []string{"name1-L1", "name1-L2"},
Diagnostic: &rdf.Diagnostic{
OriginalOutput: "name1-L1\nname1-L2",
},
ShouldReport: true,
},
{
CheckResult: &reviewdog.CheckResult{
Lines: []string{"name1.2-L1", "name1.2-L2"},
Diagnostic: &rdf.Diagnostic{
OriginalOutput: "name1.2-L1\nname1.2-L2",
},
ShouldReport: false,
},
Expand All @@ -456,8 +446,8 @@ func TestReportResults(t *testing.T) {
filteredResultSet.Store("name2", &reviewdog.FilteredResult{
FilteredCheck: []*reviewdog.FilteredCheck{
{
CheckResult: &reviewdog.CheckResult{
Lines: []string{"name1-L1", "name1-L2"},
Diagnostic: &rdf.Diagnostic{
OriginalOutput: "name1-L1\nname1-L2",
},
ShouldReport: false,
},
Expand Down Expand Up @@ -489,8 +479,8 @@ func TestReportResults_inGitHubAction(t *testing.T) {
filteredResultSet.Store("name1", &reviewdog.FilteredResult{
FilteredCheck: []*reviewdog.FilteredCheck{
{
CheckResult: &reviewdog.CheckResult{
Lines: []string{"name1-L1", "name1-L2"},
Diagnostic: &rdf.Diagnostic{
OriginalOutput: "name1-L1\nname1-L2",
},
ShouldReport: true,
},
Expand All @@ -515,14 +505,14 @@ func TestReportResults_noResultsShouldReport(t *testing.T) {
filteredResultSet.Store("name1", &reviewdog.FilteredResult{
FilteredCheck: []*reviewdog.FilteredCheck{
{
CheckResult: &reviewdog.CheckResult{
Lines: []string{"name1-L1", "name1-L2"},
Diagnostic: &rdf.Diagnostic{
OriginalOutput: "name1-L1\nname1-L2",
},
ShouldReport: false,
},
{
CheckResult: &reviewdog.CheckResult{
Lines: []string{"name1.2-L1", "name1.2-L2"},
Diagnostic: &rdf.Diagnostic{
OriginalOutput: "name1.2-L1\nname1.2-L2",
},
ShouldReport: false,
},
Expand All @@ -531,8 +521,8 @@ func TestReportResults_noResultsShouldReport(t *testing.T) {
filteredResultSet.Store("name2", &reviewdog.FilteredResult{
FilteredCheck: []*reviewdog.FilteredCheck{
{
CheckResult: &reviewdog.CheckResult{
Lines: []string{"name1-L1", "name1-L2"},
Diagnostic: &rdf.Diagnostic{
OriginalOutput: "name1-L1\nname1-L2",
},
ShouldReport: false,
},
Expand Down
3 changes: 1 addition & 2 deletions comment_iowriter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"fmt"
"io"
"strings"
)

var _ CommentService = &RawCommentWriter{}
Expand All @@ -20,7 +19,7 @@ func NewRawCommentWriter(w io.Writer) *RawCommentWriter {
}

func (s *RawCommentWriter) Post(_ context.Context, c *Comment) error {
_, err := fmt.Fprintln(s.w, strings.Join(c.Result.Lines, "\n"))
_, err := fmt.Fprintln(s.w, c.Result.Diagnostic.OriginalOutput)
return err
}

Expand Down
16 changes: 8 additions & 8 deletions comment_iowriter_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,55 +16,55 @@ func TestUnifiedCommentWriter_Post(t *testing.T) {
}{
{
in: &Comment{
Result: &FilteredCheck{CheckResult: &CheckResult{
Result: &FilteredCheck{
Diagnostic: &rdf.Diagnostic{Location: &rdf.Location{Path: "/path/to/file"}},
}},
},
ToolName: "tool name",
Body: "message",
},
want: `/path/to/file: [tool name] message`,
},
{
in: &Comment{
Result: &FilteredCheck{CheckResult: &CheckResult{
Result: &FilteredCheck{
Diagnostic: &rdf.Diagnostic{Location: &rdf.Location{
Path: "/path/to/file",
Range: &rdf.Range{Start: &rdf.Position{
Column: 14,
}},
}},
}},
},
ToolName: "tool name",
Body: "message",
},
want: `/path/to/file: [tool name] message`,
},
{
in: &Comment{
Result: &FilteredCheck{CheckResult: &CheckResult{
Result: &FilteredCheck{
Diagnostic: &rdf.Diagnostic{Location: &rdf.Location{
Path: "/path/to/file",
Range: &rdf.Range{Start: &rdf.Position{
Line: 14,
}},
}},
}},
},
ToolName: "tool name",
Body: "message",
},
want: `/path/to/file:14: [tool name] message`,
},
{
in: &Comment{
Result: &FilteredCheck{CheckResult: &CheckResult{
Result: &FilteredCheck{
Diagnostic: &rdf.Diagnostic{Location: &rdf.Location{
Path: "/path/to/file",
Range: &rdf.Range{Start: &rdf.Position{
Line: 14,
Column: 7,
}},
}},
}},
},
ToolName: "tool name",
Body: "line1\nline2",
},
Expand Down

0 comments on commit 4432b86

Please sign in to comment.