From 7499a3d2bd9bc73d1bcd9d50bd448b7265c6e52e Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Tue, 12 Mar 2024 15:24:34 -0700 Subject: [PATCH] Standardize on only two value dumping/diffing libraries 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. --- go.mod | 9 +-------- go.sum | 14 ++------------ hcldec/spec_test.go | 12 ++++++++++-- hclsyntax/parser_test.go | 37 +++++++++++++++++++++++-------------- hclsyntax/structure_test.go | 30 ++++++++++++------------------ hclsyntax/variables_test.go | 7 +++++-- hclwrite/parser_test.go | 33 ++++++--------------------------- hclwrite/round_trip_test.go | 8 ++------ 8 files changed, 61 insertions(+), 89 deletions(-) diff --git a/go.mod b/go.mod index a8005025..d9d4da45 100644 --- a/go.mod +++ b/go.mod @@ -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 @@ -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 diff --git a/go.sum b/go.sum index 62014eb6..08ed23ed 100644 --- a/go.sum +++ b/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= @@ -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= diff --git a/hcldec/spec_test.go b/hcldec/spec_test.go index 1b0594d1..a88066b5 100644 --- a/hcldec/spec_test.go +++ b/hcldec/spec_test.go @@ -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" @@ -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{ diff --git a/hclsyntax/parser_test.go b/hclsyntax/parser_test.go index 6e226a60..c3c4ccff 100644 --- a/hclsyntax/parser_test.go +++ b/hclsyntax/parser_test.go @@ -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 @@ -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}, @@ -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}, @@ -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) } }) } @@ -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) } }) } diff --git a/hclsyntax/structure_test.go b/hclsyntax/structure_test.go index c86dc328..180425bf 100644 --- a/hclsyntax/structure_test.go +++ b/hclsyntax/structure_test.go @@ -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" ) @@ -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 @@ -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), ) } }) @@ -499,7 +495,7 @@ func TestBodyJustAttributes(t *testing.T) { }, }, hiddenAttrs: map[string]struct{}{ - "foo": struct{}{}, + "foo": {}, }, }, hcl.Attributes{}, @@ -507,12 +503,6 @@ func TestBodyJustAttributes(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) { got, diags := test.body.JustAttributes() @@ -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, ) } }) diff --git a/hclsyntax/variables_test.go b/hclsyntax/variables_test.go index c841d94a..9eb3d411 100644 --- a/hclsyntax/variables_test.go +++ b/hclsyntax/variables_test.go @@ -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" ) @@ -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), + ) } }) } diff --git a/hclwrite/parser_test.go b/hclwrite/parser_test.go index e557940e..8ccae993 100644 --- a/hclwrite/parser_test.go +++ b/hclwrite/parser_test.go @@ -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" ) @@ -1360,12 +1356,6 @@ 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) @@ -1373,7 +1363,7 @@ func TestPartitionTokens(t *testing.T) { 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, ) @@ -1437,12 +1427,6 @@ 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) @@ -1450,7 +1434,7 @@ func TestPartitionLeadCommentTokens(t *testing.T) { 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, ) } @@ -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, ) } }) diff --git a/hclwrite/round_trip_test.go b/hclwrite/round_trip_test.go index bf306de5..9dbb1526 100644 --- a/hclwrite/round_trip_test.go +++ b/hclwrite/round_trip_test.go @@ -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" @@ -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))) } }) }