Skip to content

Commit

Permalink
Standardize on only two value dumping/diffing libraries
Browse files Browse the repository at this point in the history
Due to the quite messy heritage of this codebase -- including a large part
of it being just a fork of my earlier personal project ZCL -- there were
many different conventions for how to pretty-print and diff values in the
tests in different parts of the codebase.

To reduce the dependency sprawl, this commit now standardizes on:
  - github.com/davecgh/go-spew for pretty-printing
  - github.com/google/go-cmp for diffing

These two dependencies were already present anyway, are the most general
out of all of the candidates, and are also already in use by at least some
of HCL's most significant callers, such as HashiCorp Terraform.

The version of go-cmp we were previously using seems to have a bug that
causes the tests to crash when run under the Go race detector, so I've
also upgraded that dependency to latest here to clear that bug.
  • Loading branch information
apparentlymart committed Mar 12, 2024
1 parent 57f8bbf commit 7499a3d
Show file tree
Hide file tree
Showing 8 changed files with 61 additions and 89 deletions.
9 changes: 1 addition & 8 deletions go.mod
Expand Up @@ -4,15 +4,11 @@ go 1.18

require (
github.com/agext/levenshtein v1.2.1
github.com/apparentlymart/go-dump v0.0.0-20180507223929-23540a00eaa3
github.com/apparentlymart/go-textseg/v15 v15.0.0
github.com/davecgh/go-spew v1.1.1
github.com/go-test/deep v1.0.3
github.com/google/go-cmp v0.3.1
github.com/kr/pretty v0.1.0
github.com/kylelemons/godebug v0.0.0-20170820004349-d65d576e9348
github.com/google/go-cmp v0.6.0
github.com/mitchellh/go-wordwrap v0.0.0-20150314170334-ad45545899c7
github.com/sergi/go-diff v1.0.0
github.com/spf13/pflag v1.0.2
github.com/zclconf/go-cty v1.13.0
github.com/zclconf/go-cty-debug v0.0.0-20191215020915-b22d67c1ba0b
Expand All @@ -22,9 +18,6 @@ require (

require (
github.com/apparentlymart/go-textseg/v13 v13.0.0 // indirect
github.com/kr/text v0.1.0 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/stretchr/testify v1.2.2 // indirect
golang.org/x/mod v0.8.0 // indirect
golang.org/x/sys v0.5.0 // indirect
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1 // indirect
Expand Down
14 changes: 2 additions & 12 deletions go.sum
@@ -1,7 +1,5 @@
github.com/agext/levenshtein v1.2.1 h1:QmvMAjj2aEICytGiWzmxoE0x2KZvE0fvmqMOfy2tjT8=
github.com/agext/levenshtein v1.2.1/go.mod h1:JEDfjyjHDjOF/1e4FlBE/PkbqA9OfWu2ki2W0IB5558=
github.com/apparentlymart/go-dump v0.0.0-20180507223929-23540a00eaa3 h1:ZSTrOEhiM5J5RFxEaFvMZVEAM1KvT1YzbEOwB2EAGjA=
github.com/apparentlymart/go-dump v0.0.0-20180507223929-23540a00eaa3/go.mod h1:oL81AME2rN47vu18xqj1S1jPIPuN7afo62yKTNn3XMM=
github.com/apparentlymart/go-textseg v1.0.0/go.mod h1:z96Txxhf3xSFMPmb5X/1W05FF/Nj9VFpLOpjS5yuumk=
github.com/apparentlymart/go-textseg/v13 v13.0.0 h1:Y+KvPE1NYz0xl601PVImeQfFyEy6iT90AvPUL1NNfNw=
github.com/apparentlymart/go-textseg/v13 v13.0.0/go.mod h1:ZK2fH7c4NqDTLtiYLvIkEghdlcqw7yxLeM89kiTRPUo=
Expand All @@ -12,25 +10,17 @@ github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs
github.com/go-test/deep v1.0.3 h1:ZrJSEWsXzPOxaZnFteGEfooLba+ju3FYIbOrS+rQd68=
github.com/go-test/deep v1.0.3/go.mod h1:wGDj63lr65AM2AQyKZd/NYHGb0R+1RLqB8NKt3aSFNA=
github.com/golang/protobuf v1.1.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
github.com/google/go-cmp v0.3.1 h1:Xye71clBPdm5HgqGwUkwhbynsUJZhDbS20FvLhQ2izg=
github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU=
github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI=
github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo=
github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE=
github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
github.com/kylelemons/godebug v0.0.0-20170820004349-d65d576e9348 h1:MtvEpTB6LX3vkb4ax0b5D2DHbNAUsen0Gx5wZoq3lV4=
github.com/kylelemons/godebug v0.0.0-20170820004349-d65d576e9348/go.mod h1:B69LEHPfb2qLo0BaaOLcbitczOKLWTsrBG9LczfCD4k=
github.com/mitchellh/go-wordwrap v0.0.0-20150314170334-ad45545899c7 h1:DpOJ2HYzCv8LZP15IdmG+YdwD2luVPHITV96TkirNBM=
github.com/mitchellh/go-wordwrap v0.0.0-20150314170334-ad45545899c7/go.mod h1:ZXFpozHsX6DPmq2I0TCekCxypsnAUbP2oI0UX1GXzOo=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/sergi/go-diff v1.0.0 h1:Kpca3qRNrduNnOQeazBd0ysaKrUJiIuISHxogkT9RPQ=
github.com/sergi/go-diff v1.0.0/go.mod h1:0CfEIISq7TuYL3j771MWULgwwjU+GofnZX9QAmXWZgo=
github.com/spf13/pflag v1.0.2 h1:Fy0orTDgHdbnzHcsOgfCN4LtHf0ec3wwtiwJqwvf3Gc=
github.com/spf13/pflag v1.0.2/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4=
github.com/stretchr/testify v1.2.2 h1:bSDNvY7ZPG5RlJ8otE/7V6gMiyenm9RtJ7IUVIAoJ1w=
github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
github.com/vmihailenco/msgpack v3.3.3+incompatible/go.mod h1:fy3FlTQTDXWkZ7Bh6AcGMlsjHatGryHQYUTf1ShIgkk=
github.com/zclconf/go-cty v1.2.0/go.mod h1:hOPWgoHbaTUnI5k4D2ld+GRpFJSCe6bCM7m1q/N4PQ8=
github.com/zclconf/go-cty v1.13.0 h1:It5dfKTTZHe9aeppbNOda3mN7Ag7sg6QkBNm6TkyFa0=
Expand Down
12 changes: 10 additions & 2 deletions hcldec/spec_test.go
Expand Up @@ -8,7 +8,6 @@ import (
"reflect"
"testing"

"github.com/apparentlymart/go-dump/dump"
"github.com/google/go-cmp/cmp"
"github.com/zclconf/go-cty-debug/ctydebug"
"github.com/zclconf/go-cty/cty"
Expand Down Expand Up @@ -98,7 +97,16 @@ bar = barval
},
}
if !reflect.DeepEqual(gotVars, wantVars) {
t.Errorf("wrong Variables result\ngot: %s\nwant: %s", dump.Value(gotVars), dump.Value(wantVars))
t.Errorf(
"wrong Variables result\n%s",
cmp.Diff(
wantVars, gotVars,
cmp.AllowUnexported(
hcl.TraverseRoot{},
),
ctydebug.CmpOptions,
),
)
}

ctx := &hcl.EvalContext{
Expand Down
37 changes: 23 additions & 14 deletions hclsyntax/parser_test.go
Expand Up @@ -5,19 +5,16 @@ package hclsyntax

import (
"fmt"
"sync"
"testing"

"github.com/go-test/deep"
"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/hashicorp/hcl/v2"
"github.com/zclconf/go-cty-debug/ctydebug"
"github.com/zclconf/go-cty/cty"
)

func init() {
deep.MaxDepth = 999
}

func TestParseConfig(t *testing.T) {
tests := []struct {
input string
Expand Down Expand Up @@ -2197,6 +2194,7 @@ block "valid" {}
"a": {
Name: "a",
Expr: &LiteralValueExpr{
Val: cty.DynamicVal,
SrcRange: hcl.Range{
Start: hcl.Pos{Line: 1, Column: 5, Byte: 4},
End: hcl.Pos{Line: 1, Column: 6, Byte: 5},
Expand Down Expand Up @@ -2235,6 +2233,7 @@ block "valid" {}
"a": {
Name: "a",
Expr: &LiteralValueExpr{
Val: cty.DynamicVal,
SrcRange: hcl.Range{
Start: hcl.Pos{Line: 1, Column: 5, Byte: 4},
End: hcl.Pos{Line: 1, Column: 6, Byte: 5},
Expand Down Expand Up @@ -2573,11 +2572,23 @@ block "valid" {}
}

got := file.Body

if diff := deep.Equal(got, test.want); diff != nil {
for _, problem := range diff {
t.Errorf(problem)
}
diff := cmp.Diff(
test.want, got,
cmp.AllowUnexported(
Body{},
AnonSymbolExpr{},
hcl.TraverseRoot{},
hcl.TraverseAttr{},
hcl.TraverseIndex{},
),
cmpopts.IgnoreUnexported(
sync.Mutex{},
sync.RWMutex{},
),
ctydebug.CmpOptions,
)
if diff != "" {
t.Errorf("wrong result\n%s", diff)
}
})
}
Expand Down Expand Up @@ -3892,10 +3903,8 @@ func TestParseConfigDiagnostics(t *testing.T) {
t.Logf("\n%s", test.input)
_, diags := ParseConfig([]byte(test.input), "test.hcl", hcl.InitialPos)

if diff := deep.Equal(diags, test.want); diff != nil {
for _, problem := range diff {
t.Errorf(problem)
}
if diff := cmp.Diff(test.want, diags); diff != "" {
t.Errorf("wrong diagnostics\n%s", diff)
}
})
}
Expand Down
30 changes: 12 additions & 18 deletions hclsyntax/structure_test.go
Expand Up @@ -8,8 +8,10 @@ import (
"reflect"
"testing"

"github.com/davecgh/go-spew/spew"
"github.com/google/go-cmp/cmp"
"github.com/hashicorp/hcl/v2"
"github.com/kylelemons/godebug/pretty"
"github.com/zclconf/go-cty-debug/ctydebug"
"github.com/zclconf/go-cty/cty"
)

Expand Down Expand Up @@ -390,12 +392,6 @@ func TestBodyContent(t *testing.T) {
},
}

prettyConfig := &pretty.Config{
Diffable: true,
IncludeUnexported: true,
PrintStringers: true,
}

for i, test := range tests {
t.Run(fmt.Sprintf("%02d", i), func(t *testing.T) {
var got *hcl.BodyContent
Expand All @@ -416,7 +412,7 @@ func TestBodyContent(t *testing.T) {
if !reflect.DeepEqual(got, test.want) {
t.Errorf(
"wrong result\ndiff: %s",
prettyConfig.Compare(test.want, got),
cmp.Diff(test.want, got),
)
}
})
Expand Down Expand Up @@ -499,20 +495,14 @@ func TestBodyJustAttributes(t *testing.T) {
},
},
hiddenAttrs: map[string]struct{}{
"foo": struct{}{},
"foo": {},
},
},
hcl.Attributes{},
0,
},
}

prettyConfig := &pretty.Config{
Diffable: true,
IncludeUnexported: true,
PrintStringers: true,
}

for i, test := range tests {
t.Run(fmt.Sprintf("%02d", i), func(t *testing.T) {
got, diags := test.body.JustAttributes()
Expand All @@ -524,11 +514,15 @@ func TestBodyJustAttributes(t *testing.T) {
}
}

if !reflect.DeepEqual(got, test.want) {
diff := cmp.Diff(
test.want, got,
ctydebug.CmpOptions,
)
if diff != "" {
t.Errorf(
"wrong result\nbody: %s\ndiff: %s",
prettyConfig.Sprint(test.body),
prettyConfig.Compare(test.want, got),
spew.Sdump(test.body),
diff,
)
}
})
Expand Down
7 changes: 5 additions & 2 deletions hclsyntax/variables_test.go
Expand Up @@ -8,8 +8,8 @@ import (
"reflect"
"testing"

"github.com/davecgh/go-spew/spew"
"github.com/hashicorp/hcl/v2"
"github.com/kr/pretty"
"github.com/zclconf/go-cty/cty"
)

Expand Down Expand Up @@ -300,7 +300,10 @@ func TestVariables(t *testing.T) {
got := Variables(test.Expr)

if !reflect.DeepEqual(got, test.Want) {
t.Errorf("wrong result\ngot: %s\nwant: %s", pretty.Sprint(got), pretty.Sprint(test.Want))
t.Errorf(
"wrong result\ngot: %s\nwant: %s",
spew.Sdump(got), spew.Sdump(test.Want),
)
}
})
}
Expand Down
33 changes: 6 additions & 27 deletions hclwrite/parser_test.go
Expand Up @@ -5,15 +5,11 @@ package hclwrite

import (
"fmt"
"reflect"
"testing"

"github.com/davecgh/go-spew/spew"

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

"github.com/kylelemons/godebug/pretty"

"github.com/hashicorp/hcl/v2"
"github.com/hashicorp/hcl/v2/hclsyntax"
)
Expand Down Expand Up @@ -1360,20 +1356,14 @@ func TestPartitionTokens(t *testing.T) {
},
}

prettyConfig := &pretty.Config{
Diffable: true,
IncludeUnexported: true,
PrintStringers: true,
}

for i, test := range tests {
t.Run(fmt.Sprintf("%02d", i), func(t *testing.T) {
gotStart, gotEnd := partitionTokens(test.tokens, test.rng)

if gotStart != test.wantStart || gotEnd != test.wantEnd {
t.Errorf(
"wrong result\ntokens: %s\nrange: %#v\ngot: %d, %d\nwant: %d, %d",
prettyConfig.Sprint(test.tokens), test.rng,
spew.Sdump(test.tokens), test.rng,
gotStart, test.wantStart,
gotEnd, test.wantEnd,
)
Expand Down Expand Up @@ -1437,20 +1427,14 @@ func TestPartitionLeadCommentTokens(t *testing.T) {
},
}

prettyConfig := &pretty.Config{
Diffable: true,
IncludeUnexported: true,
PrintStringers: true,
}

for i, test := range tests {
t.Run(fmt.Sprintf("%02d", i), func(t *testing.T) {
gotStart := partitionLeadCommentTokens(test.tokens)

if gotStart != test.wantStart {
t.Errorf(
"wrong result\ntokens: %s\ngot: %d\nwant: %d",
prettyConfig.Sprint(test.tokens),
spew.Sdump(test.tokens),
gotStart, test.wantStart,
)
}
Expand Down Expand Up @@ -1589,20 +1573,15 @@ foo "bar" "baz" {
},
}

prettyConfig := &pretty.Config{
Diffable: true,
IncludeUnexported: true,
PrintStringers: true,
}

for _, test := range tests {
t.Run(test.input, func(t *testing.T) {
got := lexConfig([]byte(test.input))

if !reflect.DeepEqual(got, test.want) {
diff := prettyConfig.Compare(test.want, got)
diff := cmp.Diff(test.want, got)
if diff != "" {
t.Errorf(
"wrong result\ninput: %s\ndiff: %s", test.input, diff,
"wrong result\ninput: %s\ndiff:\n%s",
test.input, diff,
)
}
})
Expand Down
8 changes: 2 additions & 6 deletions hclwrite/round_trip_test.go
Expand Up @@ -7,7 +7,7 @@ import (
"bytes"
"testing"

"github.com/sergi/go-diff/diffmatchpatch"
"github.com/google/go-cmp/cmp"
"github.com/zclconf/go-cty/cty"
"github.com/zclconf/go-cty/cty/function"
"github.com/zclconf/go-cty/cty/function/stdlib"
Expand Down Expand Up @@ -71,12 +71,8 @@ block {
}

result := wr.Bytes()

if !bytes.Equal(result, src) {
dmp := diffmatchpatch.New()
diffs := dmp.DiffMain(string(src), string(result), false)
// t.Errorf("wrong result\nresult:\n%s\ninput:\n%s", result, src)
t.Errorf("wrong result\ndiff: (red indicates missing lines, and green indicates unexpected lines)\n%s", dmp.DiffPrettyText(diffs))
t.Errorf("wrong result\ndiff:\n%s", cmp.Diff(string(src), string(result)))
}
})
}
Expand Down

0 comments on commit 7499a3d

Please sign in to comment.