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.

Sign up for GitHub

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

Use go-cmp instead of reflect.DeepEqual #155

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
cec4af4
use go-cmp instead of reflect.DeepEqual
mekegi May 13, 2019
66d408d
Merge branch 'develop' into go-cmp
hslatman Mar 24, 2021
7c6e3ac
Fix tests
hslatman Mar 24, 2021
0e65e4f
Merge branch 'develop' into go-cmp
hslatman Mar 27, 2021
0a209c7
Fix PR comments
hslatman Mar 29, 2021
5603b29
Merge branch 'develop' into go-cmp
hslatman Mar 12, 2022
632e2f0
Add `--use_go_cmp` flag to make usage of google/go-cmp optional
hslatman Mar 12, 2022
2ecd7a0
Fix missing `named` flag in README.md
hslatman Mar 12, 2022
2156f80
Fix typo in `reflect.DeepEqual`
hslatman Mar 12, 2022
3ad4a92
Remove old Go version (temporarily)
hslatman Mar 13, 2022
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
3 changes: 3 additions & 0 deletions .github/workflows/go.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 4 additions & 1 deletion go.mod
View file
Open in desktop
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.5
golang.org/x/tools v0.0.0-20191109212701-97ad0ed33101
)

go 1.6
4 changes: 4 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
github.com/google/go-cmp v0.5.5 h1:Khx7svrCpmxxtHBq5j2mp/xVjsi8hQMfNLvJFAlrGgU=
github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
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=
17 changes: 17 additions & 0 deletions gotests.go
View file
Open in desktop
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 Down Expand Up @@ -153,8 +158,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
6 changes: 4 additions & 2 deletions gotests/main_test.go
View file
Open in desktop
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(got, tt.want) {
t.Errorf("valOrGetenv() = %v, want %v\ndiff=%s", got, tt.want, cmp.Diff(got, tt.want))
}
})
}
Expand Down
6 changes: 4 additions & 2 deletions gotests/process/process_test.go
View file
Open in desktop
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(got, tt.want) {
t.Errorf("%q. Run() =\n%v, want\n%v\ndiff=%s", tt.name, got, tt.want, cmp.Diff(got, tt.want))
}
}
}
3 changes: 2 additions & 1 deletion gotests_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"testing"
"unicode"

"github.com/google/go-cmp/cmp"
"golang.org/x/tools/imports"
)

Expand Down Expand Up @@ -918,7 +919,7 @@ func TestGenerateTests(t *testing.T) {
return
}
if got := string(gts[0].Output); got != tt.want {
t.Errorf("%q. GenerateTests(%v) = \n%v, want \n%v", tt.name, tt.args.srcPath, got, tt.want)
t.Errorf("%q. GenerateTests(%v) = diff=%s", tt.name, tt.args.srcPath, cmp.Diff(got, tt.want))
outputResult(t, tmp, tt.name, gts[0].Output)
}
})
Expand Down
18 changes: 11 additions & 7 deletions internal/output/output_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package output

import "testing"
import (
"testing"

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

func TestOptions_providesTemplateData(t *testing.T) {
tests := []struct {
Expand All @@ -16,8 +20,8 @@ func TestOptions_providesTemplateData(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := tt.otpions.providesTemplateData(); got != tt.want {
t.Errorf("Options.isProvidesTemplateData() = %v, want %v", got, tt.want)
if got := tt.otpions.providesTemplateData(); !cmp.Equal(got, tt.want) {
t.Errorf("Options.isProvidesTemplateData() = %v, want %v\ndiff=%s", got, tt.want, cmp.Diff(got, tt.want))
}
})
}
Expand All @@ -36,8 +40,8 @@ func TestOptions_providesTemplate(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := tt.otpions.providesTemplate(); got != tt.want {
t.Errorf("Options.isProvidesTemplate() = %v, want %v", got, tt.want)
if got := tt.otpions.providesTemplate(); !cmp.Equal(got, tt.want) {
t.Errorf("Options.isProvidesTemplate() = %v, want %v\ndiff=%s", got, tt.want, cmp.Diff(got, tt.want))
}
})
}
Expand All @@ -56,8 +60,8 @@ func TestOptions_providesTemplateDir(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := tt.otpions.providesTemplateDir(); got != tt.want {
t.Errorf("Options.isProvidesTemplate() = %v, want %v", got, tt.want)
if got := tt.otpions.providesTemplateDir(); !cmp.Equal(got, tt.want) {
t.Errorf("Options.isProvidesTemplate() = %v, want %v\ndiff=%s", got, tt.want, cmp.Diff(got, tt.want))
}
})
}
Expand Down
46 changes: 23 additions & 23 deletions internal/render/bindata/esc.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions internal/render/templates/function.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,10 @@ func {{.TestName}}(t *testing.T) {
{{- else if .IsBasicType}}
if {{if $f.OnlyReturnsOneValue}}{{Got .}} := {{template "inline" $f}}; {{end}} {{Got .}} != tt.{{Want .}} {
{{- else}}
if {{if $f.OnlyReturnsOneValue}}{{Got .}} := {{template "inline" $f}}; {{end}} !reflect.DeepEqual({{Got .}}, tt.{{Want .}}) {
if {{if $f.OnlyReturnsOneValue}}{{Got .}} := {{template "inline" $f}}; {{end}} !cmp.Equal({{Got .}}, tt.{{Want .}}) {
{{- end}}
t.Errorf("{{template "message" $f}} {{if $f.ReturnsMultiple}}{{Got .}} {{end}}= %v, want %v", {{template "inputs" $f}} {{Got .}}, tt.{{Want .}})
t.Errorf("{{template "message" $f}} {{if $f.ReturnsMultiple}}{{Got .}} {{end}}= %v, want %v{{ if (not ( or .IsWriter .IsBasicType))}}\ndiff=%s{{ end }}", {{template "inputs" $f}} {{Got .}}, tt.{{Want .}}
{{- if (not ( or .IsWriter .IsBasicType))}}, cmp.Diff({{Got .}}, tt.{{Want .}}){{ end }})
}
{{- end}}
{{- if .Subtests }} }) {{- end -}}
Expand Down
5 changes: 3 additions & 2 deletions testdata/bad_customtemplates/function.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,10 @@ func {{.TestName}}(t *testing.T) {
{{- else if .IsBasicType}}
if {{if $f.OnlyReturnsOneValue}}{{Got .}} := {{template "inline" $f}}; {{end}} {{Got .}} != tt.{{Want .}} {
{{- else}}
if {{if $f.OnlyReturnsOneValue}}{{Got .}} := {{template "inline" $f}}; {{end}} !reflect.DeepEqual({{Got .}}, tt.{{Want .}}) {
if {{if $f.OnlyReturnsOneValue}}{{Got .}} := {{template "inline" $f}}; {{end}} !cmp.Equal({{Got .}}, tt.{{Want .}}) {
{{- end}}
t.Errorf("{{template "message" $f}} {{if $f.ReturnsMultiple}}{{Got .}} {{end}}= %v, want %v", {{template "inputs" $f}} {{Got .}}, tt.{{Want .}})
t.Errorf("{{template "message" $f}} {{if $f.ReturnsMultiple}}{{Got .}} {{end}}= %v, want %v{{ if (not ( or .IsWriter .IsBasicType))}}\ndiff=%s{{ end }}", {{template "inputs" $f}} {{Got .}}, tt.{{Want .}}
{{- if (not ( or .IsWriter .IsBasicType))}}, cmp.Diff({{Got .}}, tt.{{Want .}}){{ end }})
}
{{- end}}
{{- if .Subtests }} }) {{- end -}}
Expand Down
5 changes: 3 additions & 2 deletions testdata/customtemplates/function.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,10 @@ func {{.TestName}}(t *testing.T) {
{{- else if .IsBasicType}}
if {{if $f.OnlyReturnsOneValue}}{{Got .}} := {{template "inline" $f}}; {{end}} {{Got .}} != tt.{{Want .}} {
{{- else}}
if {{if $f.OnlyReturnsOneValue}}{{Got .}} := {{template "inline" $f}}; {{end}} !reflect.DeepEqual({{Got .}}, tt.{{Want .}}) {
if {{if $f.OnlyReturnsOneValue}}{{Got .}} := {{template "inline" $f}}; {{end}} !cmp.Equal({{Got .}}, tt.{{Want .}}) {
{{- end}}
t.Errorf("{{template "message" $f}} {{if $f.ReturnsMultiple}}{{Got .}} {{end}}= %v, want %v", {{template "inputs" $f}} {{Got .}}, tt.{{Want .}})
t.Errorf("{{template "message" $f}} {{if $f.ReturnsMultiple}}{{Got .}} {{end}}= %v, want %v{{ if (not ( or .IsWriter .IsBasicType))}}\ndiff=%s{{ end }}", {{template "inputs" $f}} {{Got .}}, tt.{{Want .}}
{{- if (not ( or .IsWriter .IsBasicType))}}, cmp.Diff({{Got .}}, tt.{{Want .}}){{ end }})
}
{{- end}}
{{- if .Subtests }} }) {{- end -}}
Expand Down
7 changes: 4 additions & 3 deletions testdata/goldens/custom_importer_fails.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
package testdata

import (
"reflect"
"testing"

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

func TestFooFilter(t *testing.T) {
Expand All @@ -23,8 +24,8 @@ func TestFooFilter(t *testing.T) {
t.Errorf("%q. FooFilter() error = %v, wantErr %v", tt.name, err, tt.wantErr)
continue
}
if !reflect.DeepEqual(got, tt.want) {
t.Errorf("%q. FooFilter() = %v, want %v", tt.name, got, tt.want)
if !cmp.Equal(got, tt.want) {
t.Errorf("%q. FooFilter() = %v, want %v\ndiff=%s", tt.name, got, tt.want, cmp.Diff(got, tt.want))
}
}
}
Expand Down
7 changes: 4 additions & 3 deletions testdata/goldens/existing_test_file.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
package testdata

import (
"reflect"
"testing"

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

func TestBarBar100(t *testing.T) {
Expand Down Expand Up @@ -63,8 +64,8 @@ func TestFoo100(t *testing.T) {
t.Errorf("%q. Foo100() error = %v, wantErr %v", tt.name, err, tt.wantErr)
continue
}
if !reflect.DeepEqual(got, tt.want) {
t.Errorf("%q. Foo100() = %v, want %v", tt.name, got, tt.want)
if !cmp.Equal(got, tt.want) {
t.Errorf("%q. Foo100() = %v, want %v\ndiff=%s", tt.name, got, tt.want, cmp.Diff(got, tt.want))
}
}
}
Expand Down
7 changes: 4 additions & 3 deletions testdata/goldens/function_returning_two_results_and_an_error.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
package testdata

import (
"reflect"
"testing"

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

func TestFoo25(t *testing.T) {
Expand All @@ -27,8 +28,8 @@ func TestFoo25(t *testing.T) {
if got != tt.want {
t.Errorf("%q. Foo25() got = %v, want %v", tt.name, got, tt.want)
}
if !reflect.DeepEqual(got1, tt.want1) {
t.Errorf("%q. Foo25() got1 = %v, want %v", tt.name, got1, tt.want1)
if !cmp.Equal(got1, tt.want1) {
t.Errorf("%q. Foo25() got1 = %v, want %v\ndiff=%s", tt.name, got1, tt.want1, cmp.Diff(got1, tt.want1))
}
}
}
7 changes: 4 additions & 3 deletions testdata/goldens/function_with_channel_parameter_and_result.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
package testdata

import (
"reflect"
"testing"

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

func TestFoo23(t *testing.T) {
Expand All @@ -17,8 +18,8 @@ func TestFoo23(t *testing.T) {
// TODO: Add test cases.
}
for _, tt := range tests {
if got := Foo23(tt.args.ch); !reflect.DeepEqual(got, tt.want) {
t.Errorf("%q. Foo23() = %v, want %v", tt.name, got, tt.want)
if got := Foo23(tt.args.ch); !cmp.Equal(got, tt.want) {
t.Errorf("%q. Foo23() = %v, want %v\ndiff=%s", tt.name, got, tt.want, cmp.Diff(got, tt.want))
}
}
}
Loading