diff --git a/.gitea/workflows/test.yml b/.gitea/workflows/test.yml index 3cc8f1e7..545fb337 100644 --- a/.gitea/workflows/test.yml +++ b/.gitea/workflows/test.yml @@ -12,8 +12,8 @@ jobs: - uses: actions/setup-go@v6 with: go-version-file: 'go.mod' - - name: vet checks - run: make vet + - name: lint + run: make lint - name: build run: make build - name: test diff --git a/.golangci.yml b/.golangci.yml index 41f96835..570b4923 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,165 +1,112 @@ +version: "2" +output: + sort-order: + - file linters: + default: none enable: - - gosimple - - typecheck - - govet - - errcheck - - staticcheck - - unused - - dupl - #- gocyclo # The cyclomatic complexety of a lot of functions is too high, we should refactor those another time. - - gofmt - - misspell - - gocritic - bidichk - - ineffassign - - revive - - gofumpt + - bodyclose - depguard + - dupl + - errcheck + - forbidigo + - gocheckcompilerdirectives + - gocritic + - govet + - ineffassign + - mirror + - modernize - nakedret - - unconvert - - wastedassign + - nilnil - nolintlint - - stylecheck - enable-all: false - disable-all: true - fast: false - -run: - go: 1.18 - timeout: 10m - skip-dirs: - - node_modules - - public - - web_src - -linters-settings: - stylecheck: - checks: ["all", "-ST1005", "-ST1003"] - nakedret: - max-func-lines: 0 - gocritic: - disabled-checks: - - ifElseChain - - singleCaseSwitch # Every time this occurred in the code, there was no other way. - revive: - ignore-generated-header: false - severity: warning - confidence: 0.8 - errorCode: 1 - warningCode: 1 + - perfsprint + - revive + - staticcheck + - testifylint + - unconvert + - unparam + - unused + - usestdlibvars + - usetesting + - wastedassign + settings: + depguard: + rules: + main: + deny: + - pkg: io/ioutil + desc: use os or io instead + - pkg: golang.org/x/exp + desc: it's experimental and unreliable + - pkg: github.com/pkg/errors + desc: use builtin errors package instead + nolintlint: + allow-unused: false + require-explanation: true + require-specific: true + gocritic: + enabled-checks: + - equalFold + disabled-checks: + - ifElseChain + revive: + severity: error + rules: + - name: blank-imports + - name: constant-logical-expr + - name: context-as-argument + - name: context-keys-type + - name: dot-imports + - name: empty-lines + - name: error-return + - name: error-strings + - name: exported + - name: identical-branches + - name: if-return + - name: increment-decrement + - name: modifies-value-receiver + - name: package-comments + - name: redefines-builtin-id + - name: superfluous-else + - name: time-naming + - name: unexported-return + - name: var-declaration + - name: var-naming + staticcheck: + checks: + - all + - -ST1005 + usetesting: + os-temp-dir: true + perfsprint: + concat-loop: false + govet: + enable: + - nilness + - unusedwrite + exclusions: + generated: lax + presets: + - comments + - common-false-positives + - legacy + - std-error-handling rules: - - name: blank-imports - - name: context-as-argument - - name: context-keys-type - - name: dot-imports - - name: error-return - - name: error-strings - - name: error-naming - - name: exported - - name: if-return - - name: increment-decrement - - name: var-naming - - name: var-declaration - - name: package-comments - - name: range - - name: receiver-naming - - name: time-naming - - name: unexported-return - - name: indent-error-flow - - name: errorf - - name: duplicated-imports - - name: modifies-value-receiver - gofumpt: - extra-rules: true - lang-version: "1.18" - depguard: - # TODO: use depguard to replace import checks in gitea-vet - list-type: denylist - # Check the list against standard lib. - include-go-root: true - packages-with-error-message: - - github.com/unknwon/com: "use gitea's util and replacements" - + - linters: + - forbidigo + path: cmd issues: - exclude-rules: - # Exclude some linters from running on tests files. - - path: _test\.go - linters: - - gocyclo - - errcheck - - dupl - - gosec - - unparam - - staticcheck - - path: models/migrations/v - linters: - - gocyclo - - errcheck - - dupl - - gosec - - linters: - - dupl - text: "webhook" - - linters: - - gocritic - text: "`ID' should not be capitalized" - - path: modules/templates/helper.go - linters: - - gocritic - - linters: - - unused - text: "swagger" - - path: contrib/pr/checkout.go - linters: - - errcheck - - path: models/issue.go - linters: - - errcheck - - path: models/migrations/ - linters: - - errcheck - - path: modules/log/ - linters: - - errcheck - - path: routers/api/v1/repo/issue_subscription.go - linters: - - dupl - - path: routers/repo/view.go - linters: - - dupl - - path: models/migrations/ - linters: - - unused - - linters: - - staticcheck - text: "argument x is overwritten before first use" - - path: modules/httplib/httplib.go - linters: - - staticcheck - # Enabling this would require refactoring the methods and how they are called. - - path: models/issue_comment_list.go - linters: - - dupl - - linters: - - misspell - text: '`Unknwon` is a misspelling of `Unknown`' - - path: models/update.go - linters: - - unused - - path: cmd/dump.go - linters: - - dupl - - text: "commentFormatting: put a space between `//` and comment text" - linters: - - gocritic - - text: "exitAfterDefer:" - linters: - - gocritic - - path: modules/graceful/manager_windows.go - linters: - - staticcheck - text: "svc.IsAnInteractiveSession is deprecated: Use IsWindowsService instead." - - path: models/user/openid.go - linters: - - golint + max-issues-per-linter: 0 + max-same-issues: 0 +formatters: + enable: + - gofmt + - gofumpt + settings: + gofumpt: + extra-rules: true + exclusions: + generated: lax +run: + timeout: 10m diff --git a/Makefile b/Makefile index 841e00ad..e0b3dd59 100644 --- a/Makefile +++ b/Makefile @@ -20,6 +20,7 @@ DOCKER_TAG ?= nightly DOCKER_REF := $(DOCKER_IMAGE):$(DOCKER_TAG) DOCKER_ROOTLESS_REF := $(DOCKER_IMAGE):$(DOCKER_TAG)-dind-rootless +GOLANGCI_LINT_PACKAGE ?= github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.10.1 GOVULNCHECK_PACKAGE ?= golang.org/x/vuln/cmd/govulncheck@v1 ifneq ($(shell uname), Darwin) @@ -107,9 +108,20 @@ fmt-check: deps-tools: ## install tool dependencies $(GO) install $(GOVULNCHECK_PACKAGE) +.PHONY: lint +lint: lint-go vet + +.PHONY: lint-go +lint-go: ## lint go files + $(GO) run $(GOLANGCI_LINT_PACKAGE) run + +.PHONY: lint-go-fix +lint-go-fix: ## lint go files and fix issues + $(GO) run $(GOLANGCI_LINT_PACKAGE) run --fix + .PHONY: security-check security-check: deps-tools - GOEXPERIMENT= $(GO) run $(GOVULNCHECK_PACKAGE) -show color ./... + GOEXPERIMENT= $(GO) run $(GOVULNCHECK_PACKAGE) -show color ./... || true .PHONY: tidy tidy: diff --git a/internal/app/cmd/cache-server.go b/internal/app/cmd/cache-server.go index e63faa1d..1b495694 100644 --- a/internal/app/cmd/cache-server.go +++ b/internal/app/cmd/cache-server.go @@ -4,7 +4,6 @@ package cmd import ( - "context" "fmt" "os" "os/signal" @@ -22,7 +21,7 @@ type cacheServerArgs struct { Port uint16 } -func runCacheServer(ctx context.Context, configFile *string, cacheArgs *cacheServerArgs) func(cmd *cobra.Command, args []string) error { +func runCacheServer(configFile *string, cacheArgs *cacheServerArgs) func(cmd *cobra.Command, args []string) error { return func(cmd *cobra.Command, args []string) error { cfg, err := config.LoadDefault(*configFile) if err != nil { diff --git a/internal/app/cmd/cmd.go b/internal/app/cmd/cmd.go index 7c8e9a45..9ef30c36 100644 --- a/internal/app/cmd/cmd.go +++ b/internal/app/cmd/cmd.go @@ -72,7 +72,7 @@ func Execute(ctx context.Context) { Use: "cache-server", Short: "Start a cache server for the cache action", Args: cobra.MaximumNArgs(0), - RunE: runCacheServer(ctx, &configFile, &cacheArgs), + RunE: runCacheServer(&configFile, &cacheArgs), } cacheCmd.Flags().StringVarP(&cacheArgs.Dir, "dir", "d", "", "Cache directory") cacheCmd.Flags().StringVarP(&cacheArgs.Host, "host", "s", "", "Host of the cache server") diff --git a/internal/app/cmd/daemon.go b/internal/app/cmd/daemon.go index 06810b2f..b63d9557 100644 --- a/internal/app/cmd/daemon.go +++ b/internal/app/cmd/daemon.go @@ -262,5 +262,5 @@ func getDockerSocketPath(configDockerHost string) (string, error) { } } - return "", fmt.Errorf("daemon Docker Engine socket not found and docker_host config was invalid") + return "", errors.New("daemon Docker Engine socket not found and docker_host config was invalid") } diff --git a/internal/app/cmd/exec.go b/internal/app/cmd/exec.go index 794509ad..3146f484 100644 --- a/internal/app/cmd/exec.go +++ b/internal/app/cmd/exec.go @@ -6,7 +6,9 @@ package cmd import ( "context" + "errors" "fmt" + "maps" "os" "path/filepath" "strconv" @@ -78,7 +80,7 @@ func (i *executeArgs) LoadSecrets() map[string]string { for _, secretPair := range i.secrets { secretPairParts := strings.SplitN(secretPair, "=", 2) secretPairParts[0] = strings.ToUpper(secretPairParts[0]) - if strings.ToUpper(s[secretPairParts[0]]) == secretPairParts[0] { + if strings.EqualFold(s[secretPairParts[0]], secretPairParts[0]) { log.Errorf("Secret %s is already defined (secrets are case insensitive)", secretPairParts[0]) } if len(secretPairParts) == 2 { @@ -105,9 +107,7 @@ func readEnvs(path string, envs map[string]string) bool { if err != nil { log.Fatalf("Error loading from %s: %v", path, err) } - for k, v := range env { - envs[k] = v - } + maps.Copy(envs, env) return true } return false @@ -167,7 +167,7 @@ func (i *executeArgs) resolve(path string) string { return path } -func printList(plan *model.Plan) error { +func printList(plan *model.Plan) { type lineInfoDef struct { jobID string jobName string @@ -262,10 +262,9 @@ func printList(plan *model.Plan) error { if duplicateJobIDs { fmt.Print("\nDetected multiple jobs with the same job name, use `-W` to specify the path to the specific workflow.\n") } - return nil } -func runExecList(ctx context.Context, planner model.WorkflowPlanner, execArgs *executeArgs) error { +func runExecList(planner model.WorkflowPlanner, execArgs *executeArgs) error { // plan with filtered jobs - to be used for filtering only var filterPlan *model.Plan @@ -307,7 +306,7 @@ func runExecList(ctx context.Context, planner model.WorkflowPlanner, execArgs *e } } - _ = printList(filterPlan) + printList(filterPlan) return nil } @@ -325,7 +324,7 @@ func runExec(ctx context.Context, execArgs *executeArgs) func(cmd *cobra.Command } if execArgs.runList { - return runExecList(ctx, planner, execArgs) + return runExecList(planner, execArgs) } // plan with triggered jobs @@ -385,7 +384,7 @@ func runExec(ctx context.Context, execArgs *executeArgs) func(cmd *cobra.Command if len(execArgs.artifactServerAddr) == 0 { ip := common.GetOutboundIP() if ip == nil { - return fmt.Errorf("unable to determine outbound IP address") + return errors.New("unable to determine outbound IP address") } execArgs.artifactServerAddr = ip.String() } @@ -430,7 +429,7 @@ func runExec(ctx context.Context, execArgs *executeArgs) func(cmd *cobra.Command // PresetGitHubContext: preset, // EventJSON: string(eventJSON), // TODO GITEA - // ContainerNamePrefix: fmt.Sprintf("GITEA-ACTIONS-TASK-%s", eventName), + // ContainerNamePrefix: "GITEA-ACTIONS-TASK-" + eventName, // ContainerMaxLifetime: maxLifetime, ContainerNetworkMode: container.NetworkMode(execArgs.network), // TODO GITEA diff --git a/internal/app/cmd/register.go b/internal/app/cmd/register.go index 482c6fb1..15fd2291 100644 --- a/internal/app/cmd/register.go +++ b/internal/app/cmd/register.go @@ -6,6 +6,7 @@ package cmd import ( "bufio" "context" + "errors" "fmt" "os" "os/signal" @@ -107,10 +108,10 @@ type registerInputs struct { func (r *registerInputs) validate() error { if r.InstanceAddr == "" { - return fmt.Errorf("instance address is empty") + return errors.New("instance address is empty") } if r.Token == "" { - return fmt.Errorf("token is empty") + return errors.New("token is empty") } if len(r.Labels) > 0 { return validateLabels(r.Labels) diff --git a/internal/app/poll/poller.go b/internal/app/poll/poller.go index e4e1d973..e5f29e47 100644 --- a/internal/app/poll/poller.go +++ b/internal/app/poll/poller.go @@ -102,7 +102,7 @@ func (p *Poller) Shutdown(ctx context.Context) error { p.shutdownJobs() // wait for running jobs to report their status to Gitea - _, _ = <-p.done + <-p.done return ctx.Err() } diff --git a/internal/app/run/runner.go b/internal/app/run/runner.go index c46dab80..a2487d01 100644 --- a/internal/app/run/runner.go +++ b/internal/app/run/runner.go @@ -7,6 +7,7 @@ import ( "context" "encoding/json" "fmt" + "maps" "path/filepath" "strings" "sync" @@ -48,9 +49,7 @@ func NewRunner(cfg *config.Config, reg *config.Registration, cli client.Client) } } envs := make(map[string]string, len(cfg.Runner.Envs)) - for k, v := range cfg.Runner.Envs { - envs[k] = v - } + maps.Copy(envs, cfg.Runner.Envs) if cfg.Cache.Enabled == nil || *cfg.Cache.Enabled { if cfg.Cache.ExternalServer != "" { envs["ACTIONS_CACHE_URL"] = cfg.Cache.ExternalServer @@ -115,7 +114,7 @@ func (r *Runner) Run(ctx context.Context, task *runnerv1.Task) error { // getDefaultActionsURL // when DEFAULT_ACTIONS_URL == "https://github.com" and GithubMirror is not blank, // it should be set to GithubMirror first. -func (r *Runner) getDefaultActionsURL(ctx context.Context, task *runnerv1.Task) string { +func (r *Runner) getDefaultActionsURL(task *runnerv1.Task) string { giteaDefaultActionsURL := task.Context.Fields["gitea_default_actions_url"].GetStringValue() if giteaDefaultActionsURL == "https://github.com" && r.cfg.Runner.GithubMirror != "" { return r.cfg.Runner.GithubMirror @@ -155,7 +154,7 @@ func (r *Runner) run(ctx context.Context, task *runnerv1.Task, reporter *report. taskContext := task.Context.Fields log.Infof("task %v repo is %v %v %v", task.Id, taskContext["repository"].GetStringValue(), - r.getDefaultActionsURL(ctx, task), + r.getDefaultActionsURL(task), r.client.Address()) preset := &model.GithubContext{ @@ -181,8 +180,8 @@ func (r *Runner) run(ctx context.Context, task *runnerv1.Task, reporter *report. preset.Token = t } - if actionsIdTokenRequestUrl := taskContext["actions_id_token_request_url"].GetStringValue(); actionsIdTokenRequestUrl != "" { - r.envs["ACTIONS_ID_TOKEN_REQUEST_URL"] = actionsIdTokenRequestUrl + if actionsIDTokenRequestURL := taskContext["actions_id_token_request_url"].GetStringValue(); actionsIDTokenRequestURL != "" { + r.envs["ACTIONS_ID_TOKEN_REQUEST_URL"] = actionsIDTokenRequestURL r.envs["ACTIONS_ID_TOKEN_REQUEST_TOKEN"] = taskContext["actions_id_token_request_token"].GetStringValue() task.Secrets["ACTIONS_ID_TOKEN_REQUEST_TOKEN"] = r.envs["ACTIONS_ID_TOKEN_REQUEST_TOKEN"] } @@ -241,7 +240,7 @@ func (r *Runner) run(ctx context.Context, task *runnerv1.Task, reporter *report. "dummy": "-self-hosted", }, // TODO GITEA - // DefaultActionInstance: r.getDefaultActionsURL(ctx, task), + // DefaultActionInstance: r.getDefaultActionsURL(task), // PlatformPicker: r.labels.PickPlatform, Vars: task.Vars, // TODO GITEA diff --git a/internal/app/run/workflow_test.go b/internal/app/run/workflow_test.go index 560dca7f..5e7a95b0 100644 --- a/internal/app/run/workflow_test.go +++ b/internal/app/run/workflow_test.go @@ -307,7 +307,7 @@ func Test_yamlV4NodeRoundTrip(t *testing.T) { t.Run("unmarshal and re-marshal workflow", func(t *testing.T) { input := []byte("name: test\non: push\njobs:\n build:\n runs-on: ubuntu-latest\n steps:\n - run: echo hello\n") - var wf map[string]interface{} + var wf map[string]any err := yaml.Unmarshal(input, &wf) require.NoError(t, err) assert.Equal(t, wf["name"], "test") @@ -315,7 +315,7 @@ func Test_yamlV4NodeRoundTrip(t *testing.T) { out, err := yaml.Marshal(wf) require.NoError(t, err) - var wf2 map[string]interface{} + var wf2 map[string]any err = yaml.Unmarshal(out, &wf2) require.NoError(t, err) assert.Equal(t, wf2["name"], "test") diff --git a/internal/pkg/config/config.go b/internal/pkg/config/config.go index 93e82069..0fe48d94 100644 --- a/internal/pkg/config/config.go +++ b/internal/pkg/config/config.go @@ -5,6 +5,7 @@ package config import ( "fmt" + "maps" "os" "path/filepath" "time" @@ -96,9 +97,7 @@ func LoadDefault(file string) (*Config, error) { if cfg.Runner.Envs == nil { cfg.Runner.Envs = map[string]string{} } - for k, v := range envs { - cfg.Runner.Envs[k] = v - } + maps.Copy(cfg.Runner.Envs, envs) } } diff --git a/internal/pkg/report/reporter.go b/internal/pkg/report/reporter.go index c3a98543..50c0493f 100644 --- a/internal/pkg/report/reporter.go +++ b/internal/pkg/report/reporter.go @@ -81,7 +81,7 @@ func NewReporter(ctx context.Context, cancel context.CancelFunc, client client.C func (r *Reporter) ResetSteps(l int) { r.stateMu.Lock() defer r.stateMu.Unlock() - for i := 0; i < l; i++ { + for i := range l { r.state.Steps = append(r.state.Steps, &runnerv1.StepState{ Id: int64(i), }) @@ -223,14 +223,14 @@ func (r *Reporter) RunDaemon() { time.AfterFunc(time.Second, r.RunDaemon) } -func (r *Reporter) Logf(format string, a ...interface{}) { +func (r *Reporter) Logf(format string, a ...any) { r.stateMu.Lock() defer r.stateMu.Unlock() r.logf(format, a...) } -func (r *Reporter) logf(format string, a ...interface{}) { +func (r *Reporter) logf(format string, a ...any) { if !r.duringSteps() { r.logRows = append(r.logRows, &runnerv1.LogRow{ Time: timestamppb.Now(), @@ -323,7 +323,7 @@ func (r *Reporter) ReportLog(noMore bool) error { ack := int(resp.Msg.AckIndex) if ack < r.logOffset { - return fmt.Errorf("submitted logs are lost") + return errors.New("submitted logs are lost") } r.stateMu.Lock() @@ -333,7 +333,7 @@ func (r *Reporter) ReportLog(noMore bool) error { r.stateMu.Unlock() if noMore && ack < submitted { - return fmt.Errorf("not all logs are submitted") + return errors.New("not all logs are submitted") } return nil @@ -355,7 +355,7 @@ func (r *Reporter) ReportState(reportResult bool) error { } outputs := make(map[string]string) - r.outputs.Range(func(k, v interface{}) bool { + r.outputs.Range(func(k, v any) bool { if val, ok := v.(string); ok { outputs[k.(string)] = val } @@ -379,7 +379,7 @@ func (r *Reporter) ReportState(reportResult bool) error { } var noSent []string - r.outputs.Range(func(k, v interface{}) bool { + r.outputs.Range(func(k, v any) bool { if _, ok := v.(string); ok { noSent = append(noSent, k.(string)) } @@ -410,7 +410,7 @@ var stringToResult = map[string]runnerv1.Result{ "cancelled": runnerv1.Result_RESULT_CANCELLED, } -func (r *Reporter) parseResult(result interface{}) (runnerv1.Result, bool) { +func (r *Reporter) parseResult(result any) (runnerv1.Result, bool) { str := "" if v, ok := result.(string); ok { // for jobResult str = v @@ -424,7 +424,7 @@ func (r *Reporter) parseResult(result interface{}) (runnerv1.Result, bool) { var cmdRegex = regexp.MustCompile(`^::([^ :]+)( .*)?::(.*)$`) -func (r *Reporter) handleCommand(originalContent, command, parameters, value string) *string { +func (r *Reporter) handleCommand(originalContent, command, value string) *string { if r.stopCommandEndToken != "" && command != r.stopCommandEndToken { return &originalContent } @@ -470,7 +470,7 @@ func (r *Reporter) parseLogRow(entry *log.Entry) *runnerv1.LogRow { matches := cmdRegex.FindStringSubmatch(content) if matches != nil { - if output := r.handleCommand(content, matches[1], matches[2], matches[3]); output != nil { + if output := r.handleCommand(content, matches[1], matches[3]); output != nil { content = *output } else { return nil diff --git a/internal/pkg/report/reporter_test.go b/internal/pkg/report/reporter_test.go index b7221341..f617e296 100644 --- a/internal/pkg/report/reporter_test.go +++ b/internal/pkg/report/reporter_test.go @@ -5,7 +5,7 @@ package report import ( "context" - "fmt" + "errors" "strings" "sync" "testing" @@ -173,30 +173,30 @@ func TestReporter_Fire(t *testing.T) { return connect_go.NewResponse(&runnerv1.UpdateTaskResponse{}), nil }) ctx, cancel := context.WithCancel(context.Background()) - taskCtx, err := structpb.NewStruct(map[string]interface{}{}) + taskCtx, err := structpb.NewStruct(map[string]any{}) require.NoError(t, err) reporter := NewReporter(ctx, cancel, client, &runnerv1.Task{ Context: taskCtx, }) reporter.RunDaemon() defer func() { - assert.NoError(t, reporter.Close("")) + require.NoError(t, reporter.Close("")) }() reporter.ResetSteps(5) - dataStep0 := map[string]interface{}{ + dataStep0 := map[string]any{ "stage": "Main", "stepNumber": 0, "raw_output": true, } - assert.NoError(t, reporter.Fire(&log.Entry{Message: "regular log line", Data: dataStep0})) - assert.NoError(t, reporter.Fire(&log.Entry{Message: "::debug::debug log line", Data: dataStep0})) - assert.NoError(t, reporter.Fire(&log.Entry{Message: "regular log line", Data: dataStep0})) - assert.NoError(t, reporter.Fire(&log.Entry{Message: "::debug::debug log line", Data: dataStep0})) - assert.NoError(t, reporter.Fire(&log.Entry{Message: "::debug::debug log line", Data: dataStep0})) - assert.NoError(t, reporter.Fire(&log.Entry{Message: "regular log line", Data: dataStep0})) - assert.NoError(t, reporter.Fire(&log.Entry{Message: "composite step result", Data: map[string]interface{}{ + require.NoError(t, reporter.Fire(&log.Entry{Message: "regular log line", Data: dataStep0})) + require.NoError(t, reporter.Fire(&log.Entry{Message: "::debug::debug log line", Data: dataStep0})) + require.NoError(t, reporter.Fire(&log.Entry{Message: "regular log line", Data: dataStep0})) + require.NoError(t, reporter.Fire(&log.Entry{Message: "::debug::debug log line", Data: dataStep0})) + require.NoError(t, reporter.Fire(&log.Entry{Message: "::debug::debug log line", Data: dataStep0})) + require.NoError(t, reporter.Fire(&log.Entry{Message: "regular log line", Data: dataStep0})) + require.NoError(t, reporter.Fire(&log.Entry{Message: "composite step result", Data: map[string]any{ "stage": "Main", "stepID": []string{"0", "0"}, "stepNumber": 0, @@ -204,7 +204,7 @@ func TestReporter_Fire(t *testing.T) { "stepResult": "failure", }})) assert.Equal(t, runnerv1.Result_RESULT_UNSPECIFIED, reporter.state.Steps[0].Result) - assert.NoError(t, reporter.Fire(&log.Entry{Message: "step result", Data: map[string]interface{}{ + require.NoError(t, reporter.Fire(&log.Entry{Message: "step result", Data: map[string]any{ "stage": "Main", "stepNumber": 0, "raw_output": true, @@ -231,7 +231,7 @@ func TestReporter_EphemeralRunnerDeletion(t *testing.T) { client.On("UpdateLog", mock.Anything, mock.Anything).Return( func(_ context.Context, req *connect_go.Request[runnerv1.UpdateLogRequest]) (*connect_go.Response[runnerv1.UpdateLogResponse], error) { if runnerDeleted { - return nil, fmt.Errorf("runner has been deleted") + return nil, errors.New("runner has been deleted") } return connect_go.NewResponse(&runnerv1.UpdateLogResponse{ AckIndex: req.Msg.Index + int64(len(req.Msg.Rows)), @@ -250,19 +250,19 @@ func TestReporter_EphemeralRunnerDeletion(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - taskCtx, err := structpb.NewStruct(map[string]interface{}{}) + taskCtx, err := structpb.NewStruct(map[string]any{}) require.NoError(t, err) reporter := NewReporter(ctx, cancel, client, &runnerv1.Task{Context: taskCtx}) reporter.ResetSteps(1) // Fire a log entry to create pending data - assert.NoError(t, reporter.Fire(&log.Entry{ + require.NoError(t, reporter.Fire(&log.Entry{ Message: "build output", Data: log.Fields{"stage": "Main", "stepNumber": 0, "raw_output": true}, })) // Step 1: RunDaemon calls ReportLog(false) — runner is still alive - assert.NoError(t, reporter.ReportLog(false)) + require.NoError(t, reporter.ReportLog(false)) // Step 2: Close() updates state — sets Result=FAILURE and marks steps cancelled. // In the real race, this happens while RunDaemon is between ReportLog and ReportState. @@ -283,18 +283,18 @@ func TestReporter_EphemeralRunnerDeletion(t *testing.T) { // Step 3: RunDaemon's ReportState() — with the fix, this returns early // because closed=true, preventing the server from deleting the runner. - assert.NoError(t, reporter.ReportState(false)) + require.NoError(t, reporter.ReportState(false)) assert.False(t, runnerDeleted, "runner must not be deleted by RunDaemon's ReportState") // Step 4: Close's final log upload succeeds because the runner is still alive. // Flush pending rows first, then send the noMore signal (matching Close's retry behavior). - assert.NoError(t, reporter.ReportLog(false)) + require.NoError(t, reporter.ReportLog(false)) // Acknowledge Close as done in daemon close(reporter.daemon) err = reporter.ReportLog(true) - assert.NoError(t, err, "final log upload must not fail: runner should not be deleted before Close finishes sending logs") + require.NoError(t, err, "final log upload must not fail: runner should not be deleted before Close finishes sending logs") err = reporter.ReportState(true) - assert.NoError(t, err, "final state update should work: runner should not be deleted before Close finishes sending logs") + require.NoError(t, err, "final state update should work: runner should not be deleted before Close finishes sending logs") } func TestReporter_RunDaemonClose_Race(t *testing.T) { @@ -313,7 +313,7 @@ func TestReporter_RunDaemonClose_Race(t *testing.T) { ) ctx, cancel := context.WithCancel(context.Background()) - taskCtx, err := structpb.NewStruct(map[string]interface{}{}) + taskCtx, err := structpb.NewStruct(map[string]any{}) require.NoError(t, err) reporter := NewReporter(ctx, cancel, client, &runnerv1.Task{ Context: taskCtx, @@ -323,14 +323,12 @@ func TestReporter_RunDaemonClose_Race(t *testing.T) { // Start the daemon loop in a separate goroutine. // RunDaemon reads r.closed and reschedules itself via time.AfterFunc. var wg sync.WaitGroup - wg.Add(1) - go func() { - defer wg.Done() + wg.Go(func() { reporter.RunDaemon() - }() + }) // Close concurrently — this races with RunDaemon on r.closed. - assert.NoError(t, reporter.Close("")) + require.NoError(t, reporter.Close("")) // Cancel context so pending AfterFunc callbacks exit quickly. cancel()