Skip to content

Commit

Permalink
refactor: remove gotest.tools (#688)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
Ryan (hackercat) authored May 18, 2021
1 parent 3e22b1b commit f571290
Show file tree
Hide file tree
Showing 9 changed files with 71 additions and 43 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 21 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -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

Expand All @@ -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:
Expand All @@ -25,3 +44,5 @@ linters:
- goimports
- whitespace
- misspell
- depguard
- importas
3 changes: 2 additions & 1 deletion cmd/input.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
package cmd

import (
"log"
"path/filepath"

log "github.com/sirupsen/logrus"
)

// Input contains the input for the root command
Expand Down
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
6 changes: 3 additions & 3 deletions pkg/container/docker_pull_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (
"testing"

log "github.com/sirupsen/logrus"
"gotest.tools/v3/assert"
assert "github.com/stretchr/testify/assert"
)

func init() {
Expand Down Expand Up @@ -33,14 +33,14 @@ 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{
Image: "",
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")
}
20 changes: 10 additions & 10 deletions pkg/runner/expression_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
}
})
}
Expand Down Expand Up @@ -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)
})
}
}
Expand Down Expand Up @@ -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)
})
}
}
17 changes: 8 additions & 9 deletions pkg/runner/run_context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
})
}
}
Expand Down Expand Up @@ -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])
}
})
}
Expand All @@ -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{
Expand Down
32 changes: 16 additions & 16 deletions pkg/runner/runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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,
Expand All @@ -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)
}
})
}
Expand Down Expand Up @@ -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"))
Expand All @@ -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) {
Expand All @@ -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,
Expand All @@ -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) {
Expand Down
12 changes: 10 additions & 2 deletions pkg/runner/step_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
)
Expand Down

0 comments on commit f571290

Please sign in to comment.