From 8022a12dd161823cc2e2efc392d2641241469481 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Olivier=20Mengu=C3=A9?= Date: Thu, 8 Jun 2023 20:17:12 +0200 Subject: [PATCH 1/3] Refactor Run to extract Run signature check Extract signature check of the Run method. --- quicktest.go | 62 +++++++++++++++++++++++++++++++++------------------- 1 file changed, 39 insertions(+), 23 deletions(-) diff --git a/quicktest.go b/quicktest.go index 65243cf..dfb7268 100644 --- a/quicktest.go +++ b/quicktest.go @@ -217,6 +217,38 @@ var ( tbType = reflect.TypeOf(new(testing.TB)).Elem() ) +// getRunFuncSignature checks the signature of the Run method of the type (ex: *testing.T) +// and returns the signature of its function argument (func(t *T) for *testing.T). +func getRunFuncSignature(t reflect.Type) (reflect.Type, error) { + badType := func(detail string) (reflect.Type, error) { + return nil, fmt.Errorf("cannot execute Run with underlying concrete type %s (%s)", t, detail) + } + m, ok := t.MethodByName("Run") + if !ok { + // c.TB doesn't implement a Run method. + return badType("no Run method") + } + mt := m.Type + // fmt.Println(mt) + if mt.NumIn() != 3 || + mt.In(1) != stringType || + mt.NumOut() != 1 || + mt.Out(0) != boolType { + // The Run method doesn't have the right argument counts and types. + return badType("wrong argument count for Run method") + } + farg := mt.In(2) + if farg.Kind() != reflect.Func || + farg.NumIn() != 1 || + farg.NumOut() != 0 || + !farg.In(0).AssignableTo(tbType) { + // The first argument to the Run function arg isn't right. + return badType("bad first argument type for Run method") + } + + return farg, nil +} + // Run runs f as a subtest of t called name. It's a wrapper around // the Run method of c.TB that provides the quicktest checker to f. When // the function completes, c.Done will be called to run any @@ -243,30 +275,12 @@ var ( // A panic is raised when Run is called and the embedded concrete type does not // implement a Run method with a correct signature. func (c *C) Run(name string, f func(c *C)) bool { - badType := func(m string) { - panic(fmt.Sprintf("cannot execute Run with underlying concrete type %T (%s)", c.TB, m)) - } - m := reflect.ValueOf(c.TB).MethodByName("Run") - if !m.IsValid() { - // c.TB doesn't implement a Run method. - badType("no Run method") - } - mt := m.Type() - if mt.NumIn() != 2 || - mt.In(0) != stringType || - mt.NumOut() != 1 || - mt.Out(0) != boolType { - // The Run method doesn't have the right argument counts and types. - badType("wrong argument count for Run method") - } - farg := mt.In(1) - if farg.Kind() != reflect.Func || - farg.NumIn() != 1 || - farg.NumOut() != 0 || - !farg.In(0).AssignableTo(tbType) { - // The first argument to the Run function arg isn't right. - badType("bad first argument type for Run method") + + farg, err := getRunFuncSignature(reflect.TypeOf(c.TB)) + if err != nil { + panic(err.Error()) } + cFormat := c.getFormat() fv := reflect.MakeFunc(farg, func(args []reflect.Value) []reflect.Value { c2 := New(args[0].Interface().(testing.TB)) @@ -275,6 +289,8 @@ func (c *C) Run(name string, f func(c *C)) bool { f(c2) return nil }) + + m := reflect.ValueOf(c.TB).MethodByName("Run") return m.Call([]reflect.Value{reflect.ValueOf(name), fv})[0].Interface().(bool) } From dd6f0283f0941baa40fd7cabad6776189c01f4cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Olivier=20Mengu=C3=A9?= Date: Mon, 31 Jul 2023 22:39:30 +0200 Subject: [PATCH 2/3] C.Run: use shortcuts for common Run signatures (testing.T, testing.B) In C.Run add shortcuts to completely bypass the use of reflect for the well known cases of Run signatures with either *testing.T, *testing.B or *quicktest.C argument. --- quicktest.go | 66 +++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 52 insertions(+), 14 deletions(-) diff --git a/quicktest.go b/quicktest.go index dfb7268..5a9a407 100644 --- a/quicktest.go +++ b/quicktest.go @@ -275,23 +275,61 @@ func getRunFuncSignature(t reflect.Type) (reflect.Type, error) { // A panic is raised when Run is called and the embedded concrete type does not // implement a Run method with a correct signature. func (c *C) Run(name string, f func(c *C)) bool { + cFormat := c.getFormat() - farg, err := getRunFuncSignature(reflect.TypeOf(c.TB)) - if err != nil { - panic(err.Error()) - } + // Handle the various signatures of the Run method of c.TB + switch tb := c.TB.(type) { + + // *testing.T + case interface { + Run(string, func(*testing.T)) bool + }: + return tb.Run(name, func(t *testing.T) { + t.Helper() + cSub := New(t) + defer cSub.Done() + cSub.SetFormat(cFormat) + f(cSub) + }) - cFormat := c.getFormat() - fv := reflect.MakeFunc(farg, func(args []reflect.Value) []reflect.Value { - c2 := New(args[0].Interface().(testing.TB)) - defer c2.Done() - c2.SetFormat(cFormat) - f(c2) - return nil - }) + // *testing.B + case interface { + Run(string, func(*testing.B)) bool + }: + return tb.Run(name, func(b *testing.B) { + cSub := New(b) + defer cSub.Done() + cSub.SetFormat(cFormat) + f(cSub) + }) - m := reflect.ValueOf(c.TB).MethodByName("Run") - return m.Call([]reflect.Value{reflect.ValueOf(name), fv})[0].Interface().(bool) + // *quicktest.C + case interface{ Run(string, func(*C)) bool }: + return tb.Run(name, func(c *C) { + cSub := New(c) + defer cSub.Done() + cSub.SetFormat(cFormat) + f(cSub) + }) + + // any testing.TB, by using reflect + default: + farg, err := getRunFuncSignature(reflect.TypeOf(c.TB)) + if err != nil { + panic(err.Error()) + } + + fv := reflect.MakeFunc(farg, func(args []reflect.Value) []reflect.Value { + cSub := New(args[0].Interface().(testing.TB)) + defer cSub.Done() + cSub.SetFormat(cFormat) + f(cSub) + return nil + }) + + m := reflect.ValueOf(c.TB).MethodByName("Run") + return m.Call([]reflect.Value{reflect.ValueOf(name), fv})[0].Interface().(bool) + } } // Parallel signals that this test is to be run in parallel with (and only with) other parallel tests. From bc88e032cd10a05252a396b1f9cd833ea3ae4a9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Olivier=20Mengu=C3=A9?= Date: Tue, 1 Aug 2023 23:46:09 +0200 Subject: [PATCH 3/3] Run: factorize the setup of the qt.C Factorize the setup of the qt.C given to the subtest. That setup is now common to the reflect version and the shortcut ones instead of being duplicated. --- quicktest.go | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/quicktest.go b/quicktest.go index 5a9a407..b787dba 100644 --- a/quicktest.go +++ b/quicktest.go @@ -276,6 +276,13 @@ func getRunFuncSignature(t reflect.Type) (reflect.Type, error) { // implement a Run method with a correct signature. func (c *C) Run(name string, f func(c *C)) bool { cFormat := c.getFormat() + // A wrapper for f that prepares its *C for the subtest + callF := func(tb testing.TB) { + cSub := New(tb) + defer cSub.Done() + cSub.SetFormat(cFormat) + f(cSub) + } // Handle the various signatures of the Run method of c.TB switch tb := c.TB.(type) { @@ -286,10 +293,7 @@ func (c *C) Run(name string, f func(c *C)) bool { }: return tb.Run(name, func(t *testing.T) { t.Helper() - cSub := New(t) - defer cSub.Done() - cSub.SetFormat(cFormat) - f(cSub) + callF(t) }) // *testing.B @@ -297,19 +301,13 @@ func (c *C) Run(name string, f func(c *C)) bool { Run(string, func(*testing.B)) bool }: return tb.Run(name, func(b *testing.B) { - cSub := New(b) - defer cSub.Done() - cSub.SetFormat(cFormat) - f(cSub) + callF(b) }) // *quicktest.C case interface{ Run(string, func(*C)) bool }: return tb.Run(name, func(c *C) { - cSub := New(c) - defer cSub.Done() - cSub.SetFormat(cFormat) - f(cSub) + callF(c) }) // any testing.TB, by using reflect @@ -320,10 +318,7 @@ func (c *C) Run(name string, f func(c *C)) bool { } fv := reflect.MakeFunc(farg, func(args []reflect.Value) []reflect.Value { - cSub := New(args[0].Interface().(testing.TB)) - defer cSub.Done() - cSub.SetFormat(cFormat) - f(cSub) + callF(args[0].Interface().(testing.TB)) return nil })