From f571290b25cdbb34e863cb98eb155369fff5d572 Mon Sep 17 00:00:00 2001 From: "Ryan (hackercat)" Date: Tue, 18 May 2021 06:14:49 +0000 Subject: [PATCH] refactor: remove `gotest.tools` (#688) * refactor: remove `gotest.tools` * remove all references to `gotest.tools` and replace it with `github.com/stretchr/testify` which was originally used for tests * bump `golangci-lint` version * add `depguard` and `importas` to prevent import of unwanted packages * add custom schema and information about config since schemastore.org has broken schema for `golangci-lint` config * fix: handle more error cases --- .github/workflows/checks.yml | 2 +- .golangci.yml | 21 ++++++++++++++++++++ cmd/input.go | 3 ++- go.mod | 1 - pkg/container/docker_pull_test.go | 6 +++--- pkg/runner/expression_test.go | 20 +++++++++---------- pkg/runner/run_context_test.go | 17 ++++++++-------- pkg/runner/runner_test.go | 32 +++++++++++++++---------------- pkg/runner/step_context.go | 12 ++++++++++-- 9 files changed, 71 insertions(+), 43 deletions(-) diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index 7d9380a..4d5e4d5 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -16,7 +16,7 @@ jobs: env: CGO_ENABLED: 0 with: - version: v1.39.0 + version: v1.40.0 - uses: github/super-linter@v3 env: DEFAULT_BRANCH: master diff --git a/.golangci.yml b/.golangci.yml index 746e63f..3304591 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,3 +1,7 @@ +# yaml-language-server: $schema=https://schemastore.pages.dev/schemas/json/golangci-lint.json +# Your editor might complain about invalid types, but this is correct config +# above schema should prevent editor from "shouting" about this +# Minimum golangci-lint version required: v1.40.0 run: timeout: 3m @@ -8,6 +12,21 @@ linters-settings: gocritic: disabled-checks: - ifElseChain + importas: + aliases: + - pkg: 'github.com/sirupsen/logrus' + alias: log + - pkg: 'github.com/stretchr/testify/assert' + alias: assert + depguard: + list-type: blacklist + include-go-root: true + packages: + - gotest.tools/v3/assert + - log + packages-with-error-message: + - gotest.tools/v3: 'Please keep tests unified using only github.com/stretchr/testify' + - log: 'Please keep logging unified using only github.com/sirupsen/logrus' linters: enable: @@ -25,3 +44,5 @@ linters: - goimports - whitespace - misspell + - depguard + - importas diff --git a/cmd/input.go b/cmd/input.go index e954287..17bff93 100644 --- a/cmd/input.go +++ b/cmd/input.go @@ -1,8 +1,9 @@ package cmd import ( - "log" "path/filepath" + + log "github.com/sirupsen/logrus" ) // Input contains the input for the root command diff --git a/go.mod b/go.mod index f10c858..6f6662a 100644 --- a/go.mod +++ b/go.mod @@ -39,5 +39,4 @@ require ( google.golang.org/grpc v1.33.2 // indirect gopkg.in/sourcemap.v1 v1.0.5 // indirect gopkg.in/yaml.v3 v3.0.0-20200615113413-eeeca48fe776 - gotest.tools/v3 v3.0.2 ) diff --git a/pkg/container/docker_pull_test.go b/pkg/container/docker_pull_test.go index a7494de..58707b7 100644 --- a/pkg/container/docker_pull_test.go +++ b/pkg/container/docker_pull_test.go @@ -5,7 +5,7 @@ import ( "testing" log "github.com/sirupsen/logrus" - "gotest.tools/v3/assert" + assert "github.com/stretchr/testify/assert" ) func init() { @@ -33,7 +33,7 @@ func TestGetImagePullOptions(t *testing.T) { ctx := context.Background() options, err := getImagePullOptions(ctx, NewDockerPullExecutorInput{}) - assert.NilError(t, err, "Failed to create ImagePullOptions") + assert.Nil(t, err, "Failed to create ImagePullOptions") assert.Equal(t, options.RegistryAuth, "", "RegistryAuth should be empty if no username or password is set") options, err = getImagePullOptions(ctx, NewDockerPullExecutorInput{ @@ -41,6 +41,6 @@ func TestGetImagePullOptions(t *testing.T) { Username: "username", Password: "password", }) - assert.NilError(t, err, "Failed to create ImagePullOptions") + assert.Nil(t, err, "Failed to create ImagePullOptions") assert.Equal(t, options.RegistryAuth, "eyJ1c2VybmFtZSI6InVzZXJuYW1lIiwicGFzc3dvcmQiOiJwYXNzd29yZCJ9", "Username and Password should be provided") } diff --git a/pkg/runner/expression_test.go b/pkg/runner/expression_test.go index 55ddff5..a338187 100644 --- a/pkg/runner/expression_test.go +++ b/pkg/runner/expression_test.go @@ -8,7 +8,7 @@ import ( "testing" "github.com/nektos/act/pkg/model" - a "github.com/stretchr/testify/assert" + assert "github.com/stretchr/testify/assert" ) func TestEvaluate(t *testing.T) { @@ -115,14 +115,14 @@ func TestEvaluate(t *testing.T) { for _, table := range tables { table := table t.Run(table.in, func(t *testing.T) { - assert := a.New(t) + assertObject := assert.New(t) out, _, err := ee.Evaluate(table.in) if table.errMesg == "" { - assert.NoError(err, table.in) - assert.Equal(table.out, out, table.in) + assertObject.NoError(err, table.in) + assertObject.Equal(table.out, out, table.in) } else { - assert.Error(err, table.in) - assert.Equal(table.errMesg, err.Error(), table.in) + assertObject.Error(err, table.in) + assertObject.Equal(table.errMesg, err.Error(), table.in) } }) } @@ -188,9 +188,9 @@ func TestInterpolate(t *testing.T) { for _, table := range tables { table := table t.Run(table.in, func(t *testing.T) { - assert := a.New(t) + assertObject := assert.New(t) out := ee.Interpolate(table.in) - assert.Equal(table.out, out, table.in) + assertObject.Equal(table.out, out, table.in) }) } } @@ -277,9 +277,9 @@ func TestRewrite(t *testing.T) { for _, table := range tables { table := table t.Run(table.in, func(t *testing.T) { - assert := a.New(t) + assertObject := assert.New(t) re := ee.Rewrite(table.in) - assert.Equal(table.re, re, table.in) + assertObject.Equal(table.re, re, table.in) }) } } diff --git a/pkg/runner/run_context_test.go b/pkg/runner/run_context_test.go index 23e3cc9..bd0b79e 100644 --- a/pkg/runner/run_context_test.go +++ b/pkg/runner/run_context_test.go @@ -10,11 +10,10 @@ import ( "testing" "github.com/nektos/act/pkg/model" - a "github.com/stretchr/testify/assert" - "gotest.tools/v3/assert" log "github.com/sirupsen/logrus" "github.com/sirupsen/logrus/hooks/test" + assert "github.com/stretchr/testify/assert" ) func TestRunContext_EvalBool(t *testing.T) { @@ -142,15 +141,15 @@ func TestRunContext_EvalBool(t *testing.T) { for _, table := range tables { table := table t.Run(table.in, func(t *testing.T) { - assert := a.New(t) + assertObject := assert.New(t) defer hook.Reset() b, err := rc.EvalBool(table.in) if table.wantErr { - assert.Error(err) + assertObject.Error(err) } - assert.Equal(table.out, b, fmt.Sprintf("Expected %s to be %v, was %v", table.in, table.out, b)) - assert.Empty(hook.LastEntry(), table.in) + assertObject.Equal(table.out, b, fmt.Sprintf("Expected %s to be %v, was %v", table.in, table.out, b)) + assertObject.Empty(hook.LastEntry(), table.in) }) } } @@ -269,10 +268,10 @@ func TestRunContext_GetBindsAndMounts(t *testing.T) { if runtime.GOOS == "darwin" { fullBind += ":delegated" } - a.Contains(t, gotbind, fullBind) + assert.Contains(t, gotbind, fullBind) } else { mountkey := testcase.rc.jobContainerName() - a.EqualValues(t, testcase.wantmount, gotmount[mountkey]) + assert.EqualValues(t, testcase.wantmount, gotmount[mountkey]) } }) } @@ -284,7 +283,7 @@ func TestGetGitHubContext(t *testing.T) { log.SetLevel(log.DebugLevel) cwd, err := os.Getwd() - assert.NilError(t, err) + assert.Nil(t, err) rc := &RunContext{ Config: &Config{ diff --git a/pkg/runner/runner_test.go b/pkg/runner/runner_test.go index 44df4ff..0aaa6f6 100644 --- a/pkg/runner/runner_test.go +++ b/pkg/runner/runner_test.go @@ -11,17 +11,17 @@ import ( "github.com/joho/godotenv" log "github.com/sirupsen/logrus" - "gotest.tools/v3/assert" + assert "github.com/stretchr/testify/assert" "github.com/nektos/act/pkg/model" ) func TestGraphEvent(t *testing.T) { planner, err := model.NewWorkflowPlanner("testdata/basic", true) - assert.NilError(t, err) + assert.Nil(t, err) plan := planner.PlanEvent("push") - assert.NilError(t, err) + assert.Nil(t, err) assert.Equal(t, len(plan.Stages), 3, "stages") assert.Equal(t, len(plan.Stages[0].Runs), 1, "stage0.runs") assert.Equal(t, len(plan.Stages[1].Runs), 1, "stage1.runs") @@ -46,7 +46,7 @@ type TestJobFileInfo struct { func runTestJobFile(ctx context.Context, t *testing.T, tjfi TestJobFileInfo) { t.Run(tjfi.workflowPath, func(t *testing.T) { workdir, err := filepath.Abs(tjfi.workdir) - assert.NilError(t, err, workdir) + assert.Nil(t, err, workdir) fullWorkflowPath := filepath.Join(workdir, tjfi.workflowPath) runnerConfig := &Config{ Workdir: workdir, @@ -59,18 +59,18 @@ func runTestJobFile(ctx context.Context, t *testing.T, tjfi TestJobFileInfo) { } runner, err := New(runnerConfig) - assert.NilError(t, err, tjfi.workflowPath) + assert.Nil(t, err, tjfi.workflowPath) planner, err := model.NewWorkflowPlanner(fullWorkflowPath, true) - assert.NilError(t, err, fullWorkflowPath) + assert.Nil(t, err, fullWorkflowPath) plan := planner.PlanEvent(tjfi.eventName) err = runner.NewPlanExecutor(plan)(ctx) if tjfi.errorMessage == "" { - assert.NilError(t, err, fullWorkflowPath) + assert.Nil(t, err, fullWorkflowPath) } else { - assert.ErrorContains(t, err, tjfi.errorMessage) + assert.Error(t, err, tjfi.errorMessage) } }) } @@ -138,7 +138,7 @@ func TestRunEventSecrets(t *testing.T) { eventName := "push" workdir, err := filepath.Abs("testdata") - assert.NilError(t, err, workflowPath) + assert.Nil(t, err, workflowPath) env, _ := godotenv.Read(filepath.Join(workdir, workflowPath, ".env")) secrets, _ := godotenv.Read(filepath.Join(workdir, workflowPath, ".secrets")) @@ -152,15 +152,15 @@ func TestRunEventSecrets(t *testing.T) { Env: env, } runner, err := New(runnerConfig) - assert.NilError(t, err, workflowPath) + assert.Nil(t, err, workflowPath) planner, err := model.NewWorkflowPlanner(fmt.Sprintf("testdata/%s", workflowPath), true) - assert.NilError(t, err, workflowPath) + assert.Nil(t, err, workflowPath) plan := planner.PlanEvent(eventName) err = runner.NewPlanExecutor(plan)(ctx) - assert.NilError(t, err, workflowPath) + assert.Nil(t, err, workflowPath) } func TestRunEventPullRequest(t *testing.T) { @@ -179,7 +179,7 @@ func TestRunEventPullRequest(t *testing.T) { eventName := "pull_request" workdir, err := filepath.Abs("testdata") - assert.NilError(t, err, workflowPath) + assert.Nil(t, err, workflowPath) runnerConfig := &Config{ Workdir: workdir, @@ -189,15 +189,15 @@ func TestRunEventPullRequest(t *testing.T) { ReuseContainers: false, } runner, err := New(runnerConfig) - assert.NilError(t, err, workflowPath) + assert.Nil(t, err, workflowPath) planner, err := model.NewWorkflowPlanner(fmt.Sprintf("testdata/%s", workflowPath), true) - assert.NilError(t, err, workflowPath) + assert.Nil(t, err, workflowPath) plan := planner.PlanEvent(eventName) err = runner.NewPlanExecutor(plan)(ctx) - assert.NilError(t, err, workflowPath) + assert.Nil(t, err, workflowPath) } func TestContainerPath(t *testing.T) { diff --git a/pkg/runner/step_context.go b/pkg/runner/step_context.go index 615cf02..8241c1d 100644 --- a/pkg/runner/step_context.go +++ b/pkg/runner/step_context.go @@ -89,11 +89,19 @@ func (sc *StepContext) Executor() common.Executor { Dir: actionDir, Token: github.Token, }) + var ntErr common.Executor if err := gitClone(context.TODO()); err != nil { - err = errors.Cause(err) - return common.NewErrorExecutor(fmt.Errorf("Unable to resolve action `%s`, the provided ref `%s` is the shortened version of a commit SHA, which is not supported. Please use the full commit SHA `%s` instead", step.Uses, remoteAction.Ref, err.Error())) + if err.Error() == "short SHA references are not supported" { + err = errors.Cause(err) + return common.NewErrorExecutor(fmt.Errorf("Unable to resolve action `%s`, the provided ref `%s` is the shortened version of a commit SHA, which is not supported. Please use the full commit SHA `%s` instead", step.Uses, remoteAction.Ref, err.Error())) + } else if err.Error() != "some refs were not updated" { + return common.NewErrorExecutor(err) + } else { + ntErr = common.NewInfoExecutor("Non-terminating error while running 'git clone': %v", err) + } } return common.NewPipelineExecutor( + ntErr, sc.setupAction(actionDir, remoteAction.Path), sc.runAction(actionDir, remoteAction.Path), )