Use go-cmp to report unexpected test results.

cmp.Diff panics when comparing a struct that has unexported fields and no String
or Equal method, so change some structs to either export all their fields or
have an Equal method.

This fixes #1136.

go.mod

@@ -2,6 +2,7 @@ module src.elv.sh
 
 require (
 	github.com/creack/pty v1.1.15
+	github.com/google/go-cmp v0.5.7
 	github.com/mattn/go-isatty v0.0.14
 	github.com/sourcegraph/go-lsp v0.0.0-20200429204803-219e11d77f5d
 	github.com/sourcegraph/jsonrpc2 v0.1.0

go.sum

@@ -1,5 +1,7 @@
 github.com/creack/pty v1.1.15 h1:cKRCLMj3Ddm54bKSpemfQ8AtYFBhAI2MPmdys22fBdc=
 github.com/creack/pty v1.1.15/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4=
+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=
 github.com/gorilla/websocket v1.4.1 h1:q7AeDBpnBk8AogcD4DSag/Ukw/KV+YhzLj2bP5HvKCM=
 github.com/gorilla/websocket v1.4.1/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE=
 github.com/mattn/go-isatty v0.0.14 h1:yVuAays6BHfxijgZPzw+3Zlu5yQgKGP2/hcQbHb7S9Y=
@@ -14,3 +16,5 @@ golang.org/x/sys v0.0.0-20200923182605-d9f96fdee20d/go.mod h1:h1NjWce9XRLGQEsW7w
 golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220227234510-4e6760a101f9 h1:nhht2DYV/Sn3qOayu8lM+cU1ii9sTLUeBQwQQfUHtrs=
 golang.org/x/sys v0.0.0-20220227234510-4e6760a101f9/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+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=

pkg/edit/highlight/highlight.go

@@ -62,14 +62,14 @@ func highlight(code string, cfg Config, lateCb func(ui.Text)) (ui.Text, []error)
 	var cmdRegions []cmdRegion
 
 	for _, r := range regions {
-		if r.begin > lastEnd {
+		if r.Begin > lastEnd {
 			// Add inter-region text.
-			text = append(text, &ui.Segment{Text: code[lastEnd:r.begin]})
+			text = append(text, &ui.Segment{Text: code[lastEnd:r.Begin]})
 		}
 
-		regionCode := code[r.begin:r.end]
+		regionCode := code[r.Begin:r.End]
 		var styling ui.Styling
-		if r.typ == commandRegion {
+		if r.Type == commandRegion {
 			if cfg.HasCommand != nil {
 				// Do not highlight now, but collect the index of the region and the
 				// segment.
@@ -79,7 +79,7 @@ func highlight(code string, cfg Config, lateCb func(ui.Text)) (ui.Text, []error)
 				styling = stylingForGoodCommand
 			}
 		} else {
-			styling = stylingFor[r.typ]
+			styling = stylingFor[r.Type]
 		}
 		seg := &ui.Segment{Text: regionCode}
 		if styling != nil {
@@ -87,7 +87,7 @@ func highlight(code string, cfg Config, lateCb func(ui.Text)) (ui.Text, []error)
 		}
 
 		text = append(text, seg)
-		lastEnd = r.end
+		lastEnd = r.End
 	}
 	if len(code) > lastEnd {
 		// Add text after the last region as unstyled.

pkg/edit/highlight/regions.go

@@ -12,21 +12,21 @@ var sourceText = parse.SourceText
 
 // Represents a region to be highlighted.
 type region struct {
-	begin int
-	end   int
+	Begin int
+	End   int
 	// Regions can be lexical or semantic. Lexical regions always correspond to
 	// a leaf node in the parse tree, either a parse.Primary node or a parse.Sep
 	// node. Semantic regions may span several leaves and override all lexical
 	// regions in it.
-	kind regionKind
+	Kind regionKind
 	// In lexical regions for Primary nodes, this field corresponds to the Type
 	// field of the node (e.g. "bareword", "single-quoted"). In lexical regions
 	// for Sep nodes, this field is simply the source text itself (e.g. "(",
-	// "|"), except for comments, which have typ == "comment".
+	// "|"), except for comments, which have Type == "comment".
 	//
 	// In semantic regions, this field takes a value from a fixed list (see
 	// below).
-	typ string
+	Type string
 }
 
 type regionKind int
@@ -79,11 +79,11 @@ func fixRegions(regions []region) []region {
 	// Sort regions by the begin position, putting semantic regions before
 	// lexical regions.
 	sort.Slice(regions, func(i, j int) bool {
-		if regions[i].begin < regions[j].begin {
+		if regions[i].Begin < regions[j].Begin {
 			return true
 		}
-		if regions[i].begin == regions[j].begin {
-			return regions[i].kind == semanticRegion && regions[j].kind == lexicalRegion
+		if regions[i].Begin == regions[j].Begin {
+			return regions[i].Kind == semanticRegion && regions[j].Kind == lexicalRegion
 		}
 		return false
 	})
@@ -91,11 +91,11 @@ func fixRegions(regions []region) []region {
 	var newRegions []region
 	lastEnd := 0
 	for _, r := range regions {
-		if r.begin < lastEnd {
+		if r.Begin < lastEnd {
 			continue
 		}
 		newRegions = append(newRegions, r)
-		lastEnd = r.end
+		lastEnd = r.End
 	}
 	return newRegions
 }

pkg/eval/evaltest/evaltest.go

@@ -22,10 +22,12 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
 	"src.elv.sh/pkg/eval"
 	"src.elv.sh/pkg/eval/vals"
 	"src.elv.sh/pkg/parse"
 	"src.elv.sh/pkg/testutil"
+	"src.elv.sh/pkg/tt"
 )
 
 // Case is a test case that can be used in Test.
@@ -137,35 +139,35 @@ func Test(t *testing.T, tests ...Case) {
 // with NewEvaler and passed to the setup function.
 func TestWithSetup(t *testing.T, setup func(*eval.Evaler), tests ...Case) {
 	t.Helper()
-	for _, tt := range tests {
-		t.Run(strings.Join(tt.codes, "\n"), func(t *testing.T) {
+	for _, tc := range tests {
+		t.Run(strings.Join(tc.codes, "\n"), func(t *testing.T) {
 			t.Helper()
 			ev := eval.NewEvaler()
 			setup(ev)
-			if tt.setup != nil {
-				tt.setup(ev)
+			if tc.setup != nil {
+				tc.setup(ev)
 			}
 
-			r := evalAndCollect(t, ev, tt.codes)
+			r := evalAndCollect(t, ev, tc.codes)
 
-			if tt.verify != nil {
-				tt.verify(t)
+			if tc.verify != nil {
+				tc.verify(t)
 			}
-			if !matchOut(tt.want.ValueOut, r.ValueOut) {
-				t.Errorf("got value out %v, want %v",
-					reprs(r.ValueOut), reprs(tt.want.ValueOut))
+			if !matchOut(tc.want.ValueOut, r.ValueOut) {
+				t.Errorf("got value out (-want +got):\n%s",
+					cmp.Diff(r.ValueOut, tc.want.ValueOut, tt.CommonCmpOpt))
 			}
-			if !bytes.Equal(tt.want.BytesOut, r.BytesOut) {
-				t.Errorf("got bytes out %q, want %q", r.BytesOut, tt.want.BytesOut)
+			if !bytes.Equal(tc.want.BytesOut, r.BytesOut) {
+				t.Errorf("got bytes out %q, want %q", r.BytesOut, tc.want.BytesOut)
 			}
-			if !bytes.Contains(r.StderrOut, tt.want.StderrOut) {
-				t.Errorf("got stderr out %q, want %q", r.StderrOut, tt.want.StderrOut)
+			if !bytes.Contains(r.StderrOut, tc.want.StderrOut) {
+				t.Errorf("got stderr out %q, want %q", r.StderrOut, tc.want.StderrOut)
 			}
-			if !matchErr(tt.want.CompilationError, r.CompilationError) {
+			if !matchErr(tc.want.CompilationError, r.CompilationError) {
 				t.Errorf("got compilation error %v, want %v",
-					r.CompilationError, tt.want.CompilationError)
+					r.CompilationError, tc.want.CompilationError)
 			}
-			if !matchErr(tt.want.Exception, r.Exception) {
+			if !matchErr(tc.want.Exception, r.Exception) {
 				t.Errorf("unexpected exception")
 				if exc, ok := r.Exception.(eval.Exception); ok {
 					// For an eval.Exception report the type of the underlying error.
@@ -174,7 +176,7 @@ func TestWithSetup(t *testing.T, setup func(*eval.Evaler), tests ...Case) {
 				} else {
 					t.Logf("got: %T: %v", r.Exception, r.Exception)
 				}
-				t.Errorf("want: %v", tt.want.Exception)
+				t.Errorf("want: %v", tc.want.Exception)
 			}
 		})
 	}
@@ -264,14 +266,6 @@ func match(got, want interface{}) bool {
 	return vals.Equal(got, want)
 }
 
-func reprs(values []interface{}) []string {
-	s := make([]string, len(values))
-	for i, v := range values {
-		s[i] = vals.ReprPlain(v)
-	}
-	return s
-}
-
 func matchErr(want, got error) bool {
 	if want == nil {
 		return got == nil

pkg/eval/options_test.go

@@ -12,6 +12,9 @@ type opts struct {
 	bar int
 }
 
+// Equal is required by cmp.Diff, since opts contains unexported fields.
+func (o opts) Equal(p opts) bool { return o == p }
+
 func TestScanOptions(t *testing.T) {
 	// A wrapper of ScanOptions, to make it easier to test
 	wrapper := func(src RawOptions, dstInit interface{}) (interface{}, error) {

pkg/eval/vals/conversion_test.go

@@ -10,7 +10,7 @@ import (
 )
 
 type someType struct {
-	foo string
+	Foo string
 }
 
 func TestScanToGo_ConcreteTypeDst(t *testing.T) {
@@ -164,6 +164,9 @@ type aStruct struct {
 	bar interface{}
 }
 
+// Equal is required by cmp.Diff, since aStruct contains unexported fields.
+func (a aStruct) Equal(b aStruct) bool { return a == b }
+
 func TestScanMapToGo(t *testing.T) {
 	// A wrapper around ScanMapToGo, to make it easier to test.
 	scanMapToGo := func(src Map, dstInit interface{}) (interface{}, error) {

pkg/parse/quote_test.go

@@ -7,7 +7,7 @@ import (
 )
 
 func TestQuote(t *testing.T) {
-	Test(t, Fn("Quote", Quote).ArgsFmt("(%q)").RetsFmt("%q"), Table{
+	Test(t, Fn("Quote", Quote).ArgsFmt("(%q)"), Table{
 		// Empty string is single-quoted.
 		Args("").Rets(`''`),
 
@@ -37,7 +37,7 @@ func TestQuote(t *testing.T) {
 }
 
 func TestQuoteAs(t *testing.T) {
-	Test(t, Fn("QuoteAs", QuoteAs).ArgsFmt("(%q, %s)").RetsFmt("(%q, %s)"), Table{
+	Test(t, Fn("QuoteAs", QuoteAs).ArgsFmt("(%q, %s)"), Table{
 		// DoubleQuote is always respected.
 		Args("", DoubleQuoted).Rets(`""`, DoubleQuoted),
 		Args("a", DoubleQuoted).Rets(`"a"`, DoubleQuoted),
@@ -52,7 +52,7 @@ func TestQuoteAs(t *testing.T) {
 }
 
 func TestQuoteVariableName(t *testing.T) {
-	Test(t, Fn("QuoteVariableName", QuoteVariableName).ArgsFmt("(%q)").RetsFmt("%q"), Table{
+	Test(t, Fn("QuoteVariableName", QuoteVariableName).ArgsFmt("(%q)"), Table{
 		Args("").Rets("''"),
 		Args("foo").Rets("foo"),
 		Args("a/b").Rets("'a/b'"),

pkg/tt/cmpopt.go

@@ -0,0 +1,40 @@
+package tt
+
+import (
+	"math/big"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
+	"src.elv.sh/pkg/persistent/hashmap"
+	"src.elv.sh/pkg/persistent/vector"
+)
+
+// CommonCmpOpt is cmp.Option shared between tt and evaltest.
+var CommonCmpOpt = cmp.Options([]cmp.Option{
+	cmp.Transformer("transformList", transformList),
+	cmp.Transformer("transformMap", transformMap),
+	cmp.Comparer(func(x, y *big.Int) bool { return x.Cmp(y) == 0 }),
+	cmp.Comparer(func(x, y *big.Rat) bool { return x.Cmp(y) == 0 }),
+})
+
+var cmpopt = cmp.Options([]cmp.Option{
+	cmpopts.EquateErrors(),
+	CommonCmpOpt,
+})
+
+func transformList(l vector.Vector) []interface{} {
+	res := make([]interface{}, 0, l.Len())
+	for it := l.Iterator(); it.HasElem(); it.Next() {
+		res = append(res, it.Elem())
+	}
+	return res
+}
+
+func transformMap(m hashmap.Map) map[interface{}]interface{} {
+	res := make(map[interface{}]interface{}, m.Len())
+	for it := m.Iterator(); it.HasElem(); it.Next() {
+		k, v := it.Elem()
+		res[k] = v
+	}
+	return res
+}

pkg/tt/tt.go

@@ -4,9 +4,11 @@
 package tt
 
 import (
-	"bytes"
 	"fmt"
 	"reflect"
+	"strings"
+
+	"github.com/google/go-cmp/cmp"
 )
 
 // Table represents a test table.
@@ -74,20 +76,19 @@ func Test(t T, fn *FnToTest, tests Table) {
 		rets := call(fn.body, test.args)
 		for _, retsMatcher := range test.retsMatchers {
 			if !match(retsMatcher, rets) {
-				var argsString, retsString, wantRetsString string
+				var args string
 				if fn.argsFmt == "" {
-					argsString = sprintArgs(test.args...)
+					args = sprintArgs(test.args...)
 				} else {
-					argsString = fmt.Sprintf(fn.argsFmt, test.args...)
+					args = fmt.Sprintf(fn.argsFmt, test.args...)
 				}
-				if fn.retsFmt == "" {
-					retsString = sprintRets(rets...)
-					wantRetsString = sprintRets(retsMatcher...)
+				var diff string
+				if len(retsMatcher) == 1 && len(rets) == 1 {
+					diff = cmp.Diff(retsMatcher[0], rets[0], cmpopt)
 				} else {
-					retsString = fmt.Sprintf(fn.retsFmt, rets...)
-					wantRetsString = fmt.Sprintf(fn.retsFmt, retsMatcher...)
+					diff = cmp.Diff(retsMatcher, rets, cmpopt)
 				}
-				t.Errorf("%s(%s) -> %s, want %s", fn.name, argsString, retsString, wantRetsString)
+				t.Errorf("%s(%s) returns (-want +got):\n%s", fn.name, args, diff)
 			}
 		}
 	}
@@ -127,18 +128,7 @@ func matchOne(m, a interface{}) bool {
 }
 
 func sprintArgs(args ...interface{}) string {
-	return sprintCommaDelimited(args...)
-}
-
-func sprintRets(rets ...interface{}) string {
-	if len(rets) == 1 {
-		return fmt.Sprint(rets[0])
-	}
-	return "(" + sprintCommaDelimited(rets...) + ")"
-}
-
-func sprintCommaDelimited(args ...interface{}) string {
-	var b bytes.Buffer
+	var b strings.Builder
 	for i, arg := range args {
 		if i > 0 {
 			b.WriteString(", ")

pkg/tt/tt_test.go

@@ -2,6 +2,7 @@ package tt
 
 import (
 	"fmt"
+	"strings"
 	"testing"
 )
 
@@ -41,7 +42,7 @@ func TestTTFailDefaultFmtOneReturn(t *testing.T) {
 		Fn("add", add),
 		Table{Args(1, 10).Rets(12)},
 	)
-	assertOneError(t, testT, "add(1, 10) -> 11, want 12")
+	assertOneError(t, testT, "add(1, 10) returns (-want +got):\n")
 }
 
 func TestTTFailDefaultFmtMultiReturn(t *testing.T) {
@@ -50,7 +51,7 @@ func TestTTFailDefaultFmtMultiReturn(t *testing.T) {
 		Fn("addsub", addsub),
 		Table{Args(1, 10).Rets(11, -90)},
 	)
-	assertOneError(t, testT, "addsub(1, 10) -> (11, -9), want (11, -90)")
+	assertOneError(t, testT, "addsub(1, 10) returns (-want +got):\n")
 }
 
 func TestTTFailCustomFmt(t *testing.T) {
@@ -60,16 +61,17 @@ func TestTTFailCustomFmt(t *testing.T) {
 		Table{Args(1, 10).Rets(11, -90)},
 	)
 	assertOneError(t, testT,
-		"addsub(x = 1, y = 10) -> (a = 11, b = -9), want (a = 11, b = -90)")
+		"addsub(x = 1, y = 10) returns (-want +got):\n")
 }
 
-func assertOneError(t *testing.T, testT testT, want string) {
+func assertOneError(t *testing.T, testT testT, wantPrefix string) {
+	t.Helper()
 	switch len(testT) {
 	case 0:
 		t.Errorf("Test didn't error when it should")
 	case 1:
-		if testT[0] != want {
-			t.Errorf("Test wrote message %q, want %q", testT[0], want)
+		if !strings.HasPrefix(testT[0], wantPrefix) {
+			t.Errorf("Test wrote message:\ngot:  %q\nwant: %q...", testT[0], wantPrefix)
 		}
 	default:
 		t.Errorf("Test wrote too many error messages")

pkg/ui/color.go

@@ -83,17 +83,17 @@ func (c xterm256Color) fgSGR() string  { return "38;5;" + strconv.Itoa(int(c)) }
 func (c xterm256Color) bgSGR() string  { return "48;5;" + strconv.Itoa(int(c)) }
 func (c xterm256Color) String() string { return "color" + strconv.Itoa(int(c)) }
 
-type trueColor struct{ r, g, b uint8 }
+type trueColor struct{ R, G, B uint8 }
 
 func (c trueColor) fgSGR() string { return "38;2;" + c.rgbSGR() }
 func (c trueColor) bgSGR() string { return "48;2;" + c.rgbSGR() }
 
 func (c trueColor) String() string {
-	return fmt.Sprintf("#%02x%02x%02x", c.r, c.g, c.b)
+	return fmt.Sprintf("#%02x%02x%02x", c.R, c.G, c.B)
 }
 
 func (c trueColor) rgbSGR() string {
-	return fmt.Sprintf("%d;%d;%d", c.r, c.g, c.b)
+	return fmt.Sprintf("%d;%d;%d", c.R, c.G, c.B)
 }
 
 func parseColor(name string) Color {

website/go.sum

@@ -1,6 +1,8 @@
 github.com/BurntSushi/toml v1.0.0 h1:dtDWrepsVPfW9H/4y7dDgFc2MBUSeJhlaDtK13CxFlU=
 github.com/BurntSushi/toml v1.0.0/go.mod h1:CxXYINrC8qIiEnFrOxCa7Jy5BFHlXnUU2pbicEuybxQ=
 github.com/creack/pty v1.1.15/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4=
+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=
 github.com/gorilla/websocket v1.4.1/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE=
 github.com/mattn/go-isatty v0.0.14/go.mod h1:7GGIvUiUoEMVVmxf/4nioHXj79iQHKdU27kJ6hsGG94=
 github.com/sourcegraph/go-lsp v0.0.0-20200429204803-219e11d77f5d/go.mod h1:SULmZY7YNBsvNiQbrb/BEDdEJ84TGnfyUQxaHt8t8rY=
@@ -10,3 +12,4 @@ golang.org/x/sys v0.0.0-20200923182605-d9f96fdee20d/go.mod h1:h1NjWce9XRLGQEsW7w
 golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220227234510-4e6760a101f9 h1:nhht2DYV/Sn3qOayu8lM+cU1ii9sTLUeBQwQQfUHtrs=
 golang.org/x/sys v0.0.0-20220227234510-4e6760a101f9/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=