Skip to content

Commit

Permalink
handler: support artifact env
Browse files Browse the repository at this point in the history
  • Loading branch information
CarlJi committed Jul 1, 2024
1 parent e6c7b98 commit 4cd0676
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 8 deletions.
3 changes: 3 additions & 0 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,9 @@ type GithubReportType string
const (
GithubCheckRuns GithubReportType = "github_check_run"
GithubPRReview GithubReportType = "github_pr_review"

// for debug and testing.
Quiet GithubReportType = "quiet"
)

func boolPtr(b bool) *bool {
Expand Down
58 changes: 51 additions & 7 deletions internal/linters/linters.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ var (
linterLanguages = map[string][]string{}
)

const artifactDirFlag = "reviewbot"

// PullRequestHandlerFunc knows how to handle a pull request event.
type PullRequestHandlerFunc func(*xlog.Logger, Agent) error

Expand Down Expand Up @@ -130,7 +132,7 @@ type LinterParser func(*xlog.Logger, []byte) (map[string][]LinterOutput, []strin

func GeneralHandler(log *xlog.Logger, a Agent, linterParser func(*xlog.Logger, []byte) (map[string][]LinterOutput, []string)) error {
linterName := a.LinterName
output, err := ExecRun(a.LinterConfig.WorkDir, a.LinterConfig.Command, a.LinterConfig.Args...)
output, err := ExecRun(a.LinterConfig.WorkDir, a.LinterConfig.Command, a.LinterConfig.Args)
if err != nil {
// NOTE(CarlJi): the error is *ExitError, it seems to have little information and needs to be handled in a better way.
log.Warnf("%s run with exit code: %v, mark and continue", linterName, err)
Expand All @@ -151,21 +153,61 @@ func GeneralHandler(log *xlog.Logger, a Agent, linterParser func(*xlog.Logger, [
}

// ExecRun executes a command.
func ExecRun(workDir string, command []string, args ...string) ([]byte, error) {
func ExecRun(workDir string, command []string, args []string) ([]byte, error) {
executable := command[0]
var cmdArgs []string
if len(command) > 1 {
cmdArgs = command[1:]
}
cmdArgs = append(cmdArgs, args...)
c := exec.Command(executable, cmdArgs...)
c.Dir = workDir

if len(args) > 0 {
cmdArgs = append(cmdArgs, args...)
// create a temp dir for the artifact
artifact, err := os.MkdirTemp("", artifactDirFlag)
if err != nil {
return nil, err
}
defer os.RemoveAll(artifact)
c.Env = append(os.Environ(), fmt.Sprintf("ARTIFACT=%s", artifact))

c := exec.Command(executable, cmdArgs...)
c.Dir = workDir
log.Infof("run command: %v", c)
return c.CombinedOutput()
output, execErr := c.CombinedOutput()

// read all files under the artifact dir
var fileContent []byte
artifactFiles, err := os.ReadDir(artifact)
if err != nil {
return nil, err
}

var idx int
for _, file := range artifactFiles {
if file.IsDir() {
continue
}

content, err := os.ReadFile(fmt.Sprintf("%s/%s", artifact, file.Name()))
if err != nil {
return nil, err
}
if len(content) == 0 {
continue
}
if idx > 0 {
fileContent = append(fileContent, '\n')
}
fileContent = append(fileContent, content...)
idx++
}

// use the content of the files under Artifact dir as first priority
if len(fileContent) > 0 {
log.Debugf("artifact files used instead. legacy output:\n%v", string(output))
output = fileContent
}

return output, execErr
}

// GeneralParse parses the output of a linter command.
Expand Down Expand Up @@ -246,6 +288,8 @@ func Report(log *xlog.Logger, a Agent, lintResults map[string][]LinterOutput) er
}
log.Infof("[%s] add %d comments for this PR %d (%s) \n", linterName, len(addedCmts), num, orgRepo)
metric.NotifyWebhookByText(ConstructGotchaMsg(linterName, a.PullRequestEvent.GetPullRequest().GetHTMLURL(), addedCmts[0].GetHTMLURL(), lintResults))
case config.Quiet:
return nil
default:
log.Errorf("unsupported report format: %v", a.LinterConfig.ReportFormat)
}
Expand Down
68 changes: 68 additions & 0 deletions internal/linters/linters_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,12 @@
package linters

import (
"errors"
"fmt"
"reflect"
"testing"

"github.com/qiniu/reviewbot/config"
)

func TestFormatStaticcheckLine(t *testing.T) {
Expand Down Expand Up @@ -168,3 +171,68 @@ func TestConstructUnknownMsg(t *testing.T) {
}
}
}

func TestExecRun(t *testing.T) {
tp := true
tcs := []struct {
id string
input Agent
output []byte
err error
}{
{
id: "case1 - without ARTIFACT",
input: Agent{
LinterName: "ut",
LinterConfig: config.Linter{
Enable: &tp,
Command: []string{"/bin/bash", "-c", "--"},
Args: []string{"echo file:line:column:message"},
ReportFormat: config.Quiet,
},
},
output: []byte("file:line:column:message\n"),
err: nil,
},
{
id: "case2 - with ARTIFACT",
input: Agent{
LinterName: "ut",
LinterConfig: config.Linter{
Enable: &tp,
Command: []string{"/bin/bash", "-c", "--"},
Args: []string{"echo file2:6:7:message >> $ARTIFACT/golangci-lint.log 2>&1"},
ReportFormat: config.Quiet,
},
},
output: []byte("file2:6:7:message\n"),
err: nil,
},
{
id: "case2 - with multi files under ARTIFACT",
input: Agent{
LinterName: "ut",
LinterConfig: config.Linter{
Enable: &tp,
Command: []string{"/bin/bash", "-c", "--"},
Args: []string{"echo file2:6:7:message >> $ARTIFACT/golangci-lint.log 2>&1 ;echo file3:6:7:message >> $ARTIFACT/golangci-lint.log 2>&1"},
ReportFormat: config.Quiet,
},
},
output: []byte("file2:6:7:message\nfile3:6:7:message\n"),
err: nil,
},
}
for _, tc := range tcs {
t.Run(tc.id, func(t *testing.T) {
output, err := ExecRun(tc.input.LinterConfig.WorkDir, tc.input.LinterConfig.Command, tc.input.LinterConfig.Args)
if !errors.Is(err, tc.err) {
t.Errorf("expected: %v, got: %v", tc.err, err)
}

if string(output) != string(tc.output) {
t.Errorf("expected: %v, got: %v", string(tc.output), string(output))
}
})
}
}
2 changes: 1 addition & 1 deletion internal/linters/shell/shellcheck/shellcheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func shellcheck(log *xlog.Logger, a linters.Agent) error {
a.LinterConfig.Args = args
}

output, err := linters.ExecRun(a.LinterConfig.WorkDir, a.LinterConfig.Command, a.LinterConfig.Args...)
output, err := linters.ExecRun(a.LinterConfig.WorkDir, a.LinterConfig.Command, a.LinterConfig.Args)
if err != nil {
log.Warnf("%s run with error: %v, mark and continue", cmd, err)
}
Expand Down

0 comments on commit 4cd0676

Please sign in to comment.