diff --git a/internal/app/cmd/exec.go b/internal/app/cmd/exec.go index 5a9290ec..b02469ef 100644 --- a/internal/app/cmd/exec.go +++ b/internal/app/cmd/exec.go @@ -312,7 +312,7 @@ func runExecList(planner model.WorkflowPlanner, execArgs *executeArgs) error { } func runExec(ctx context.Context, execArgs *executeArgs) func(cmd *cobra.Command, args []string) error { - return func(cmd *cobra.Command, args []string) error { + return func(_ *cobra.Command, args []string) error { planner, err := model.NewWorkflowPlanner(execArgs.WorkflowsPath(), model.PlannerConfig{ Recursive: !execArgs.noWorkflowRecurse, Workflow: model.WorkflowConfig{ diff --git a/internal/eval/functions/format_test.go b/internal/eval/functions/format_test.go index 885efc49..2245e4e3 100644 --- a/internal/eval/functions/format_test.go +++ b/internal/eval/functions/format_test.go @@ -1,14 +1,13 @@ package functions import ( - "fmt" "testing" "github.com/stretchr/testify/require" ) func TestFormat(t *testing.T) { - s, err := Format("Hello {0}, you have {1} new messages", "Alice", 5) + _, err := Format("Hello {0}, you have {1} new messages", "Alice", 5) require.NoError(t, err) - fmt.Println(s) // Hello Alice, you have 5 new messages + // fmt.Println(s) // Hello Alice, you have 5 new messages } diff --git a/internal/eval/v2/evaluator.go b/internal/eval/v2/evaluator.go index 92286e2e..04109a1a 100644 --- a/internal/eval/v2/evaluator.go +++ b/internal/eval/v2/evaluator.go @@ -187,7 +187,7 @@ func (e *Evaluator) evalBinaryNodeLeft(node *exprparser.BinaryNode, left *Evalua return CreateIntermediateResult(e.Context(), ret), nil } } - return nil, nil + return nil, errors.ErrUnsupported } func (e *Evaluator) evalBinaryNodeRight(node *exprparser.BinaryNode, left *EvaluationResult, right *EvaluationResult) (*EvaluationResult, error) { diff --git a/internal/pkg/labels/labels_test.go b/internal/pkg/labels/labels_test.go index e46a27bf..262d0482 100644 --- a/internal/pkg/labels/labels_test.go +++ b/internal/pkg/labels/labels_test.go @@ -6,8 +6,8 @@ package labels import ( "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "gotest.tools/v3/assert" ) func TestParse(t *testing.T) { @@ -57,7 +57,7 @@ func TestParse(t *testing.T) { return } require.NoError(t, err) - assert.DeepEqual(t, got, tt.want) + assert.Equal(t, tt.want, got) }) } } diff --git a/pkg/artifacts/artifacts_v4.go b/pkg/artifacts/artifacts_v4.go index cb6ee1d6..94153099 100644 --- a/pkg/artifacts/artifacts_v4.go +++ b/pkg/artifacts/artifacts_v4.go @@ -134,14 +134,14 @@ func (c ArtifactContext) JSON(status int, _ ...any) { c.Resp.WriteHeader(status) } -func validateRunIDV4(ctx *ArtifactContext, rawRunID string) (any, int64, bool) { +func validateRunIDV4(ctx *ArtifactContext, rawRunID string) (int64, bool) { runID, err := strconv.ParseInt(rawRunID, 10, 64) if err != nil /* || task.Job.RunID != runID*/ { log.Error("Error runID not match") ctx.Error(http.StatusBadRequest, "run-id does not match") - return nil, 0, false + return 0, false } - return nil, runID, true + return runID, true } func RoutesV4(router *httprouter.Router, baseDir string, fsys WriteFS, rfs fs.FS) { @@ -271,7 +271,7 @@ func (r *artifactV4Routes) createArtifact(ctx *ArtifactContext) { if ok := r.parseProtbufBody(ctx, &req); !ok { return } - _, runID, ok := validateRunIDV4(ctx, req.WorkflowRunBackendId) + runID, ok := validateRunIDV4(ctx, req.WorkflowRunBackendId) if !ok { return } @@ -342,7 +342,7 @@ func (r *artifactV4Routes) finalizeArtifact(ctx *ArtifactContext) { if ok := r.parseProtbufBody(ctx, &req); !ok { return } - _, _, ok := validateRunIDV4(ctx, req.WorkflowRunBackendId) + _, ok := validateRunIDV4(ctx, req.WorkflowRunBackendId) if !ok { return } @@ -360,7 +360,7 @@ func (r *artifactV4Routes) listArtifacts(ctx *ArtifactContext) { if ok := r.parseProtbufBody(ctx, &req); !ok { return } - _, runID, ok := validateRunIDV4(ctx, req.WorkflowRunBackendId) + runID, ok := validateRunIDV4(ctx, req.WorkflowRunBackendId) if !ok { return } @@ -405,7 +405,7 @@ func (r *artifactV4Routes) getSignedArtifactURL(ctx *ArtifactContext) { if ok := r.parseProtbufBody(ctx, &req); !ok { return } - _, runID, ok := validateRunIDV4(ctx, req.WorkflowRunBackendId) + runID, ok := validateRunIDV4(ctx, req.WorkflowRunBackendId) if !ok { return } @@ -439,7 +439,7 @@ func (r *artifactV4Routes) deleteArtifact(ctx *ArtifactContext) { if ok := r.parseProtbufBody(ctx, &req); !ok { return } - _, runID, ok := validateRunIDV4(ctx, req.WorkflowRunBackendId) + runID, ok := validateRunIDV4(ctx, req.WorkflowRunBackendId) if !ok { return } diff --git a/pkg/artifacts/server_test.go b/pkg/artifacts/server_test.go index a60e6e55..fd323938 100644 --- a/pkg/artifacts/server_test.go +++ b/pkg/artifacts/server_test.go @@ -277,7 +277,7 @@ func TestArtifactFlow(t *testing.T) { func runTestJobFile(ctx context.Context, t *testing.T, tjfi TestJobFileInfo) { t.Run(tjfi.workflowPath, func(t *testing.T) { - fmt.Printf("::group::%s\n", tjfi.workflowPath) + t.Logf("::group::%s\n", tjfi.workflowPath) if err := os.RemoveAll(artifactsPath); err != nil { panic(err) @@ -317,7 +317,7 @@ func runTestJobFile(ctx context.Context, t *testing.T, tjfi TestJobFileInfo) { assert.Nil(t, plan) } - fmt.Println("::endgroup::") + t.Log("::endgroup::") }) } diff --git a/pkg/container/docker_cli_test.go b/pkg/container/docker_cli_test.go index 4e321154..568fdb29 100644 --- a/pkg/container/docker_cli_test.go +++ b/pkg/container/docker_cli_test.go @@ -302,7 +302,7 @@ func TestParseWithMacAddress(t *testing.T) { t.Fatalf("Expected an error with %v mac-address, got %v", invalidMacAddress, err) } config, hostConfig, _ := mustParse(t, validMacAddress) - fmt.Printf("MacAddress: %+v\n", hostConfig) + t.Logf("MacAddress: %+v\n", hostConfig) assert.Equal(t, "92:d0:c6:0a:29:33", config.MacAddress) //nolint:staticcheck } diff --git a/pkg/container/docker_run_test.go b/pkg/container/docker_run_test.go index bcd7e3ab..d924db43 100644 --- a/pkg/container/docker_run_test.go +++ b/pkg/container/docker_run_test.go @@ -154,7 +154,7 @@ func TestDockerExecAbort(t *testing.T) { cancel() err := <-channel - assert.ErrorIs(t, err, context.Canceled) + require.ErrorIs(t, err, context.Canceled) conn.AssertExpectations(t) client.AssertExpectations(t) @@ -231,7 +231,7 @@ func TestDockerCopyTarStreamErrorInCopyFiles(t *testing.T) { } err := cr.CopyTarStream(ctx, "/var/run/act", &bytes.Buffer{}) - assert.ErrorIs(t, err, merr) + require.ErrorIs(t, err, merr) conn.AssertExpectations(t) client.AssertExpectations(t) @@ -256,7 +256,7 @@ func TestDockerCopyTarStreamErrorInMkdir(t *testing.T) { } err := cr.CopyTarStream(ctx, "/var/run/act", &bytes.Buffer{}) - assert.ErrorIs(t, err, merr) + require.ErrorIs(t, err, merr) conn.AssertExpectations(t) client.AssertExpectations(t) diff --git a/pkg/container/docker_socket_test.go b/pkg/container/docker_socket_test.go index deb9857a..7e493941 100644 --- a/pkg/container/docker_socket_test.go +++ b/pkg/container/docker_socket_test.go @@ -20,7 +20,7 @@ func TestGetSocketAndHostWithSocket(t *testing.T) { CommonSocketLocations = originalCommonSocketLocations dockerHost := "unix:///my/docker/host.sock" socketURI := "/path/to/my.socket" - os.Setenv("DOCKER_HOST", dockerHost) + t.Setenv("DOCKER_HOST", dockerHost) // Act ret, err := GetSocketAndHost(socketURI) @@ -33,7 +33,7 @@ func TestGetSocketAndHostWithSocket(t *testing.T) { func TestGetSocketAndHostNoSocket(t *testing.T) { // Arrange dockerHost := "unix:///my/docker/host.sock" - os.Setenv("DOCKER_HOST", dockerHost) + t.Setenv("DOCKER_HOST", dockerHost) // Act ret, err := GetSocketAndHost("") @@ -64,7 +64,7 @@ func TestGetSocketAndHostDontMount(t *testing.T) { // Arrange CommonSocketLocations = originalCommonSocketLocations dockerHost := "unix:///my/docker/host.sock" - os.Setenv("DOCKER_HOST", dockerHost) + t.Setenv("DOCKER_HOST", dockerHost) // Act ret, err := GetSocketAndHost("-") diff --git a/pkg/container/host_environment_test.go b/pkg/container/host_environment_test.go index 6a800226..acf2639c 100644 --- a/pkg/container/host_environment_test.go +++ b/pkg/container/host_environment_test.go @@ -38,8 +38,7 @@ func TestCopyDir(t *testing.T) { } func TestGetContainerArchive(t *testing.T) { - dir, err := os.MkdirTemp("", "test-host-env-*") - require.NoError(t, err) + dir := t.TempDir() defer os.RemoveAll(dir) ctx := context.Background() e := &HostEnvironment{ @@ -55,7 +54,7 @@ func TestGetContainerArchive(t *testing.T) { _ = os.MkdirAll(e.ToolCache, 0700) _ = os.MkdirAll(e.ActPath, 0700) expectedContent := []byte("sdde/7sh") - err = os.WriteFile(filepath.Join(e.Path, "action.yml"), expectedContent, 0600) + err := os.WriteFile(filepath.Join(e.Path, "action.yml"), expectedContent, 0600) require.NoError(t, err) archive, err := e.GetContainerArchive(ctx, e.Path) require.NoError(t, err) diff --git a/pkg/container/linux_container_environment_extensions_test.go b/pkg/container/linux_container_environment_extensions_test.go index 98d47eb6..c0633403 100644 --- a/pkg/container/linux_container_environment_extensions_test.go +++ b/pkg/container/linux_container_environment_extensions_test.go @@ -35,18 +35,13 @@ func TestContainerPath(t *testing.T) { {fmt.Sprintf("/mnt/%v/act", rootDriveLetter), "act", rootDrive + "\\"}, } { if v.workDir != "" { - if err := os.Chdir(v.workDir); err != nil { - log.Error(err) - t.Fail() - } + t.Chdir(v.workDir) } assert.Equal(t, v.destinationPath, linuxcontainerext.ToContainerPath(v.sourcePath)) } - if err := os.Chdir(cwd); err != nil { - log.Error(err) - } + t.Chdir(cwd) } else { cwd, err := os.Getwd() if err != nil { diff --git a/pkg/runner/action_cache_test.go b/pkg/runner/action_cache_test.go index d0e0cfd4..6806a26b 100644 --- a/pkg/runner/action_cache_test.go +++ b/pkg/runner/action_cache_test.go @@ -9,6 +9,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) //nolint:gosec @@ -69,7 +70,7 @@ func TestActionCache(t *testing.T) { buf := &bytes.Buffer{} // G110: Potential DoS vulnerability via decompression bomb (gosec) _, err = io.Copy(buf, mytar) - a.NoError(err) + require.NoError(t, err) str := buf.String() a.NotEmpty(str) }) diff --git a/pkg/runner/expression.go b/pkg/runner/expression.go index fe706125..32c0929c 100644 --- a/pkg/runner/expression.go +++ b/pkg/runner/expression.go @@ -3,6 +3,7 @@ package runner import ( "bytes" "context" + "errors" "fmt" "maps" "path" @@ -431,7 +432,7 @@ func rewriteSubExpression(ctx context.Context, in string, forceFormat bool) (str if strStart > -1 { matches := strPattern.FindStringIndex(in[pos:]) if matches == nil { - panic("unclosed string.") + return "", errors.New("unclosed string.") } strStart = -1 diff --git a/pkg/runner/expression_test.go b/pkg/runner/expression_test.go index dbd714bb..a241b830 100644 --- a/pkg/runner/expression_test.go +++ b/pkg/runner/expression_test.go @@ -189,7 +189,7 @@ func TestEvaluateStep(t *testing.T) { require.NoError(t, err, table.in) assertObject.Equal(table.out, out, table.in) } else { - assertObject.Error(err, table.in) + require.Error(t, err, table.in) assertObject.Equal(table.errMesg, err.Error(), table.in) } }) diff --git a/pkg/runner/job_executor_test.go b/pkg/runner/job_executor_test.go index 291dd1c3..f4756387 100644 --- a/pkg/runner/job_executor_test.go +++ b/pkg/runner/job_executor_test.go @@ -3,7 +3,6 @@ package runner import ( "context" "errors" - "fmt" "io" "slices" "testing" @@ -240,7 +239,7 @@ func TestNewJobExecutor(t *testing.T) { for _, tt := range table { t.Run(tt.name, func(t *testing.T) { - fmt.Printf("::group::%s\n", tt.name) + t.Log("::group::%s\n", tt.name) ctx := common.WithJobErrorContainer(context.Background()) jim := &jobInfoMock{} @@ -331,7 +330,7 @@ func TestNewJobExecutor(t *testing.T) { jim.AssertExpectations(t) sfm.AssertExpectations(t) - fmt.Println("::endgroup::") + t.Log("::endgroup::") }) } } diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index 11e330ce..5ed648d4 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -620,7 +620,7 @@ func (rc *RunContext) waitForServiceContainer(c container.ExecutionsEnvironment) return func(ctx context.Context) error { sctx, cancel := context.WithTimeout(ctx, time.Minute*5) defer cancel() - health := container.HealthStarting + var health container.Health delay := time.Second for i := 0; ; i++ { health = c.GetHealth(sctx) @@ -1119,7 +1119,7 @@ func (rc *RunContext) setMainCtxVars(env map[string]string, name string, value s } } -func (rc *RunContext) withGithubEnv(ctx context.Context, github *model.GithubContext, env map[string]string) map[string]string { +func (rc *RunContext) withGithubEnv(ctx context.Context, github *model.GithubContext, env map[string]string) { env["CI"] = "true" rc.setMainCtxVars(env, "WORKFLOW", github.Workflow) rc.setMainCtxVars(env, "RUN_ATTEMPT", github.RunAttempt) @@ -1165,8 +1165,6 @@ func (rc *RunContext) withGithubEnv(ctx context.Context, github *model.GithubCon } } } - - return env } func setActionRuntimeVars(rc *RunContext, env map[string]string) { diff --git a/pkg/runner/run_context_test.go b/pkg/runner/run_context_test.go index f2cda42c..b8d0125e 100644 --- a/pkg/runner/run_context_test.go +++ b/pkg/runner/run_context_test.go @@ -160,7 +160,7 @@ func TestRunContext_EvalBool(t *testing.T) { assertObject := assert.New(t) b, err := EvalBool(context.Background(), rc.ExprEval, table.in, exprparser.DefaultStatusCheckSuccess) if table.wantErr { - assertObject.Error(err) + require.Error(t, err) } assertObject.Equal(table.out, b, "Expected %s to be %v, was %v", table.in, table.out, b) diff --git a/pkg/runner/runner_test.go b/pkg/runner/runner_test.go index 1471c2d6..1c38d347 100644 --- a/pkg/runner/runner_test.go +++ b/pkg/runner/runner_test.go @@ -6,7 +6,6 @@ import ( "context" "encoding/json" "errors" - "fmt" "io" "os" "os/exec" @@ -117,7 +116,7 @@ func TestGraphWithMissing(t *testing.T) { plan, err := planner.PlanEvent("push") assert.NotNil(t, plan) assert.Empty(t, plan.Stages) - assert.EqualError(t, err, "unable to build dependency graph for missing (missing.yml)") + require.EqualError(t, err, "unable to build dependency graph for missing (missing.yml)") assert.Contains(t, buf.String(), "unable to build dependency graph for missing (missing.yml)") log.SetOutput(out) } @@ -174,7 +173,7 @@ type TestJobFileInfo struct { } func (j *TestJobFileInfo) runTest(ctx context.Context, t *testing.T, cfg *Config) { - fmt.Printf("::group::%s\n", j.workflowPath) + t.Log("::group::%s\n", j.workflowPath) log.SetLevel(logLevel) @@ -218,7 +217,7 @@ func (j *TestJobFileInfo) runTest(ctx context.Context, t *testing.T, cfg *Config } } - fmt.Println("::endgroup::") + t.Log("::endgroup::") } type TestConfig struct { @@ -739,7 +738,7 @@ func TestMaskValues(t *testing.T) { assertNoSecret := func(text string, _ string) { found := strings.Contains(text, "composite secret") if found { - fmt.Printf("\nFound Secret in the given text:\n%s\n", text) + t.Logf("\nFound Secret in the given text:\n%s\n", text) } assert.NotContains(t, text, "composite secret") } diff --git a/pkg/runner/step.go b/pkg/runner/step.go index 60b33b12..6be25e66 100644 --- a/pkg/runner/step.go +++ b/pkg/runner/step.go @@ -110,10 +110,7 @@ func runStepExecutor(step step, stage stepStage, executor common.Executor) commo rc.StepResults[rc.CurrentStep] = stepResult } - err := setupEnv(ctx, step) - if err != nil { - return err - } + setupEnv(ctx, step) cctx := common.JobCancelContext(ctx) rc.Cancelled = cctx != nil && cctx.Err() != nil @@ -241,7 +238,7 @@ func evaluateStepTimeout(ctx context.Context, exprEval ExpressionEvaluator, step return ctx, func() {} } -func setupEnv(ctx context.Context, step step) error { +func setupEnv(ctx context.Context, step step) { rc := step.getRunContext() mergeEnv(ctx, step) @@ -264,8 +261,6 @@ func setupEnv(ctx context.Context, step step) error { } common.Logger(ctx).Debugf("setupEnv => %v", *step.getEnv()) - - return nil } func mergeEnv(ctx context.Context, step step) { diff --git a/pkg/runner/step_action_remote_test.go b/pkg/runner/step_action_remote_test.go index 2efd5ced..86bfcefb 100644 --- a/pkg/runner/step_action_remote_test.go +++ b/pkg/runner/step_action_remote_test.go @@ -237,7 +237,7 @@ func TestStepActionRemote(t *testing.T) { err = sar.main()(ctx) } - assert.ErrorIs(t, err, tt.runError) + require.ErrorIs(t, err, tt.runError) assert.Equal(t, sar.RunContext.StepResults["step"], tt.result) sarm.AssertExpectations(t) diff --git a/pkg/runner/step_run.go b/pkg/runner/step_run.go index 2ef08039..4b8dabb7 100644 --- a/pkg/runner/step_run.go +++ b/pkg/runner/step_run.go @@ -205,7 +205,7 @@ func (sr *stepRun) setupShell(ctx context.Context) { func (sr *stepRun) setupWorkingDirectory(ctx context.Context) { rc := sr.RunContext step := sr.Step - workingdirectory := "" + var workingdirectory string if step.WorkingDirectory == "" { workingdirectory = rc.Run.Job().Defaults.Run.WorkingDirectory diff --git a/pkg/runner/step_test.go b/pkg/runner/step_test.go index 6d1f0f7f..01e84573 100644 --- a/pkg/runner/step_test.go +++ b/pkg/runner/step_test.go @@ -152,8 +152,7 @@ func TestSetupEnv(t *testing.T) { sm.On("getStepModel").Return(step) sm.On("getEnv").Return(&env) - err := setupEnv(context.Background(), sm) - require.NoError(t, err) + setupEnv(context.Background(), sm) // These are commit or system specific delete((env), "GITHUB_REF")