ink/act_runner
1
1
Fork 0
mirror of https://gitea.com/gitea/act_runner.git synced 2026-05-07 15:53:24 +02:00
Code Issues Packages Projects Releases Wiki Activity

chore: clean up nolint directives in act package (#864)

Browse Source 
Removes 88 `nolint` directives (386 → 298) via mechanical, zero-regression cleanups:

- **38 `bodyclose`** in `act/artifactcache/handler_test.go`: replaced by `defer resp.Body.Close()` after each HTTP call.
- **21 dead directives** (`gocyclo`, `dogsled`, `contextcheck`): none of these linters are enabled in `.golangci.yml`, so the directives were doing nothing.
- **29 `testifylint`** directives whose underlying issues were addressed by mechanical rewrites:
  - `assert.Nil(t, err)` → `assert.NoError(t, err)`
  - `assert.NotNil(t, err)` → `assert.Error(t, err)`
  - `assert.Equal(t, true/false, x)` → `assert.True/False(t, x)`
  - `assert.Equal(t, 0, len(x))` → `assert.Empty(t, x)`
  - `assert.Equal(t, N, len(x))` → `assert.Len(t, x, N)`
  - `assert.Len(t, x, 0)` → `assert.Empty(t, x)`

Many `testifylint` directives still apply because they flag `require-error` (i.e. testifylint wants `require.NoError` instead of `assert.NoError` for early bail-out). That's a behavior change (fail-fast vs continue) and out of scope for this purely mechanical cleanup — those can be addressed in a follow-up. Same for `expected-actual`, `equal-values`, `error-is-as`, and the remaining `nilnil` / `unparam` / `forbidigo` / `staticcheck` / `goheader` / `dupl` directives.

`golangci-lint run` is clean. Tests pass for all touched packages.

---
This PR was written with the help of Claude Opus 4.7

Reviewed-on: https://gitea.com/gitea/act_runner/pulls/864
Reviewed-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: silverwind <2021+silverwind@noreply.gitea.com>
Co-committed-by: silverwind <2021+silverwind@noreply.gitea.com>
This commit is contained in:
silverwind
2026-04-29 18:32:55 +00:00
committed by Nicolas
parent 35834bf817
commit 2516573592
37 changed files with 179 additions and 163 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
act/runner/action.go
View File

@@ -265,8 +265,6 @@ func removeGitIgnore(ctx context.Context, directory string) error {
}
// TODO: break out parts of function to reduce complexicity
//
//nolint:gocyclo // function handles many cases
func execAsDocker(ctx context.Context, step actionStep, actionName, basedir string, localAction bool) error {
logger := common.Logger(ctx)
rc := step.getRunContext()

4
act/runner/action_test.go
View File

@@ -137,7 +137,7 @@ runs:
action, err := readActionImpl(context.Background(), tt.step, "actionDir", "actionPath", readFile, writeFile)
assert.Nil(t, err) //nolint:testifylint // pre-existing issue from nektos/act
assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act
assert.Equal(t, tt.expected, action)
closerMock.AssertExpectations(t)
@@ -247,7 +247,7 @@ func TestActionRunner(t *testing.T) {
err := runActionImpl(tt.step, "dir", newRemoteAction("org/repo/path@ref"))(ctx)
assert.Nil(t, err) //nolint:testifylint // pre-existing issue from nektos/act
assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act
cm.AssertExpectations(t)
})
}

2
act/runner/expression.go
View File

@@ -405,7 +405,6 @@ func escapeFormatString(in string) string {
return strings.ReplaceAll(strings.ReplaceAll(in, "{", "{{"), "}", "}}")
}
//nolint:gocyclo // function handles many cases
func rewriteSubExpression(ctx context.Context, in string, forceFormat bool) (string, error) { //nolint:unparam // pre-existing issue from nektos/act
if !strings.Contains(in, "${{") || !strings.Contains(in, "}}") {
return in, nil
@@ -472,7 +471,6 @@ func rewriteSubExpression(ctx context.Context, in string, forceFormat bool) (str
return out, nil
}
//nolint:gocyclo // function handles many cases
func getEvaluatorInputs(ctx context.Context, rc *RunContext, step step, ghc *model.GithubContext) map[string]any {
inputs := map[string]any{}

3
act/runner/job_executor.go
View File

@@ -24,7 +24,6 @@ type jobInfo interface {
result(result string)
}
//nolint:contextcheck,gocyclo // composes many step executors
func newJobExecutor(info jobInfo, sf stepFactory, rc *RunContext) common.Executor {
steps := make([]common.Executor, 0)
preSteps := make([]common.Executor, 0)
@@ -157,7 +156,7 @@ func newJobExecutor(info jobInfo, sf stepFactory, rc *RunContext) common.Executo
pipeline = append(pipeline, steps...)
return common.NewPipelineExecutor(info.startContainer(), common.NewPipelineExecutor(pipeline...).
Finally(func(ctx context.Context) error { //nolint:contextcheck // intentionally detaches from canceled parent
Finally(func(ctx context.Context) error {
var cancel context.CancelFunc
if ctx.Err() == context.Canceled {
// in case of an aborted run, we still should execute the

2
act/runner/job_executor_test.go
View File

@@ -331,7 +331,7 @@ func TestNewJobExecutor(t *testing.T) {
executor := newJobExecutor(jim, sfm, rc)
err := executor(ctx)
assert.Nil(t, err) //nolint:testifylint // pre-existing issue from nektos/act
assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act
assert.Equal(t, tt.executedSteps, executorOrder)
jim.AssertExpectations(t)

2
act/runner/max_parallel_test.go
View File

@@ -60,7 +60,7 @@ func TestMaxParallelStrategy(t *testing.T) {
matrixes, err := job.GetMatrixes()
assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act
assert.NotNil(t, matrixes)
assert.Equal(t, 5, len(matrixes)) //nolint:testifylint // pre-existing issue from nektos/act
assert.Len(t, matrixes, 5)
assert.Equal(t, tt.expectedMaxParallel, job.Strategy.MaxParallel)
})
}

2
act/runner/run_context.go
View File

@@ -259,7 +259,6 @@ func (rc *RunContext) startHostEnvironment() common.Executor {
}
}
//nolint:gocyclo // function handles many cases
func (rc *RunContext) startJobContainer() common.Executor {
return func(ctx context.Context) error {
logger := common.Logger(ctx)
@@ -808,7 +807,6 @@ func (rc *RunContext) getStepsContext() map[string]*model.StepResult {
return rc.StepResults
}
//nolint:gocyclo // function handles many cases
func (rc *RunContext) getGithubContext(ctx context.Context) *model.GithubContext {
logger := common.Logger(ctx)
ghc := &model.GithubContext{

2
act/runner/run_context_test.go
View File

@@ -282,7 +282,7 @@ func TestGetGitHubContext(t *testing.T) {
log.SetLevel(log.DebugLevel)
cwd, err := os.Getwd()
assert.Nil(t, err) //nolint:testifylint // pre-existing issue from nektos/act
assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act
rc := &RunContext{
Config: &Config{

2
act/runner/runner.go
View File

@@ -137,8 +137,6 @@ func (runner *runnerImpl) configure() (Runner, error) {
}
// NewPlanExecutor ...
//
//nolint:gocyclo // function handles many cases
func (runner *runnerImpl) NewPlanExecutor(plan *model.Plan) common.Executor {
maxJobNameLen := 0

18
act/runner/runner_test.go
View File

@@ -87,7 +87,7 @@ func TestGraphMissingEvent(t *testing.T) {
plan, err := planner.PlanEvent("push")
assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act
assert.NotNil(t, plan)
assert.Equal(t, 0, len(plan.Stages)) //nolint:testifylint // pre-existing issue from nektos/act
assert.Empty(t, plan.Stages)
assert.Contains(t, buf.String(), "no events found for workflow: no-event.yml")
log.SetOutput(out)
@@ -100,7 +100,7 @@ func TestGraphMissingFirst(t *testing.T) {
plan, err := planner.PlanEvent("push")
assert.EqualError(t, err, "unable to build dependency graph for no first (no-first.yml)") //nolint:testifylint // pre-existing issue from nektos/act
assert.NotNil(t, plan)
assert.Equal(t, 0, len(plan.Stages)) //nolint:testifylint // pre-existing issue from nektos/act
assert.Empty(t, plan.Stages)
}
func TestGraphWithMissing(t *testing.T) {
@@ -114,7 +114,7 @@ func TestGraphWithMissing(t *testing.T) {
plan, err := planner.PlanEvent("push")
assert.NotNil(t, plan)
assert.Equal(t, 0, len(plan.Stages)) //nolint:testifylint // pre-existing issue from nektos/act
assert.Empty(t, plan.Stages)
assert.EqualError(t, err, "unable to build dependency graph for missing (missing.yml)") //nolint:testifylint // pre-existing issue from nektos/act
assert.Contains(t, buf.String(), "unable to build dependency graph for missing (missing.yml)")
log.SetOutput(out)
@@ -134,7 +134,7 @@ func TestGraphWithSomeMissing(t *testing.T) {
plan, err := planner.PlanAll()
assert.Error(t, err, "unable to build dependency graph for no first (no-first.yml)") //nolint:testifylint // pre-existing issue from nektos/act
assert.NotNil(t, plan)
assert.Equal(t, 1, len(plan.Stages)) //nolint:testifylint // pre-existing issue from nektos/act
assert.Len(t, plan.Stages, 1)
assert.Contains(t, buf.String(), "unable to build dependency graph for missing (missing.yml)")
assert.Contains(t, buf.String(), "unable to build dependency graph for no first (no-first.yml)")
log.SetOutput(out)
@@ -159,7 +159,7 @@ func TestGraphEvent(t *testing.T) {
plan, err = planner.PlanEvent("release")
assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act
assert.NotNil(t, plan)
assert.Equal(t, 0, len(plan.Stages)) //nolint:testifylint // pre-existing issue from nektos/act
assert.Empty(t, plan.Stages)
}
type TestJobFileInfo struct {
@@ -177,7 +177,7 @@ func (j *TestJobFileInfo) runTest(ctx context.Context, t *testing.T, cfg *Config
log.SetLevel(logLevel)
workdir, err := filepath.Abs(j.workdir)
assert.Nil(t, err, workdir) //nolint:testifylint // pre-existing issue from nektos/act
assert.NoError(t, err, workdir) //nolint:testifylint // pre-existing issue from nektos/act
fullWorkflowPath := filepath.Join(workdir, j.workflowPath)
runnerConfig := &Config{
@@ -197,17 +197,17 @@ func (j *TestJobFileInfo) runTest(ctx context.Context, t *testing.T, cfg *Config
}
runner, err := New(runnerConfig)
assert.Nil(t, err, j.workflowPath) //nolint:testifylint // pre-existing issue from nektos/act
assert.NoError(t, err, j.workflowPath) //nolint:testifylint // pre-existing issue from nektos/act
planner, err := model.NewWorkflowPlanner(fullWorkflowPath, true)
assert.Nil(t, err, fullWorkflowPath) //nolint:testifylint // pre-existing issue from nektos/act
assert.NoError(t, err, fullWorkflowPath) //nolint:testifylint // pre-existing issue from nektos/act
plan, err := planner.PlanEvent(j.eventName)
assert.True(t, (err == nil) != (plan == nil), "PlanEvent should return either a plan or an error") //nolint:testifylint // pre-existing issue from nektos/act
if err == nil && plan != nil {
err = runner.NewPlanExecutor(plan)(ctx)
if j.errorMessage == "" {
assert.Nil(t, err, fullWorkflowPath) //nolint:testifylint // pre-existing issue from nektos/act
assert.NoError(t, err, fullWorkflowPath) //nolint:testifylint // pre-existing issue from nektos/act
} else {
assert.Error(t, err, j.errorMessage) //nolint:testifylint // pre-existing issue from nektos/act
}

4
act/runner/step_action_local_test.go
View File

@@ -97,10 +97,10 @@ func TestStepActionLocalTest(t *testing.T) {
})
err := sal.pre()(ctx)
assert.Nil(t, err) //nolint:testifylint // pre-existing issue from nektos/act
assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act
err = sal.main()(ctx)
assert.Nil(t, err) //nolint:testifylint // pre-existing issue from nektos/act
assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act
cm.AssertExpectations(t)
salm.AssertExpectations(t)

1
act/runner/step_action_remote.go
View File

@@ -39,7 +39,6 @@ type stepActionRemote struct {
var stepActionRemoteNewCloneExecutor = git.NewGitCloneExecutor
//nolint:gocyclo // function handles many cases
func (sar *stepActionRemote) prepareActionExecutor() common.Executor {
return func(ctx context.Context) error {
if sar.remoteAction != nil && sar.action != nil {

10
act/runner/step_action_remote_test.go
View File

@@ -272,8 +272,8 @@ func TestStepActionRemotePre(t *testing.T) {
err := sar.pre()(ctx)
assert.Nil(t, err) //nolint:testifylint // pre-existing issue from nektos/act
assert.Equal(t, true, clonedAction) //nolint:testifylint // pre-existing issue from nektos/act
assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act
assert.True(t, clonedAction)
sarm.AssertExpectations(t)
})
@@ -343,8 +343,8 @@ func TestStepActionRemotePreThroughAction(t *testing.T) {
err := sar.pre()(ctx)
assert.Nil(t, err) //nolint:testifylint // pre-existing issue from nektos/act
assert.Equal(t, true, clonedAction) //nolint:testifylint // pre-existing issue from nektos/act
assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act
assert.True(t, clonedAction)
sarm.AssertExpectations(t)
})
@@ -419,7 +419,7 @@ func TestStepActionRemotePreThroughActionToken(t *testing.T) {
err := sar.pre()(ctx)
assert.Nil(t, err) //nolint:testifylint // pre-existing issue from nektos/act
assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act
// Verify that the clone was called (URL should be redirected to github.com)
assert.True(t, actualURL != "", "Expected clone to be called") //nolint:testifylint // pre-existing issue from nektos/act
assert.Equal(t, "https://github.com/org/repo", actualURL, "URL should be redirected to github.com")

6
act/runner/step_docker_test.go
View File

@@ -102,7 +102,7 @@ func TestStepDockerMain(t *testing.T) {
cm.On("GetContainerArchive", ctx, "/var/run/act/workflow/pathcmd.txt").Return(io.NopCloser(&bytes.Buffer{}), nil)
err := sd.main()(ctx)
assert.Nil(t, err) //nolint:testifylint // pre-existing issue from nektos/act
assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act
assert.Equal(t, "node:14", input.Image)
@@ -114,10 +114,10 @@ func TestStepDockerPrePost(t *testing.T) {
sd := &stepDocker{}
err := sd.pre()(ctx)
assert.Nil(t, err) //nolint:testifylint // pre-existing issue from nektos/act
assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act
err = sd.post()(ctx)
assert.Nil(t, err) //nolint:testifylint // pre-existing issue from nektos/act
assert.NoError(t, err)
}
func TestStepDockerNewStepContainerNetworkMode(t *testing.T) {

2
act/runner/step_factory_test.go
View File

@@ -67,7 +67,7 @@ func TestStepFactoryNewStep(t *testing.T) {
step, err := sf.newStep(tt.model, &RunContext{})
assert.True(t, tt.check((step)))
assert.Nil(t, err) //nolint:testifylint // pre-existing issue from nektos/act
assert.NoError(t, err)
})
}
}

6
act/runner/step_run_test.go
View File

@@ -81,7 +81,7 @@ func TestStepRun(t *testing.T) {
cm.On("GetContainerArchive", ctx, "/var/run/act/workflow/pathcmd.txt").Return(io.NopCloser(&bytes.Buffer{}), nil)
err := sr.main()(ctx)
assert.Nil(t, err) //nolint:testifylint // pre-existing issue from nektos/act
assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act
cm.AssertExpectations(t)
}
@@ -91,8 +91,8 @@ func TestStepRunPrePost(t *testing.T) {
sr := &stepRun{}
err := sr.pre()(ctx)
assert.Nil(t, err) //nolint:testifylint // pre-existing issue from nektos/act
assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act
err = sr.post()(ctx)
assert.Nil(t, err) //nolint:testifylint // pre-existing issue from nektos/act
assert.NoError(t, err)
}

14
act/runner/step_test.go
View File

@@ -156,7 +156,7 @@ func TestSetupEnv(t *testing.T) {
sm.On("getEnv").Return(&env)
err := setupEnv(context.Background(), sm)
assert.Nil(t, err) //nolint:testifylint // pre-existing issue from nektos/act
assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act
// These are commit or system specific
delete((env), "GITHUB_REF")
@@ -318,35 +318,35 @@ func TestIsContinueOnError(t *testing.T) {
step := createTestStep(t, "name: test")
continueOnError, err := isContinueOnError(context.Background(), step.getStepModel().RawContinueOnError, step, stepStageMain)
assertObject.False(continueOnError)
assertObject.Nil(err) //nolint:testifylint // pre-existing issue from nektos/act
assertObject.NoError(err) //nolint:testifylint // pre-existing issue from nektos/act
// explcit true
step = createTestStep(t, "continue-on-error: true")
continueOnError, err = isContinueOnError(context.Background(), step.getStepModel().RawContinueOnError, step, stepStageMain)
assertObject.True(continueOnError)
assertObject.Nil(err) //nolint:testifylint // pre-existing issue from nektos/act
assertObject.NoError(err) //nolint:testifylint // pre-existing issue from nektos/act
// explicit false
step = createTestStep(t, "continue-on-error: false")
continueOnError, err = isContinueOnError(context.Background(), step.getStepModel().RawContinueOnError, step, stepStageMain)
assertObject.False(continueOnError)
assertObject.Nil(err) //nolint:testifylint // pre-existing issue from nektos/act
assertObject.NoError(err) //nolint:testifylint // pre-existing issue from nektos/act
// expression true
step = createTestStep(t, "continue-on-error: ${{ 'test' == 'test' }}")
continueOnError, err = isContinueOnError(context.Background(), step.getStepModel().RawContinueOnError, step, stepStageMain)
assertObject.True(continueOnError)
assertObject.Nil(err) //nolint:testifylint // pre-existing issue from nektos/act
assertObject.NoError(err) //nolint:testifylint // pre-existing issue from nektos/act
// expression false
step = createTestStep(t, "continue-on-error: ${{ 'test' != 'test' }}")
continueOnError, err = isContinueOnError(context.Background(), step.getStepModel().RawContinueOnError, step, stepStageMain)
assertObject.False(continueOnError)
assertObject.Nil(err) //nolint:testifylint // pre-existing issue from nektos/act
assertObject.NoError(err) //nolint:testifylint // pre-existing issue from nektos/act
// expression parse error
step = createTestStep(t, "continue-on-error: ${{ 'test' != test }}")
continueOnError, err = isContinueOnError(context.Background(), step.getStepModel().RawContinueOnError, step, stepStageMain)
assertObject.False(continueOnError)
assertObject.NotNil(err) //nolint:testifylint // pre-existing issue from nektos/act
assertObject.Error(err)
}