Skip to content

Commit

Permalink
internal/cuetdtest: move testing.T out of M
Browse files Browse the repository at this point in the history 
It's common to run multiple subtests inside a single point in the
matrix, but it turns out that's problematic: if one of those tests ends
up failing by using `m.T`, it will fail the outer test, not the one
that's currently running, ending up with the testing package printing
"subtest may have called FailNow on a parent test".

We could make sure to update `M.T` for each subtest, but that's
problematic itself, because multiple subtests will share a given `*M`
instance and if any of them happens to invoke `T.Parallel` then there's
a race condition.

Instead, make `M` just about the point in the matrix and add an explicit
`*testing.T` argument to the methods that it has that do checks.

Signed-off-by: Roger Peppe <rogpeppe@gmail.com>
Change-Id: Id83ba0fac6c4db7780d4086820aed4190dd5bd65
Dispatch-Trailer: {"type":"trybot","CL":1200260,"patchset":4,"ref":"refs/changes/60/1200260/4","targetBranch":"master"}
  • Loading branch information
@rogpeppe @cueckoo
rogpeppe authored and cueckoo committed Aug 30, 2024
1 parent e48fb9d commit 7edbd93
Show file tree
Hide file tree
Showing 11 changed files with 162 additions and 171 deletions.
34 changes: 17 additions & 17 deletions cue/attribute_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,14 +96,13 @@ func TestAttributes(t *testing.T) {
out: "[]",
}}
for _, tc := range testCases {
cuetdtest.FullMatrix.Run(t, tc.path, func(t *cuetdtest.M) {
v := getValue(t, config).LookupPath(cue.ParsePath(tc.path))
cuetdtest.FullMatrix.Run(t, tc.path, func(t *testing.T, m *cuetdtest.M) {
v := getValue(m, config).LookupPath(cue.ParsePath(tc.path))
a := v.Attributes(tc.flags)
got := fmt.Sprint(a)
if got != tc.out {
t.Errorf("got %v; want %v", got, tc.out)
}

})
}
}
Expand Down Expand Up @@ -137,8 +136,8 @@ func TestAttributeErr(t *testing.T) {
err: errors.New(`attribute "bar" does not exist`),
}}
for _, tc := range testCases {
cuetdtest.FullMatrix.Run(t, tc.path+"-"+tc.attr, func(t *cuetdtest.M) {
v := getValue(t, config).Lookup("a", tc.path)
cuetdtest.FullMatrix.Run(t, tc.path+"-"+tc.attr, func(t *testing.T, m *cuetdtest.M) {
v := getValue(m, config).Lookup("a", tc.path)
a := v.Attribute(tc.attr)
err := a.Err()
if !cmpError(err, tc.err) {
Expand All @@ -152,8 +151,8 @@ func TestAttributeName(t *testing.T) {
const config = `
a: 0 @foo(a,b,c=1) @bar()
`
cuetdtest.FullMatrix.Do(t, func(t *cuetdtest.M) {
v := getValue(t, config).Lookup("a")
cuetdtest.FullMatrix.Do(t, func(t *testing.T, m *cuetdtest.M) {
v := getValue(m, config).Lookup("a")
a := v.Attribute("foo")
if got, want := a.Name(), "foo"; got != want {
t.Errorf("got %v; want %v", got, want)
Expand Down Expand Up @@ -210,8 +209,8 @@ func TestAttributeString(t *testing.T) {
err: errors.New("field does not exist"),
}}
for _, tc := range testCases {
cuetdtest.FullMatrix.Run(t, fmt.Sprintf("%s.%s:%d", tc.path, tc.attr, tc.pos), func(t *cuetdtest.M) {
v := getValue(t, config).Lookup("a", tc.path)
cuetdtest.FullMatrix.Run(t, fmt.Sprintf("%s.%s:%d", tc.path, tc.attr, tc.pos), func(t *testing.T, m *cuetdtest.M) {
v := getValue(m, config).Lookup("a", tc.path)
a := v.Attribute(tc.attr)
got, err := a.String(tc.pos)
if !cmpError(err, tc.err) {
Expand All @@ -221,6 +220,7 @@ func TestAttributeString(t *testing.T) {
t.Errorf("str: got %v; want %v", got, tc.str)
}
})

}
}

Expand Down Expand Up @@ -270,8 +270,8 @@ func TestAttributeArg(t *testing.T) {
raw: " s= spaces in value ",
}}
for _, tc := range testCases {
cuetdtest.FullMatrix.Run(t, fmt.Sprintf("%d", tc.pos), func(t *cuetdtest.M) {
v := getValue(t, config).Lookup("a")
cuetdtest.FullMatrix.Run(t, fmt.Sprintf("%d", tc.pos), func(t *testing.T, m *cuetdtest.M) {
v := getValue(m, config).Lookup("a")
a := v.Attribute("foo")
key, val := a.Arg(tc.pos)
raw := a.RawArg(tc.pos)
Expand Down Expand Up @@ -327,8 +327,8 @@ func TestAttributeInt(t *testing.T) {
err: errors.New(`strconv.ParseInt: parsing "c=1": invalid syntax`),
}}
for _, tc := range testCases {
cuetdtest.FullMatrix.Run(t, fmt.Sprintf("%s.%s:%d", tc.path, tc.attr, tc.pos), func(t *cuetdtest.M) {
v := getValue(t, config).Lookup("a", tc.path)
cuetdtest.FullMatrix.Run(t, fmt.Sprintf("%s.%s:%d", tc.path, tc.attr, tc.pos), func(t *testing.T, m *cuetdtest.M) {
v := getValue(m, config).Lookup("a", tc.path)
a := v.Attribute(tc.attr)
got, err := a.Int(tc.pos)
if !cmpError(err, tc.err) {
Expand Down Expand Up @@ -384,8 +384,8 @@ func TestAttributeFlag(t *testing.T) {
err: errors.New("field does not exist"),
}}
for _, tc := range testCases {
cuetdtest.FullMatrix.Run(t, fmt.Sprintf("%s.%s:%d", tc.path, tc.attr, tc.pos), func(t *cuetdtest.M) {
v := getValue(t, config).Lookup("a", tc.path)
cuetdtest.FullMatrix.Run(t, fmt.Sprintf("%s.%s:%d", tc.path, tc.attr, tc.pos), func(t *testing.T, m *cuetdtest.M) {
v := getValue(m, config).Lookup("a", tc.path)
a := v.Attribute(tc.attr)
got, err := a.Flag(tc.pos, tc.flag)
if !cmpError(err, tc.err) {
Expand Down Expand Up @@ -459,8 +459,8 @@ func TestAttributeLookup(t *testing.T) {
err: errors.New("field does not exist"),
}}
for _, tc := range testCases {
cuetdtest.FullMatrix.Run(t, fmt.Sprintf("%s.%s:%d", tc.path, tc.attr, tc.pos), func(t *cuetdtest.M) {
v := getValue(t, config).Lookup("a", tc.path)
cuetdtest.FullMatrix.Run(t, fmt.Sprintf("%s.%s:%d", tc.path, tc.attr, tc.pos), func(t *testing.T, m *cuetdtest.M) {
v := getValue(m, config).Lookup("a", tc.path)
a := v.Attribute(tc.attr)
got, _, err := a.Lookup(tc.pos, tc.key)
if !cmpError(err, tc.err) {
Expand Down
12 changes: 6 additions & 6 deletions cue/decode_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -241,9 +241,9 @@ func TestDecode(t *testing.T) {
want: []interface{}{},
}}
for _, tc := range testCases {
cuetdtest.FullMatrix.Run(t, tc.value, func(t *cuetdtest.M) {
err := getValue(t, tc.value).Decode(tc.dst)
checkFatal(t.T, err, tc.err, "init")
cuetdtest.FullMatrix.Run(t, tc.value, func(t *testing.T, m *cuetdtest.M) {
err := getValue(m, tc.value).Decode(tc.dst)
checkFatal(t, err, tc.err, "init")

got := reflect.ValueOf(tc.dst).Elem().Interface()
if diff := cmp.Diff(got, tc.want); diff != "" {
Expand All @@ -255,21 +255,21 @@ func TestDecode(t *testing.T) {
}

func TestDecodeIntoCUEValue(t *testing.T) {
cuetdtest.FullMatrix.Do(t, func(t *cuetdtest.M) {
cuetdtest.FullMatrix.Do(t, func(t *testing.T, m *cuetdtest.M) {
// We should be able to decode into a CUE value so we can
// decode partially incomplete values into Go.
// This test doesn't fit within the table used by TestDecode
// because cue values aren't easily comparable with cmp.Diff.
var st struct {
X cue.Value `json:"x"`
}
err := getValue(t, `x: string`).Decode(&st)
err := getValue(m, `x: string`).Decode(&st)
qt.Assert(t, qt.IsNil(err))
qt.Assert(t, qt.Equals(fmt.Sprint(st.X), "string"))

// Check we can decode into a top level value.
var v cue.Value
err = getValue(t, `int`).Decode(&v)
err = getValue(m, `int`).Decode(&v)
qt.Assert(t, qt.IsNil(err))
qt.Assert(t, qt.Equals(fmt.Sprint(v), "int"))
})
Expand Down
Loading

0 comments on commit 7edbd93

Please sign in to comment.