From 1cd370fdb2f6d1f0aa3823ace876cbf1b8b1c599 Mon Sep 17 00:00:00 2001 From: drodriguezhdez Date: Tue, 24 Mar 2020 10:10:15 +0100 Subject: [PATCH 1/4] Add testing patch functions as helpers --- go.mod | 3 +++ instrumentation/testing/logger.go | 11 +++++++++++ reflection/reflect.go | 30 ++++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+) diff --git a/go.mod b/go.mod index e7306cf3..9cb774d3 100644 --- a/go.mod +++ b/go.mod @@ -15,10 +15,13 @@ require ( github.com/stretchr/testify v1.5.1 github.com/undefinedlabs/go-mpatch v0.0.0-20200122175732-0044123dbb98 github.com/vmihailenco/msgpack v4.0.4+incompatible + golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3 // indirect golang.org/x/net v0.0.0-20200301022130-244492dfa37a golang.org/x/sys v0.0.0-20200302150141-5c8b2ff67527 // indirect + golang.org/x/tools v0.0.0-20190524140312-2c0ae7006135 // indirect google.golang.org/appengine v1.6.5 // indirect google.golang.org/grpc v1.27.1 gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 // indirect gopkg.in/tomb.v2 v2.0.0-20161208151619-d5d1b5820637 + honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc // indirect ) diff --git a/instrumentation/testing/logger.go b/instrumentation/testing/logger.go index 26ef389c..67a34170 100644 --- a/instrumentation/testing/logger.go +++ b/instrumentation/testing/logger.go @@ -53,6 +53,7 @@ func UnpatchTestingLogger() { func patchError() { patch("Error", func(test *Test, argsValues []reflect.Value) { + test.t.Helper() args := getArgs(argsValues[0]) test.Error(args...) }) @@ -60,6 +61,7 @@ func patchError() { func patchErrorf() { patch("Errorf", func(test *Test, argsValues []reflect.Value) { + test.t.Helper() format := argsValues[0].String() args := getArgs(argsValues[1]) test.Errorf(format, args...) @@ -68,6 +70,7 @@ func patchErrorf() { func patchFatal() { patch("Fatal", func(test *Test, argsValues []reflect.Value) { + test.t.Helper() args := getArgs(argsValues[0]) test.Fatal(args...) }) @@ -75,6 +78,7 @@ func patchFatal() { func patchFatalf() { patch("Fatalf", func(test *Test, argsValues []reflect.Value) { + test.t.Helper() format := argsValues[0].String() args := getArgs(argsValues[1]) test.Fatalf(format, args...) @@ -83,6 +87,7 @@ func patchFatalf() { func patchLog() { patch("Log", func(test *Test, argsValues []reflect.Value) { + test.t.Helper() args := getArgs(argsValues[0]) test.Log(args...) }) @@ -90,6 +95,7 @@ func patchLog() { func patchLogf() { patch("Logf", func(test *Test, argsValues []reflect.Value) { + test.t.Helper() format := argsValues[0].String() args := getArgs(argsValues[1]) test.Logf(format, args...) @@ -98,6 +104,7 @@ func patchLogf() { func patchSkip() { patch("Skip", func(test *Test, argsValues []reflect.Value) { + test.t.Helper() args := getArgs(argsValues[0]) test.Skip(args...) }) @@ -105,6 +112,7 @@ func patchSkip() { func patchSkipf() { patch("Skipf", func(test *Test, argsValues []reflect.Value) { + test.t.Helper() format := argsValues[0].String() args := getArgs(argsValues[1]) test.Skipf(format, args...) @@ -137,6 +145,9 @@ func patch(methodName string, methodBody func(test *Test, argsValues []reflect.V var err error methodPatch, err = mpatch.PatchMethodWithMakeFunc(method, func(in []reflect.Value) []reflect.Value { t := (*testing.T)(unsafe.Pointer(in[0].Pointer())) + t.Helper() + reflection.AddToHelpersMap(t, []string{"reflect.callReflect"}) + if t == nil { instrumentation.Logger().Println("testing.T is nil") return nil diff --git a/reflection/reflect.go b/reflection/reflect.go index 41714dc3..c0345f30 100644 --- a/reflection/reflect.go +++ b/reflection/reflect.go @@ -35,6 +35,36 @@ func GetTestMutex(t *testing.T) *sync.RWMutex { return nil } +func GetHelpersMap(t *testing.T) map[string]struct{} { + t.Helper() // Ensure map creation before accessing it + mu := GetTestMutex(t) + if mu != nil { + mu.Lock() + defer mu.Unlock() + } + if pointer, err := GetFieldPointerOf(t, "helpers"); err == nil { + return *(*map[string]struct{})(pointer) + } + return nil +} + +func AddToHelpersMap(t *testing.T, frameFnNames []string) { + helpers := GetHelpersMap(t) + if helpers == nil { + return + } + + mu := GetTestMutex(t) + if mu != nil { + mu.Lock() + defer mu.Unlock() + } + + for _, fnName := range frameFnNames { + helpers[fnName] = struct{}{} + } +} + func GetIsParallel(t *testing.T) bool { mu := GetTestMutex(t) if mu != nil { From 9780980c20ee76239826380d69b7ff34fe0ffbb5 Mon Sep 17 00:00:00 2001 From: drodriguezhdez Date: Tue, 24 Mar 2020 11:43:18 +0100 Subject: [PATCH 2/4] Add external functions to t.helpers map --- instrumentation/testing/logger.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/instrumentation/testing/logger.go b/instrumentation/testing/logger.go index 67a34170..c1998120 100644 --- a/instrumentation/testing/logger.go +++ b/instrumentation/testing/logger.go @@ -146,7 +146,10 @@ func patch(methodName string, methodBody func(test *Test, argsValues []reflect.V methodPatch, err = mpatch.PatchMethodWithMakeFunc(method, func(in []reflect.Value) []reflect.Value { t := (*testing.T)(unsafe.Pointer(in[0].Pointer())) t.Helper() - reflection.AddToHelpersMap(t, []string{"reflect.callReflect"}) + reflection.AddToHelpersMap(t, []string{ + "reflect.callReflect", + "reflect.makeFuncStub", + }) if t == nil { instrumentation.Logger().Println("testing.T is nil") From 3cc99e212eb77570f93434bbc336352198cf22a1 Mon Sep 17 00:00:00 2001 From: drodriguezhdez Date: Tue, 24 Mar 2020 12:01:19 +0100 Subject: [PATCH 3/4] Revert go.mod modifications --- go.mod | 3 --- 1 file changed, 3 deletions(-) diff --git a/go.mod b/go.mod index 9cb774d3..e7306cf3 100644 --- a/go.mod +++ b/go.mod @@ -15,13 +15,10 @@ require ( github.com/stretchr/testify v1.5.1 github.com/undefinedlabs/go-mpatch v0.0.0-20200122175732-0044123dbb98 github.com/vmihailenco/msgpack v4.0.4+incompatible - golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3 // indirect golang.org/x/net v0.0.0-20200301022130-244492dfa37a golang.org/x/sys v0.0.0-20200302150141-5c8b2ff67527 // indirect - golang.org/x/tools v0.0.0-20190524140312-2c0ae7006135 // indirect google.golang.org/appengine v1.6.5 // indirect google.golang.org/grpc v1.27.1 gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 // indirect gopkg.in/tomb.v2 v2.0.0-20161208151619-d5d1b5820637 - honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc // indirect ) From 70f9eb59dc0ffa8f17939052b6992ee20a8b9e68 Mon Sep 17 00:00:00 2001 From: drodriguezhdez Date: Tue, 24 Mar 2020 12:05:29 +0100 Subject: [PATCH 4/4] Applying PR feedback --- instrumentation/testing/logger.go | 9 +++++---- reflection/reflect.go | 21 +++++---------------- 2 files changed, 10 insertions(+), 20 deletions(-) diff --git a/instrumentation/testing/logger.go b/instrumentation/testing/logger.go index c1998120..08665537 100644 --- a/instrumentation/testing/logger.go +++ b/instrumentation/testing/logger.go @@ -145,16 +145,17 @@ func patch(methodName string, methodBody func(test *Test, argsValues []reflect.V var err error methodPatch, err = mpatch.PatchMethodWithMakeFunc(method, func(in []reflect.Value) []reflect.Value { t := (*testing.T)(unsafe.Pointer(in[0].Pointer())) + if t == nil { + instrumentation.Logger().Println("testing.T is nil") + return nil + } + t.Helper() reflection.AddToHelpersMap(t, []string{ "reflect.callReflect", "reflect.makeFuncStub", }) - if t == nil { - instrumentation.Logger().Println("testing.T is nil") - return nil - } test := GetTest(t) if test == nil { instrumentation.Logger().Printf("test struct for %v doesn't exist\n", t.Name()) diff --git a/reflection/reflect.go b/reflection/reflect.go index c0345f30..ef281283 100644 --- a/reflection/reflect.go +++ b/reflection/reflect.go @@ -35,31 +35,20 @@ func GetTestMutex(t *testing.T) *sync.RWMutex { return nil } -func GetHelpersMap(t *testing.T) map[string]struct{} { - t.Helper() // Ensure map creation before accessing it +func AddToHelpersMap(t *testing.T, frameFnNames []string) { + t.Helper() mu := GetTestMutex(t) if mu != nil { mu.Lock() defer mu.Unlock() } - if pointer, err := GetFieldPointerOf(t, "helpers"); err == nil { - return *(*map[string]struct{})(pointer) - } - return nil -} -func AddToHelpersMap(t *testing.T, frameFnNames []string) { - helpers := GetHelpersMap(t) - if helpers == nil { + pointer, err := GetFieldPointerOf(t, "helpers") + if err != nil { return } - mu := GetTestMutex(t) - if mu != nil { - mu.Lock() - defer mu.Unlock() - } - + helpers := *(*map[string]struct{})(pointer) for _, fnName := range frameFnNames { helpers[fnName] = struct{}{} }