Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use go-cmp instead of reflect.DeepEqual #155

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .github/workflows/go.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
go: [ 1.16.x, 1.15.x, 1.14.x, 1.13.x, 1.12.x, 1.11.x, 1.10.x, 1.9.x ]
go: [ 1.17.x, 1.16.x, 1.15.x, 1.14.x]
env:
GOPATH: ${{ github.workspace }}
GO111MODULE: auto
Expand All @@ -35,6 +35,9 @@ jobs:
- name: Get Dependencies (github.com/golang.org/x/tools/imports)
run: go get -v ./...

- name: Get go-cmp
run: go get github.com/google/go-cmp/cmp
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to manually fetch this package? If a user forgets to do this will gotests not work for them? IMO go get -v ./... should cover installing this dependency.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this explicit call the tests will fail in GitHub Actions for Go 1.9 - 1.12:

Run go test -v ./...
  go test -v ./...
  shell: /usr/bin/bash -e {0}
  env:
    GOPATH: /home/runner/work/gotests/gotests
    GO111MODULE: auto
    GOROOT: /opt/hostedtoolcache/go/1.12.17/x64
# github.com/cweill/gotests
FAIL	github.com/cweill/gotests [setup failed]
gotests_test.go:16:2: cannot find package "github.com/google/go-cmp/cmp" in any of:
	/opt/hostedtoolcache/go/1.12.17/x64/src/github.com/google/go-cmp/cmp (from $GOROOT)
	/home/runner/work/gotests/gotests/src/github.com/google/go-cmp/cmp (from $GOPATH)
FAIL	github.com/cweill/gotests/gotests [setup failed]

Other solutions to make this work may exist. The GO111MODULE env var could be set to on, but that'll likely only work for Go 1.11 and up. To me this seemed the easiest solution to make it work across versions without having to make the CI configuration more complex.

I tried the go-cmp version of gotests locally on a sample project. It resulted in a _test.go file with github.com/google/go-cmp/cmp in the import section. Running the tests for the first time results in go-cmp being inserted into go.mod. The go.mod of gotests itself does contain go-cmp, so that should work correctly too.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My concern is for who don't have go-cmp installed, will this change break them unless they manually install the package? Is there any way we can install it automatically when they install gotests?


- name: Test
run: |
go test -v ./...
Expand Down
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ Available options:

-i print test inputs in error messages

-named switch table tests from using slice to map (with test name for the key)

-only regexp. generate tests for functions and methods that match only.
Takes precedence over -all

Expand All @@ -58,6 +60,8 @@ Available options:
-template_params_file read external parameters to template by json with file

-template_params read external parameters to template by json with stdin

-use_go_cmp use cmp.Equal (google/go-cmp) instead of reflect.DeepEqual
```

## Contributions
Expand Down
5 changes: 4 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
module github.com/cweill/gotests

require golang.org/x/tools v0.0.0-20191109212701-97ad0ed33101
require (
github.com/google/go-cmp v0.5.7
golang.org/x/tools v0.0.0-20191109212701-97ad0ed33101
)

go 1.6
4 changes: 4 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
github.com/google/go-cmp v0.5.7 h1:81/ik6ipDQS2aGcBfIN5dHDB36BwrStyeAQquSYCV4o=
github.com/google/go-cmp v0.5.7/go.mod h1:n+brtR0CgQNWTVd5ZUFpTBC8YFBDLK/h/bpaJ8/DtOE=
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
Expand All @@ -6,3 +8,5 @@ golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/tools v0.0.0-20191109212701-97ad0ed33101 h1:LCmXVkvpQCDj724eX6irUTPCJP5GelFHxqGSWL2D1R0=
golang.org/x/tools v0.0.0-20191109212701-97ad0ed33101/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4=
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
19 changes: 19 additions & 0 deletions gotests.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ import (
"github.com/cweill/gotests/internal/output"
)

var cmpImport = &models.Import{
Name: "",
Path: `"github.com/google/go-cmp/cmp"`,
}

// Options provides custom filters and parameters for generating tests.
type Options struct {
Only *regexp.Regexp // Includes only functions that match.
Expand All @@ -30,6 +35,7 @@ type Options struct {
TemplateDir string // Path to custom template set
TemplateParams map[string]interface{} // Custom external parameters
TemplateData [][]byte // Data slice for templates
UseGoCmp bool // Use google/go-cmp instead of reflect.DeepEquals
}

// A GeneratedTest contains information about a test file with generated tests.
Expand Down Expand Up @@ -129,6 +135,7 @@ func generateTest(src models.Path, files []models.Path, opt *Options) (*Generate
TemplateDir: opt.TemplateDir,
TemplateParams: opt.TemplateParams,
TemplateData: opt.TemplateData,
UseGoCmp: opt.UseGoCmp,
}

b, err := options.Process(h, funcs)
Expand All @@ -155,8 +162,20 @@ func parseTestFile(p *goparser.Parser, testPath string, h *models.Header) (*mode
return nil, nil, fmt.Errorf("Parser.Parse test file: %v", err)
}
var testFuncs []string
cmpImportNeeded := false
for _, fun := range tr.Funcs {
testFuncs = append(testFuncs, fun.Name)
if cmpImportNeeded {
continue
}
for _, field := range fun.Parameters {
if !(field.IsWriter() || field.IsBasicType()) {
cmpImportNeeded = true
}
}
}
if cmpImportNeeded {
tr.Header.Imports = append(tr.Header.Imports, cmpImport)
}
tr.Header.Imports = append(tr.Header.Imports, h.Imports...)
h = tr.Header
Expand Down
4 changes: 4 additions & 0 deletions gotests/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@
// -template_params_file read external parameters to template by json with file
//
// -template_params read external parameters to template by json with stdin
//
// -use_go_cmp use cmp.Equal (google/go-cmp) instead of reflect.DeepEqual
package main

import (
Expand All @@ -60,6 +62,7 @@ var (
template = flag.String("template", "", `optional. Specify custom test code templates, e.g. testify. This can also be set via environment variable GOTESTS_TEMPLATE`)
templateParamsPath = flag.String("template_params_file", "", "read external parameters to template by json with file")
templateParams = flag.String("template_params", "", "read external parameters to template by json with stdin")
useGoCmp = flag.Bool("use_go_cmp", false, `use cmp.Equal (google/go-cmp) instead of reflect.DeepEqual to perform equality checks`)
)

var (
Expand Down Expand Up @@ -92,6 +95,7 @@ func main() {
Template: valOrGetenv(*template, "GOTESTS_TEMPLATE"),
TemplateDir: valOrGetenv(*templateDir, "GOTESTS_TEMPLATE_DIR"),
TemplateParamsPath: *templateParamsPath,
UseGoCmp: *useGoCmp,
})
}

Expand Down
6 changes: 4 additions & 2 deletions gotests/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package main
import (
"os"
"testing"

"github.com/google/go-cmp/cmp"
)

func Test_valOrGetenv(t *testing.T) {
Expand Down Expand Up @@ -65,8 +67,8 @@ func Test_valOrGetenv(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := valOrGetenv(tt.args.val, tt.args.key); got != tt.want {
t.Errorf("valOrGetenv() = %v, want %v", got, tt.want)
if got := valOrGetenv(tt.args.val, tt.args.key); !cmp.Equal(tt.want, got) {
t.Errorf("valOrGetenv() = %v, want %v\ndiff=%s", got, tt.want, cmp.Diff(tt.want, got))
}
})
}
Expand Down
2 changes: 2 additions & 0 deletions gotests/process/process.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ type Options struct {
TemplateDir string // Path to custom template set
TemplateParamsPath string // Path to custom parameters json file(s).
TemplateData [][]byte // Data slice for templates
UseGoCmp bool // Use google/go-cmp instead of reflect.DeepEquals
}

// Generates tests for the Go files defined in args with the given options.
Expand Down Expand Up @@ -102,6 +103,7 @@ func parseOptions(out io.Writer, opt *Options) *gotests.Options {
TemplateDir: opt.TemplateDir,
TemplateParams: templateParams,
TemplateData: opt.TemplateData,
UseGoCmp: opt.UseGoCmp,
}
}

Expand Down
6 changes: 4 additions & 2 deletions gotests/process/process_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package process
import (
"bytes"
"testing"

"github.com/google/go-cmp/cmp"
)

func TestRun(t *testing.T) {
Expand Down Expand Up @@ -53,8 +55,8 @@ func TestRun(t *testing.T) {
for _, tt := range tests {
out := &bytes.Buffer{}
Run(out, tt.args, tt.opts)
if got := out.String(); got != tt.want {
t.Errorf("%q. Run() =\n%v, want\n%v", tt.name, got, tt.want)
if got := out.String(); !cmp.Equal(tt.want, got) {
t.Errorf("%q. Run() =\n%v, want\n%v\ndiff=%s", tt.name, got, tt.want, cmp.Diff(tt.want, got))
}
}
}
Loading