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 07962ab
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 5 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

Check warning on line 159 in config/config.go

View check run for this annotation

qiniu-x / golangci-lint

config/config.go#L159

Comment should end in a period (godot)
Quiet GithubReportType = "quiet"
)

func boolPtr(b bool) *bool {
Expand Down
47 changes: 42 additions & 5 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 @@ -157,15 +159,48 @@ func ExecRun(workDir string, command []string, args ...string) ([]byte, error) {
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
}

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
}

fileContent = append(fileContent, content...)
}

// 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 +281,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
48 changes: 48 additions & 0 deletions internal/linters/linters_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ import (
"fmt"
"reflect"
"testing"

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

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

func TestGeneralHandler(t *testing.T) {
tp := true
tcs := []struct {
id string
input Agent
err error
}{
{
id: "case1 - without ARTIFACT",
input: Agent{
LinterName: "ut",
LinterConfig: config.Linter{
Enable: &tp,
Command: []string{"/bin/bash", "-c", "--"},
Args: []string{"golangci-lint run"},
ReportFormat: config.Quiet,
},
},
err: nil,
},
{
id: "case2 - with ARTIFACT",
input: Agent{
LinterName: "ut",
LinterConfig: config.Linter{
Enable: &tp,
Command: []string{"/bin/bash", "-c", "--"},
Args: []string{"golangci-lint run >> $ARTIFACT/golangci-lint.log 2>&1"},
ReportFormat: config.Quiet,
},
},
err: nil,
},
}

for _, tc := range tcs {
t.Run(tc.id, func(t *testing.T) {
got := GeneralHandler(xlog.New("UnitTest"), tc.input, GeneralParse)
if got != tc.err {

Check warning on line 213 in internal/linters/linters_test.go

View check run for this annotation

qiniu-x / golangci-lint

internal/linters/linters_test.go#L213

do not compare errors directly "got != tc.err", use "!errors.Is(got, tc.err)" instead (err113)
t.Errorf("handler() = %v, want %v", got, tc.err)
}
})
}
}

0 comments on commit 07962ab

Please sign in to comment.