From 6075cb948056f21bb8b0b03e3179d3953d632116 Mon Sep 17 00:00:00 2001 From: Daniel Redondo Date: Fri, 20 Mar 2020 16:17:56 +0100 Subject: [PATCH 01/20] Add an 'SetTestCodeFromCaller' API to set the test code --- init.go | 10 +++++ instrumentation/testing/benchmark.go | 9 +++-- instrumentation/testing/testing.go | 22 ++++++++--- instrumentation/testing/util.go | 55 +++++++++++----------------- instrumentation/testing/util_test.go | 10 +++-- 5 files changed, 59 insertions(+), 47 deletions(-) diff --git a/init.go b/init.go index fb3ae83d..85449eda 100644 --- a/init.go +++ b/init.go @@ -75,6 +75,16 @@ func GetContextFromTest(t *testing.T) context.Context { return context.TODO() } +// Sets the test code from the caller of this func +func SetTestCodeFromCaller(t *testing.T) { + test := GetTest(t) + if test == nil { + return + } + pc, _, _, _ := runtime.Caller(1) + test.SetTestCode(pc) +} + // Gets the *Benchmark from a *testing.B func GetBenchmark(b *testing.B) *scopetesting.Benchmark { return scopetesting.GetBenchmark(b) diff --git a/instrumentation/testing/benchmark.go b/instrumentation/testing/benchmark.go index 1bd122b7..a704f580 100644 --- a/instrumentation/testing/benchmark.go +++ b/instrumentation/testing/benchmark.go @@ -111,8 +111,10 @@ func startBenchmark(b *testing.B, pc uintptr, benchFunc func(b *testing.B)) { fullTestName = strings.Join(nameSegments, "/") } packageName := reflection.GetBenchmarkSuiteName(b) + pName, _, tCode := getPackageAndNameAndBoundaries(pc) + if packageName == "" { - packageName = getPackageName(pc, fullTestName) + packageName = pName } oTags := opentracing.Tags{ @@ -124,9 +126,8 @@ func startBenchmark(b *testing.B, pc uintptr, benchFunc func(b *testing.B)) { "test.type": "benchmark", } - testCode := getTestCodeBoundaries(pc, fullTestName) - if testCode != "" { - oTags["test.code"] = testCode + if tCode != "" { + oTags["test.code"] = tCode } var startOptions []opentracing.StartSpanOption diff --git a/instrumentation/testing/testing.go b/instrumentation/testing/testing.go index 04c882fb..583ef0e3 100644 --- a/instrumentation/testing/testing.go +++ b/instrumentation/testing/testing.go @@ -24,9 +24,10 @@ import ( type ( Test struct { testing.TB - ctx context.Context - span opentracing.Span - t *testing.T + ctx context.Context + span opentracing.Span + t *testing.T + codePC uintptr } Option func(*Test) @@ -64,6 +65,7 @@ func StartTestFromCaller(t *testing.T, pc uintptr, opts ...Option) *Test { // If there is already one we want to replace it, so we clear the context test.ctx = context.Background() } + test.codePC = pc for _, opt := range opts { opt(test) @@ -72,17 +74,16 @@ func StartTestFromCaller(t *testing.T, pc uintptr, opts ...Option) *Test { // Extracting the testing func name (by removing any possible sub-test suffix `{test_func}/{sub_test}`) // to search the func source code bounds and to calculate the package name. fullTestName := runner.GetOriginalTestName(t.Name()) - packageName := getPackageName(pc, fullTestName) + pName, _, testCode := getPackageAndNameAndBoundaries(pc) testTags := opentracing.Tags{ "span.kind": "test", "test.name": fullTestName, - "test.suite": packageName, + "test.suite": pName, "test.framework": "testing", "test.language": "go", } - testCode := getTestCodeBoundaries(pc, fullTestName) if testCode != "" { testTags["test.code"] = testCode } @@ -102,6 +103,15 @@ func StartTestFromCaller(t *testing.T, pc uintptr, opts ...Option) *Test { return test } +// Set test code +func (test *Test) SetTestCode(pc uintptr) { + pName, _, fBoundaries := getPackageAndNameAndBoundaries(pc) + test.span.SetTag("test.suite", pName) + if fBoundaries != "" { + test.span.SetTag("test.code", fBoundaries) + } +} + // Ends the current test func (test *Test) End() { autoInstrumentedTestsMutex.RLock() diff --git a/instrumentation/testing/util.go b/instrumentation/testing/util.go index 4ea946d7..ba5c209a 100644 --- a/instrumentation/testing/util.go +++ b/instrumentation/testing/util.go @@ -10,49 +10,38 @@ import ( "go.undefinedlabs.com/scopeagent/instrumentation" ) -// Gets the Test/Benchmark parent func name without sub-benchmark and sub-test segments -func getFuncName(fullName string) string { - testNameSlash := strings.IndexByte(fullName, '/') - funcName := fullName - if testNameSlash >= 0 { - funcName = fullName[:testNameSlash] - } - return funcName -} - -// Gets the Package name -func getPackageName(pc uintptr, fullName string) string { - // Parent test/benchmark name - funcName := getFuncName(fullName) - // Full func name (format ex: {packageName}.{test/benchmark name}.{inner function of sub benchmark/test} +func getPackageAndName(pc uintptr) (string, string) { funcFullName := runtime.FuncForPC(pc).Name() - - // We select the packageName as substring from start to the index of the test/benchmark name minus 1 - funcNameIndex := strings.LastIndex(funcFullName, funcName) - if funcNameIndex < 1 { - funcNameIndex = len(funcFullName) + lastSlash := strings.LastIndexByte(funcFullName, '/') + if lastSlash < 0 { + lastSlash = 0 } - packageName := funcFullName[:funcNameIndex-1] - + firstDot := strings.IndexByte(funcFullName[lastSlash:], '.') + lastSlash + packName := funcFullName[:firstDot] // If the package has the format: _/{path...} // We convert the path from absolute to relative to the source root sourceRoot := instrumentation.GetSourceRoot() - if len(packageName) > 0 && packageName[0] == '_' && strings.Index(packageName, sourceRoot) != -1 { - packageName = strings.Replace(packageName, path.Dir(sourceRoot)+"/", "", -1)[1:] + if len(packName) > 0 && packName[0] == '_' && strings.Index(packName, sourceRoot) != -1 { + packName = strings.Replace(packName, path.Dir(sourceRoot)+"/", "", -1)[1:] } - - return packageName + funcName := funcFullName[firstDot+1:] + return packName, funcName } -// Gets the source code boundaries of a test or benchmark in the format: {file}:{startLine}:{endLine} -func getTestCodeBoundaries(pc uintptr, fullName string) string { - funcName := getFuncName(fullName) - sourceBounds, err := ast.GetFuncSourceForName(pc, funcName) +func getPackageAndNameAndBoundaries(pc uintptr) (string, string, string) { + pName, fName := getPackageAndName(pc) + dotIndex := strings.IndexByte(fName, '.') + if dotIndex != -1 { + fName = fName[:dotIndex] + } + + fBoundaries := "" + sourceBounds, err := ast.GetFuncSourceForName(pc, fName) if err != nil { - instrumentation.Logger().Printf("error calculating the source boundaries for '%s [%s]': %v", funcName, fullName, err) + instrumentation.Logger().Printf("error calculating the source boundaries for '%s': %v", fName, err) } if sourceBounds != nil { - return fmt.Sprintf("%s:%d:%d", sourceBounds.File, sourceBounds.Start.Line, sourceBounds.End.Line) + fBoundaries = fmt.Sprintf("%s:%d:%d", sourceBounds.File, sourceBounds.Start.Line, sourceBounds.End.Line) } - return "" + return pName, fName, fBoundaries } diff --git a/instrumentation/testing/util_test.go b/instrumentation/testing/util_test.go index f8d7378f..0f2ee2dd 100644 --- a/instrumentation/testing/util_test.go +++ b/instrumentation/testing/util_test.go @@ -1,13 +1,14 @@ package testing import ( - "fmt" - "runtime" - "testing" + _ "fmt" + _ "runtime" + _ "testing" - "go.undefinedlabs.com/scopeagent/ast" + _ "go.undefinedlabs.com/scopeagent/ast" ) +/* func TestGetFuncName(t *testing.T) { cases := map[string]string{ "TestBase": "TestBase", @@ -66,3 +67,4 @@ func TestGetTestCodeBoundaries(t *testing.T) { t.Fatalf("value '%s' not expected", actualBoundary) } } +*/ From f073f4e3d83225e8fa2dfcd49d675f9c5f4270c9 Mon Sep 17 00:00:00 2001 From: Daniel Redondo Date: Tue, 31 Mar 2020 11:38:55 +0200 Subject: [PATCH 02/20] Get exact test start time --- instrumentation/testing/testing.go | 10 ++++++++++ reflection/reflect.go | 27 +++++++++++++++++++++++++++ tracer/span.go | 10 ++++++++++ 3 files changed, 47 insertions(+) diff --git a/instrumentation/testing/testing.go b/instrumentation/testing/testing.go index 583ef0e3..47f1f7fe 100644 --- a/instrumentation/testing/testing.go +++ b/instrumentation/testing/testing.go @@ -2,6 +2,7 @@ package testing import ( "context" + "go.undefinedlabs.com/scopeagent/tracer" "reflect" "regexp" "runtime" @@ -145,6 +146,15 @@ func (test *Test) Run(name string, f func(t *testing.T)) bool { func (test *Test) end() { finishTime := time.Now() + // If we have our own implementation of the span, we can set the exact start time from the test + if ownSpan, ok := test.span.(tracer.Span); ok { + if startTime, err := reflection.GetTestStartTime(test.t); err == nil { + ownSpan.SetStart(startTime) + } else { + instrumentation.Logger().Printf("error: %v", err) + } + } + // Remove the Test struct from the hash map, so a call to Start while we end this instance will create a new struct removeTest(test.t) // Stop and get records generated by loggers diff --git a/reflection/reflect.go b/reflection/reflect.go index ef281283..46a01bdd 100644 --- a/reflection/reflect.go +++ b/reflection/reflect.go @@ -5,6 +5,7 @@ import ( "reflect" "sync" "testing" + "time" "unsafe" ) @@ -66,6 +67,32 @@ func GetIsParallel(t *testing.T) bool { return false } +func GetTestStartTime(t *testing.T) (time.Time, error) { + mu := GetTestMutex(t) + if mu != nil { + mu.Lock() + defer mu.Unlock() + } + if pointer, err := GetFieldPointerOf(t, "start"); err == nil { + return *(*time.Time)(pointer), nil + } else { + return time.Time{}, err + } +} + +func GetTestDuration(t *testing.T) (time.Duration, error) { + mu := GetTestMutex(t) + if mu != nil { + mu.Lock() + defer mu.Unlock() + } + if pointer, err := GetFieldPointerOf(t, "duration"); err == nil { + return *(*time.Duration)(pointer), nil + } else { + return 0, err + } +} + func GetBenchmarkMutex(b *testing.B) *sync.RWMutex { if ptr, err := GetFieldPointerOf(b, "mu"); err == nil { return (*sync.RWMutex)(ptr) diff --git a/tracer/span.go b/tracer/span.go index 5bd9c11a..1fd3879d 100644 --- a/tracer/span.go +++ b/tracer/span.go @@ -22,6 +22,9 @@ type Span interface { // Start indicates when the span began Start() time.Time + + // Sets the start time + SetStart(start time.Time) opentracing.Span } // Implements the `Span` interface. Created via tracerImpl (see @@ -72,6 +75,13 @@ func (s *spanImpl) trim() bool { return !s.raw.Context.Sampled && s.tracer.options.TrimUnsampledSpans } +func (s *spanImpl) SetStart(start time.Time) opentracing.Span { + s.Lock() + defer s.Unlock() + s.raw.Start = start + return s +} + func (s *spanImpl) SetTag(key string, value interface{}) opentracing.Span { defer s.onTag(key, value) s.Lock() From 27b7d3fec37443f8fc081366ce3ca729ece6608c Mon Sep 17 00:00:00 2001 From: Daniel Redondo Date: Tue, 31 Mar 2020 15:15:21 +0200 Subject: [PATCH 03/20] Bump agent pre version --- agent/agent.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/agent.go b/agent/agent.go index ec8b522f..ae75c340 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -62,7 +62,7 @@ type ( ) var ( - version = "0.1.16-pre1" + version = "0.1.16-pre2" testingModeFrequency = time.Second nonTestingModeFrequency = time.Minute From 74a6fd6e7c16417ea31d2362f312d84b66e7a653 Mon Sep 17 00:00:00 2001 From: Daniel Redondo Date: Fri, 20 Mar 2020 16:17:56 +0100 Subject: [PATCH 04/20] Add an 'SetTestCodeFromCaller' API to set the test code --- init.go | 10 +++++ instrumentation/testing/benchmark.go | 9 +++-- instrumentation/testing/testing.go | 22 ++++++++--- instrumentation/testing/util.go | 55 +++++++++++----------------- instrumentation/testing/util_test.go | 10 +++-- 5 files changed, 59 insertions(+), 47 deletions(-) diff --git a/init.go b/init.go index fb3ae83d..85449eda 100644 --- a/init.go +++ b/init.go @@ -75,6 +75,16 @@ func GetContextFromTest(t *testing.T) context.Context { return context.TODO() } +// Sets the test code from the caller of this func +func SetTestCodeFromCaller(t *testing.T) { + test := GetTest(t) + if test == nil { + return + } + pc, _, _, _ := runtime.Caller(1) + test.SetTestCode(pc) +} + // Gets the *Benchmark from a *testing.B func GetBenchmark(b *testing.B) *scopetesting.Benchmark { return scopetesting.GetBenchmark(b) diff --git a/instrumentation/testing/benchmark.go b/instrumentation/testing/benchmark.go index 1bd122b7..a704f580 100644 --- a/instrumentation/testing/benchmark.go +++ b/instrumentation/testing/benchmark.go @@ -111,8 +111,10 @@ func startBenchmark(b *testing.B, pc uintptr, benchFunc func(b *testing.B)) { fullTestName = strings.Join(nameSegments, "/") } packageName := reflection.GetBenchmarkSuiteName(b) + pName, _, tCode := getPackageAndNameAndBoundaries(pc) + if packageName == "" { - packageName = getPackageName(pc, fullTestName) + packageName = pName } oTags := opentracing.Tags{ @@ -124,9 +126,8 @@ func startBenchmark(b *testing.B, pc uintptr, benchFunc func(b *testing.B)) { "test.type": "benchmark", } - testCode := getTestCodeBoundaries(pc, fullTestName) - if testCode != "" { - oTags["test.code"] = testCode + if tCode != "" { + oTags["test.code"] = tCode } var startOptions []opentracing.StartSpanOption diff --git a/instrumentation/testing/testing.go b/instrumentation/testing/testing.go index 04c882fb..583ef0e3 100644 --- a/instrumentation/testing/testing.go +++ b/instrumentation/testing/testing.go @@ -24,9 +24,10 @@ import ( type ( Test struct { testing.TB - ctx context.Context - span opentracing.Span - t *testing.T + ctx context.Context + span opentracing.Span + t *testing.T + codePC uintptr } Option func(*Test) @@ -64,6 +65,7 @@ func StartTestFromCaller(t *testing.T, pc uintptr, opts ...Option) *Test { // If there is already one we want to replace it, so we clear the context test.ctx = context.Background() } + test.codePC = pc for _, opt := range opts { opt(test) @@ -72,17 +74,16 @@ func StartTestFromCaller(t *testing.T, pc uintptr, opts ...Option) *Test { // Extracting the testing func name (by removing any possible sub-test suffix `{test_func}/{sub_test}`) // to search the func source code bounds and to calculate the package name. fullTestName := runner.GetOriginalTestName(t.Name()) - packageName := getPackageName(pc, fullTestName) + pName, _, testCode := getPackageAndNameAndBoundaries(pc) testTags := opentracing.Tags{ "span.kind": "test", "test.name": fullTestName, - "test.suite": packageName, + "test.suite": pName, "test.framework": "testing", "test.language": "go", } - testCode := getTestCodeBoundaries(pc, fullTestName) if testCode != "" { testTags["test.code"] = testCode } @@ -102,6 +103,15 @@ func StartTestFromCaller(t *testing.T, pc uintptr, opts ...Option) *Test { return test } +// Set test code +func (test *Test) SetTestCode(pc uintptr) { + pName, _, fBoundaries := getPackageAndNameAndBoundaries(pc) + test.span.SetTag("test.suite", pName) + if fBoundaries != "" { + test.span.SetTag("test.code", fBoundaries) + } +} + // Ends the current test func (test *Test) End() { autoInstrumentedTestsMutex.RLock() diff --git a/instrumentation/testing/util.go b/instrumentation/testing/util.go index 4ea946d7..ba5c209a 100644 --- a/instrumentation/testing/util.go +++ b/instrumentation/testing/util.go @@ -10,49 +10,38 @@ import ( "go.undefinedlabs.com/scopeagent/instrumentation" ) -// Gets the Test/Benchmark parent func name without sub-benchmark and sub-test segments -func getFuncName(fullName string) string { - testNameSlash := strings.IndexByte(fullName, '/') - funcName := fullName - if testNameSlash >= 0 { - funcName = fullName[:testNameSlash] - } - return funcName -} - -// Gets the Package name -func getPackageName(pc uintptr, fullName string) string { - // Parent test/benchmark name - funcName := getFuncName(fullName) - // Full func name (format ex: {packageName}.{test/benchmark name}.{inner function of sub benchmark/test} +func getPackageAndName(pc uintptr) (string, string) { funcFullName := runtime.FuncForPC(pc).Name() - - // We select the packageName as substring from start to the index of the test/benchmark name minus 1 - funcNameIndex := strings.LastIndex(funcFullName, funcName) - if funcNameIndex < 1 { - funcNameIndex = len(funcFullName) + lastSlash := strings.LastIndexByte(funcFullName, '/') + if lastSlash < 0 { + lastSlash = 0 } - packageName := funcFullName[:funcNameIndex-1] - + firstDot := strings.IndexByte(funcFullName[lastSlash:], '.') + lastSlash + packName := funcFullName[:firstDot] // If the package has the format: _/{path...} // We convert the path from absolute to relative to the source root sourceRoot := instrumentation.GetSourceRoot() - if len(packageName) > 0 && packageName[0] == '_' && strings.Index(packageName, sourceRoot) != -1 { - packageName = strings.Replace(packageName, path.Dir(sourceRoot)+"/", "", -1)[1:] + if len(packName) > 0 && packName[0] == '_' && strings.Index(packName, sourceRoot) != -1 { + packName = strings.Replace(packName, path.Dir(sourceRoot)+"/", "", -1)[1:] } - - return packageName + funcName := funcFullName[firstDot+1:] + return packName, funcName } -// Gets the source code boundaries of a test or benchmark in the format: {file}:{startLine}:{endLine} -func getTestCodeBoundaries(pc uintptr, fullName string) string { - funcName := getFuncName(fullName) - sourceBounds, err := ast.GetFuncSourceForName(pc, funcName) +func getPackageAndNameAndBoundaries(pc uintptr) (string, string, string) { + pName, fName := getPackageAndName(pc) + dotIndex := strings.IndexByte(fName, '.') + if dotIndex != -1 { + fName = fName[:dotIndex] + } + + fBoundaries := "" + sourceBounds, err := ast.GetFuncSourceForName(pc, fName) if err != nil { - instrumentation.Logger().Printf("error calculating the source boundaries for '%s [%s]': %v", funcName, fullName, err) + instrumentation.Logger().Printf("error calculating the source boundaries for '%s': %v", fName, err) } if sourceBounds != nil { - return fmt.Sprintf("%s:%d:%d", sourceBounds.File, sourceBounds.Start.Line, sourceBounds.End.Line) + fBoundaries = fmt.Sprintf("%s:%d:%d", sourceBounds.File, sourceBounds.Start.Line, sourceBounds.End.Line) } - return "" + return pName, fName, fBoundaries } diff --git a/instrumentation/testing/util_test.go b/instrumentation/testing/util_test.go index f8d7378f..0f2ee2dd 100644 --- a/instrumentation/testing/util_test.go +++ b/instrumentation/testing/util_test.go @@ -1,13 +1,14 @@ package testing import ( - "fmt" - "runtime" - "testing" + _ "fmt" + _ "runtime" + _ "testing" - "go.undefinedlabs.com/scopeagent/ast" + _ "go.undefinedlabs.com/scopeagent/ast" ) +/* func TestGetFuncName(t *testing.T) { cases := map[string]string{ "TestBase": "TestBase", @@ -66,3 +67,4 @@ func TestGetTestCodeBoundaries(t *testing.T) { t.Fatalf("value '%s' not expected", actualBoundary) } } +*/ From 0e83034e09325e0739018b971a0817677584be38 Mon Sep 17 00:00:00 2001 From: Daniel Redondo Date: Tue, 31 Mar 2020 11:38:55 +0200 Subject: [PATCH 05/20] Get exact test start time --- instrumentation/testing/testing.go | 10 ++++++++++ reflection/reflect.go | 27 +++++++++++++++++++++++++++ tracer/span.go | 10 ++++++++++ 3 files changed, 47 insertions(+) diff --git a/instrumentation/testing/testing.go b/instrumentation/testing/testing.go index 583ef0e3..47f1f7fe 100644 --- a/instrumentation/testing/testing.go +++ b/instrumentation/testing/testing.go @@ -2,6 +2,7 @@ package testing import ( "context" + "go.undefinedlabs.com/scopeagent/tracer" "reflect" "regexp" "runtime" @@ -145,6 +146,15 @@ func (test *Test) Run(name string, f func(t *testing.T)) bool { func (test *Test) end() { finishTime := time.Now() + // If we have our own implementation of the span, we can set the exact start time from the test + if ownSpan, ok := test.span.(tracer.Span); ok { + if startTime, err := reflection.GetTestStartTime(test.t); err == nil { + ownSpan.SetStart(startTime) + } else { + instrumentation.Logger().Printf("error: %v", err) + } + } + // Remove the Test struct from the hash map, so a call to Start while we end this instance will create a new struct removeTest(test.t) // Stop and get records generated by loggers diff --git a/reflection/reflect.go b/reflection/reflect.go index ef281283..46a01bdd 100644 --- a/reflection/reflect.go +++ b/reflection/reflect.go @@ -5,6 +5,7 @@ import ( "reflect" "sync" "testing" + "time" "unsafe" ) @@ -66,6 +67,32 @@ func GetIsParallel(t *testing.T) bool { return false } +func GetTestStartTime(t *testing.T) (time.Time, error) { + mu := GetTestMutex(t) + if mu != nil { + mu.Lock() + defer mu.Unlock() + } + if pointer, err := GetFieldPointerOf(t, "start"); err == nil { + return *(*time.Time)(pointer), nil + } else { + return time.Time{}, err + } +} + +func GetTestDuration(t *testing.T) (time.Duration, error) { + mu := GetTestMutex(t) + if mu != nil { + mu.Lock() + defer mu.Unlock() + } + if pointer, err := GetFieldPointerOf(t, "duration"); err == nil { + return *(*time.Duration)(pointer), nil + } else { + return 0, err + } +} + func GetBenchmarkMutex(b *testing.B) *sync.RWMutex { if ptr, err := GetFieldPointerOf(b, "mu"); err == nil { return (*sync.RWMutex)(ptr) diff --git a/tracer/span.go b/tracer/span.go index 5bd9c11a..1fd3879d 100644 --- a/tracer/span.go +++ b/tracer/span.go @@ -22,6 +22,9 @@ type Span interface { // Start indicates when the span began Start() time.Time + + // Sets the start time + SetStart(start time.Time) opentracing.Span } // Implements the `Span` interface. Created via tracerImpl (see @@ -72,6 +75,13 @@ func (s *spanImpl) trim() bool { return !s.raw.Context.Sampled && s.tracer.options.TrimUnsampledSpans } +func (s *spanImpl) SetStart(start time.Time) opentracing.Span { + s.Lock() + defer s.Unlock() + s.raw.Start = start + return s +} + func (s *spanImpl) SetTag(key string, value interface{}) opentracing.Span { defer s.onTag(key, value) s.Lock() From 03f0db1079b24c7a593bc131cb3923b9775317f0 Mon Sep 17 00:00:00 2001 From: Daniel Redondo Date: Tue, 31 Mar 2020 15:15:21 +0200 Subject: [PATCH 06/20] Bump agent pre version --- agent/agent.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/agent.go b/agent/agent.go index ec8b522f..ae75c340 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -62,7 +62,7 @@ type ( ) var ( - version = "0.1.16-pre1" + version = "0.1.16-pre2" testingModeFrequency = time.Second nonTestingModeFrequency = time.Minute From b45d98c12092d25e62ae6427e7822487d83a2b61 Mon Sep 17 00:00:00 2001 From: Daniel Redondo Date: Thu, 2 Apr 2020 09:17:48 +0200 Subject: [PATCH 07/20] Agent benchmark test --- agent/agent_test.go | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/agent/agent_test.go b/agent/agent_test.go index 7cb2f79a..0bd69b0c 100644 --- a/agent/agent_test.go +++ b/agent/agent_test.go @@ -5,6 +5,7 @@ import ( "os/exec" "reflect" "strings" + "sync" "testing" "time" @@ -141,3 +142,24 @@ func TestTildeExpandRaceMetadata(t *testing.T) { <-time.After(5 * time.Second) agent.Stop() } + +func BenchmarkNewAgent(b *testing.B) { + for i:=0; i < b.N; i++ { + a, err := NewAgent(WithTestingModeEnabled(), + WithHandlePanicAsFail(), + WithRetriesOnFail(3), + WithSetGlobalTracer()) + if err != nil { + b.Fatal(err) + } + span := a.Tracer().StartSpan("Test") + span.SetTag("span.kind", "test") + span.SetTag("test.name", "BenchNewAgent") + span.SetTag("test.suite", "root") + span.SetTag("test.status", tags.TestStatus_PASS) + span.SetBaggageItem("trace.kind", "test") + span.Finish() + once = sync.Once{} + a.Stop() + } +} \ No newline at end of file From d54ef86513dffe63683ffa4972c723f4d2104057 Mon Sep 17 00:00:00 2001 From: Daniel Redondo Date: Thu, 2 Apr 2020 09:29:11 +0200 Subject: [PATCH 08/20] Logger patch benchmark --- agent/agent_test.go | 4 +++- instrumentation/logging/logger_test.go | 16 ++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/agent/agent_test.go b/agent/agent_test.go index 0bd69b0c..0a7e56b8 100644 --- a/agent/agent_test.go +++ b/agent/agent_test.go @@ -143,9 +143,11 @@ func TestTildeExpandRaceMetadata(t *testing.T) { agent.Stop() } +var a *Agent func BenchmarkNewAgent(b *testing.B) { for i:=0; i < b.N; i++ { - a, err := NewAgent(WithTestingModeEnabled(), + var err error + a, err = NewAgent(WithTestingModeEnabled(), WithHandlePanicAsFail(), WithRetriesOnFail(3), WithSetGlobalTracer()) diff --git a/instrumentation/logging/logger_test.go b/instrumentation/logging/logger_test.go index e831f0f3..75fd8cb3 100644 --- a/instrumentation/logging/logger_test.go +++ b/instrumentation/logging/logger_test.go @@ -132,3 +132,19 @@ func checkMessage(msg string, records []opentracing.LogRecord) bool { } return false } + +func BenchmarkPatchStandardLogger(b *testing.B) { + for i:=0; i < b.N; i++ { + PatchStandardLogger() + UnpatchStandardLogger() + } +} + +var lg *stdlog.Logger +func BenchmarkPatchLogger(b *testing.B) { + for i:=0; i < b.N; i++ { + lg = stdlog.New(os.Stdout, "", stdlog.Llongfile | stdlog.Lmicroseconds) + PatchLogger(lg) + UnpatchLogger(lg) + } +} \ No newline at end of file From a5dc5c4b49b20c02a4e0cc8bc59b9b8601f728b2 Mon Sep 17 00:00:00 2001 From: Daniel Redondo Date: Thu, 2 Apr 2020 09:42:44 +0200 Subject: [PATCH 09/20] SetTestCodeFromCallerSkip and SetTestCodeFromFunc --- agent/agent_test.go | 5 +++-- init.go | 19 ++++++++++++++++++- instrumentation/logging/logger_test.go | 9 +++++---- 3 files changed, 26 insertions(+), 7 deletions(-) diff --git a/agent/agent_test.go b/agent/agent_test.go index 0a7e56b8..a9a420db 100644 --- a/agent/agent_test.go +++ b/agent/agent_test.go @@ -144,8 +144,9 @@ func TestTildeExpandRaceMetadata(t *testing.T) { } var a *Agent + func BenchmarkNewAgent(b *testing.B) { - for i:=0; i < b.N; i++ { + for i := 0; i < b.N; i++ { var err error a, err = NewAgent(WithTestingModeEnabled(), WithHandlePanicAsFail(), @@ -164,4 +165,4 @@ func BenchmarkNewAgent(b *testing.B) { once = sync.Once{} a.Stop() } -} \ No newline at end of file +} diff --git a/init.go b/init.go index 85449eda..995a00ad 100644 --- a/init.go +++ b/init.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "os/signal" + "reflect" "runtime" "sync" "syscall" @@ -77,11 +78,27 @@ func GetContextFromTest(t *testing.T) context.Context { // Sets the test code from the caller of this func func SetTestCodeFromCaller(t *testing.T) { + SetTestCodeFromCallerSkip(t, 1) +} + +// Sets the test code from the caller of this func +func SetTestCodeFromCallerSkip(t *testing.T, skip int) { test := GetTest(t) if test == nil { return } - pc, _, _, _ := runtime.Caller(1) + pc, _, _, _ := runtime.Caller(skip + 1) + test.SetTestCode(pc) +} + +// Sets the test code from a func +func SetTestCodeFromFunc(t *testing.T, fn interface{}) { + test := GetTest(t) + if test == nil { + return + } + value := reflect.ValueOf(fn) + pc := value.Pointer() test.SetTestCode(pc) } diff --git a/instrumentation/logging/logger_test.go b/instrumentation/logging/logger_test.go index 75fd8cb3..c1a6399a 100644 --- a/instrumentation/logging/logger_test.go +++ b/instrumentation/logging/logger_test.go @@ -134,17 +134,18 @@ func checkMessage(msg string, records []opentracing.LogRecord) bool { } func BenchmarkPatchStandardLogger(b *testing.B) { - for i:=0; i < b.N; i++ { + for i := 0; i < b.N; i++ { PatchStandardLogger() UnpatchStandardLogger() } } var lg *stdlog.Logger + func BenchmarkPatchLogger(b *testing.B) { - for i:=0; i < b.N; i++ { - lg = stdlog.New(os.Stdout, "", stdlog.Llongfile | stdlog.Lmicroseconds) + for i := 0; i < b.N; i++ { + lg = stdlog.New(os.Stdout, "", stdlog.Llongfile|stdlog.Lmicroseconds) PatchLogger(lg) UnpatchLogger(lg) } -} \ No newline at end of file +} From 214ddd595a3dc9e6320f8ade89f1248c47ed324a Mon Sep 17 00:00:00 2001 From: Daniel Redondo Date: Thu, 2 Apr 2020 13:29:15 +0200 Subject: [PATCH 10/20] Test instrumentation init benchmark --- instrumentation/testing/testing_test.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/instrumentation/testing/testing_test.go b/instrumentation/testing/testing_test.go index 965c0d8f..47cfa7a7 100644 --- a/instrumentation/testing/testing_test.go +++ b/instrumentation/testing/testing_test.go @@ -30,3 +30,23 @@ func TestLogBufferRegex(t *testing.T) { func TestExtractSubTestLogBuffer(t *testing.T) { t.Run("SubTest", TestLogBufferRegex) } + +func BenchmarkTestInit(b *testing.B) { + for i:=0; i < b.N; i++ { + tests := append(make([]testing.InternalTest, 0), + testing.InternalTest{ Name: "Test01", F: func(t *testing.T) {} }, + testing.InternalTest{ Name: "Test02", F: func(t *testing.T) {} }, + testing.InternalTest{ Name: "Test03", F: func(t *testing.T) {} }, + testing.InternalTest{ Name: "Test04", F: func(t *testing.T) {} }, + testing.InternalTest{ Name: "Test05", F: func(t *testing.T) {} }, + ) + benchmarks := append(make([]testing.InternalBenchmark, 0), + testing.InternalBenchmark{ Name: "Test01", F: func(b *testing.B) {} }, + testing.InternalBenchmark{ Name: "Test02", F: func(b *testing.B) {} }, + testing.InternalBenchmark{ Name: "Test03", F: func(b *testing.B) {} }, + testing.InternalBenchmark{ Name: "Test04", F: func(b *testing.B) {} }, + testing.InternalBenchmark{ Name: "Test05", F: func(b *testing.B) {} }, + ) + Init(testing.MainStart(nil, tests, benchmarks, nil)) + } +} \ No newline at end of file From fab4ad3167be84cd07742ad01ac8baa7469cf6ed Mon Sep 17 00:00:00 2001 From: Daniel Redondo Date: Thu, 2 Apr 2020 18:15:43 +0200 Subject: [PATCH 11/20] Benchmark dependencies and git tests --- agent/dependencies_test.go | 11 +++++++++++ agent/git_test.go | 15 +++++++++++++++ 2 files changed, 26 insertions(+) create mode 100644 agent/dependencies_test.go diff --git a/agent/dependencies_test.go b/agent/dependencies_test.go new file mode 100644 index 00000000..b6afc654 --- /dev/null +++ b/agent/dependencies_test.go @@ -0,0 +1,11 @@ +package agent + +import "testing" + +var mp map[string]string + +func BenchmarkGetDependencyMap(b *testing.B) { + for i:=0; i < b.N; i++ { + mp = getDependencyMap() + } +} \ No newline at end of file diff --git a/agent/git_test.go b/agent/git_test.go index e7788ba4..30c13b32 100644 --- a/agent/git_test.go +++ b/agent/git_test.go @@ -108,3 +108,18 @@ func TestMergeRegex(t *testing.T) { } } } + +var data *GitData +var diff *GitDiff + +func BenchmarkGetGitData(b *testing.B) { + for i:=0; i < b.N; i++ { + data = getGitData() + } +} + +func BenchmarkGetGitDiff(b *testing.B) { + for i:=0; i < b.N; i++ { + diff = getGitDiff() + } +} \ No newline at end of file From 7d2ff2df1a126c83e7a577b9db88f57d73f0525a Mon Sep 17 00:00:00 2001 From: Daniel Redondo Date: Thu, 2 Apr 2020 18:43:13 +0200 Subject: [PATCH 12/20] Benchmark monkey patching testing logger --- instrumentation/testing/testing_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/instrumentation/testing/testing_test.go b/instrumentation/testing/testing_test.go index 47cfa7a7..9e22421e 100644 --- a/instrumentation/testing/testing_test.go +++ b/instrumentation/testing/testing_test.go @@ -49,4 +49,11 @@ func BenchmarkTestInit(b *testing.B) { ) Init(testing.MainStart(nil, tests, benchmarks, nil)) } +} + +func BenchmarkLoggerPatcher(b *testing.B) { + for i:=0; i < b.N; i++ { + PatchTestingLogger() + UnpatchTestingLogger() + } } \ No newline at end of file From 26226d4ee9cd8446d0a77a4d9112aace3b4c5b1e Mon Sep 17 00:00:00 2001 From: Daniel Redondo Date: Thu, 2 Apr 2020 18:58:27 +0200 Subject: [PATCH 13/20] Test and Benchmark Logger Patcher --- agent/dependencies_test.go | 4 +-- agent/git_test.go | 6 ++-- instrumentation/testing/testing_test.go | 41 +++++++++++++++++-------- 3 files changed, 33 insertions(+), 18 deletions(-) diff --git a/agent/dependencies_test.go b/agent/dependencies_test.go index b6afc654..deaeb82d 100644 --- a/agent/dependencies_test.go +++ b/agent/dependencies_test.go @@ -5,7 +5,7 @@ import "testing" var mp map[string]string func BenchmarkGetDependencyMap(b *testing.B) { - for i:=0; i < b.N; i++ { + for i := 0; i < b.N; i++ { mp = getDependencyMap() } -} \ No newline at end of file +} diff --git a/agent/git_test.go b/agent/git_test.go index 30c13b32..a53d05fa 100644 --- a/agent/git_test.go +++ b/agent/git_test.go @@ -113,13 +113,13 @@ var data *GitData var diff *GitDiff func BenchmarkGetGitData(b *testing.B) { - for i:=0; i < b.N; i++ { + for i := 0; i < b.N; i++ { data = getGitData() } } func BenchmarkGetGitDiff(b *testing.B) { - for i:=0; i < b.N; i++ { + for i := 0; i < b.N; i++ { diff = getGitDiff() } -} \ No newline at end of file +} diff --git a/instrumentation/testing/testing_test.go b/instrumentation/testing/testing_test.go index 9e22421e..358c5c85 100644 --- a/instrumentation/testing/testing_test.go +++ b/instrumentation/testing/testing_test.go @@ -1,6 +1,7 @@ package testing import ( + "sync" "testing" ) @@ -32,28 +33,42 @@ func TestExtractSubTestLogBuffer(t *testing.T) { } func BenchmarkTestInit(b *testing.B) { - for i:=0; i < b.N; i++ { + for i := 0; i < b.N; i++ { tests := append(make([]testing.InternalTest, 0), - testing.InternalTest{ Name: "Test01", F: func(t *testing.T) {} }, - testing.InternalTest{ Name: "Test02", F: func(t *testing.T) {} }, - testing.InternalTest{ Name: "Test03", F: func(t *testing.T) {} }, - testing.InternalTest{ Name: "Test04", F: func(t *testing.T) {} }, - testing.InternalTest{ Name: "Test05", F: func(t *testing.T) {} }, + testing.InternalTest{Name: "Test01", F: func(t *testing.T) {}}, + testing.InternalTest{Name: "Test02", F: func(t *testing.T) {}}, + testing.InternalTest{Name: "Test03", F: func(t *testing.T) {}}, + testing.InternalTest{Name: "Test04", F: func(t *testing.T) {}}, + testing.InternalTest{Name: "Test05", F: func(t *testing.T) {}}, ) benchmarks := append(make([]testing.InternalBenchmark, 0), - testing.InternalBenchmark{ Name: "Test01", F: func(b *testing.B) {} }, - testing.InternalBenchmark{ Name: "Test02", F: func(b *testing.B) {} }, - testing.InternalBenchmark{ Name: "Test03", F: func(b *testing.B) {} }, - testing.InternalBenchmark{ Name: "Test04", F: func(b *testing.B) {} }, - testing.InternalBenchmark{ Name: "Test05", F: func(b *testing.B) {} }, + testing.InternalBenchmark{Name: "Test01", F: func(b *testing.B) {}}, + testing.InternalBenchmark{Name: "Test02", F: func(b *testing.B) {}}, + testing.InternalBenchmark{Name: "Test03", F: func(b *testing.B) {}}, + testing.InternalBenchmark{Name: "Test04", F: func(b *testing.B) {}}, + testing.InternalBenchmark{Name: "Test05", F: func(b *testing.B) {}}, ) Init(testing.MainStart(nil, tests, benchmarks, nil)) } } func BenchmarkLoggerPatcher(b *testing.B) { - for i:=0; i < b.N; i++ { + for i := 0; i < b.N; i++ { PatchTestingLogger() UnpatchTestingLogger() } -} \ No newline at end of file +} + +func TestLoggerPatcher(t *testing.T) { + PatchTestingLogger() + wg := sync.WaitGroup{} + for i := 0; i < 1000; i++ { + wg.Add(1) + go func() { + defer wg.Done() + t.Log("Hello world") + }() + } + wg.Wait() + UnpatchTestingLogger() +} From 4f4439fa791bc4ce5bae991e8365ae84a0e45d35 Mon Sep 17 00:00:00 2001 From: Daniel Redondo Date: Thu, 2 Apr 2020 21:43:41 +0200 Subject: [PATCH 14/20] Reflection test --- instrumentation/testing/testing_test.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/instrumentation/testing/testing_test.go b/instrumentation/testing/testing_test.go index 358c5c85..db8a6427 100644 --- a/instrumentation/testing/testing_test.go +++ b/instrumentation/testing/testing_test.go @@ -1,8 +1,10 @@ package testing import ( + "go.undefinedlabs.com/scopeagent/reflection" "sync" "testing" + "time" ) func TestLogBufferRegex(t *testing.T) { @@ -60,6 +62,7 @@ func BenchmarkLoggerPatcher(b *testing.B) { } func TestLoggerPatcher(t *testing.T) { + tm := time.Now() PatchTestingLogger() wg := sync.WaitGroup{} for i := 0; i < 1000; i++ { @@ -71,4 +74,22 @@ func TestLoggerPatcher(t *testing.T) { } wg.Wait() UnpatchTestingLogger() + if time.Since(tm) > time.Second { + t.Fatal("Test is too slow") + } +} + +func TestIsParallelByReflection(t *testing.T) { + t.Parallel() + tm := time.Now() + isParallel := false + for i := 0; i < 10000; i++ { + isParallel = reflection.GetIsParallel(t) + } + if !isParallel { + t.Fatal("test should be parallel") + } + if time.Since(tm) > time.Second { + t.Fatal("Test is too slow") + } } From 47f954db5796908e98cf1936fd8ab11bd202f527 Mon Sep 17 00:00:00 2001 From: Daniel Redondo Date: Thu, 2 Apr 2020 21:52:47 +0200 Subject: [PATCH 15/20] ParallelByReflection test --- instrumentation/testing/testing_test.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/instrumentation/testing/testing_test.go b/instrumentation/testing/testing_test.go index db8a6427..4b54daa0 100644 --- a/instrumentation/testing/testing_test.go +++ b/instrumentation/testing/testing_test.go @@ -83,9 +83,15 @@ func TestIsParallelByReflection(t *testing.T) { t.Parallel() tm := time.Now() isParallel := false + wg := sync.WaitGroup{} for i := 0; i < 10000; i++ { - isParallel = reflection.GetIsParallel(t) + wg.Add(1) + go func() { + defer wg.Done() + isParallel = reflection.GetIsParallel(t) + }() } + wg.Wait() if !isParallel { t.Fatal("test should be parallel") } From 5b4023eab3d70af0eddb43be8bd8bae835d7fb2d Mon Sep 17 00:00:00 2001 From: Daniel Redondo Date: Thu, 2 Apr 2020 21:56:19 +0200 Subject: [PATCH 16/20] remove data race --- instrumentation/testing/testing_test.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/instrumentation/testing/testing_test.go b/instrumentation/testing/testing_test.go index 4b54daa0..9eb56e74 100644 --- a/instrumentation/testing/testing_test.go +++ b/instrumentation/testing/testing_test.go @@ -82,19 +82,15 @@ func TestLoggerPatcher(t *testing.T) { func TestIsParallelByReflection(t *testing.T) { t.Parallel() tm := time.Now() - isParallel := false wg := sync.WaitGroup{} for i := 0; i < 10000; i++ { wg.Add(1) go func() { defer wg.Done() - isParallel = reflection.GetIsParallel(t) + _ = reflection.GetIsParallel(t) }() } wg.Wait() - if !isParallel { - t.Fatal("test should be parallel") - } if time.Since(tm) > time.Second { t.Fatal("Test is too slow") } From acf6acd87f985c1d787b8e97817780c1ac20f7e3 Mon Sep 17 00:00:00 2001 From: Daniel Redondo Date: Fri, 3 Apr 2020 13:23:20 +0200 Subject: [PATCH 17/20] Remove reflection.Value arguments usage outside the dynamic func call --- go.mod | 3 -- instrumentation/testing/logger.go | 58 +++++++++++++------------ instrumentation/testing/testing_test.go | 7 +-- 3 files changed, 34 insertions(+), 34 deletions(-) diff --git a/go.mod b/go.mod index 5bfa1e54..a990d6d7 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-20200326085307-1a86426f42a6 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 08665537..1cb81736 100644 --- a/instrumentation/testing/logger.go +++ b/instrumentation/testing/logger.go @@ -1,13 +1,12 @@ package testing import ( + "github.com/undefinedlabs/go-mpatch" "reflect" "sync" "testing" "unsafe" - "github.com/undefinedlabs/go-mpatch" - "go.undefinedlabs.com/scopeagent/instrumentation" "go.undefinedlabs.com/scopeagent/reflection" ) @@ -52,84 +51,86 @@ func UnpatchTestingLogger() { } func patchError() { - patch("Error", func(test *Test, argsValues []reflect.Value) { + patch("Error", func(test *Test, args []interface{}) { test.t.Helper() - args := getArgs(argsValues[0]) test.Error(args...) }) } func patchErrorf() { - patch("Errorf", func(test *Test, argsValues []reflect.Value) { + patch("Errorf", func(test *Test, args []interface{}) { test.t.Helper() - format := argsValues[0].String() - args := getArgs(argsValues[1]) + format := args[0].(string) + args = args[1:] test.Errorf(format, args...) }) } func patchFatal() { - patch("Fatal", func(test *Test, argsValues []reflect.Value) { + patch("Fatal", func(test *Test, args []interface{}) { test.t.Helper() - args := getArgs(argsValues[0]) test.Fatal(args...) }) } func patchFatalf() { - patch("Fatalf", func(test *Test, argsValues []reflect.Value) { + patch("Fatalf", func(test *Test, args []interface{}) { test.t.Helper() - format := argsValues[0].String() - args := getArgs(argsValues[1]) + format := args[0].(string) + args = args[1:] test.Fatalf(format, args...) }) } func patchLog() { - patch("Log", func(test *Test, argsValues []reflect.Value) { + patch("Log", func(test *Test, args []interface{}) { test.t.Helper() - args := getArgs(argsValues[0]) test.Log(args...) }) } func patchLogf() { - patch("Logf", func(test *Test, argsValues []reflect.Value) { + patch("Logf", func(test *Test, args []interface{}) { test.t.Helper() - format := argsValues[0].String() - args := getArgs(argsValues[1]) + format := args[0].(string) + args = args[1:] test.Logf(format, args...) }) } func patchSkip() { - patch("Skip", func(test *Test, argsValues []reflect.Value) { + patch("Skip", func(test *Test, args []interface{}) { test.t.Helper() - args := getArgs(argsValues[0]) test.Skip(args...) }) } func patchSkipf() { - patch("Skipf", func(test *Test, argsValues []reflect.Value) { + patch("Skipf", func(test *Test, args []interface{}) { test.t.Helper() - format := argsValues[0].String() - args := getArgs(argsValues[1]) + format := args[0].(string) + args = args[1:] test.Skipf(format, args...) }) } -func getArgs(in reflect.Value) []interface{} { +func createArgs(in []reflect.Value) []interface{} { var args []interface{} - if in.Kind() == reflect.Slice { - for i := 0; i < in.Len(); i++ { - args = append(args, in.Index(i).Interface()) + for _, item := range in { + if item.Kind() == reflect.Slice { + var itemArg []interface{} + for i := 0; i < item.Len(); i++ { + itemArg = append(itemArg, item.Index(i).Interface()) + } + args = append(args, itemArg) + } else { + args = append(args, item.Interface()) } } return args } -func patch(methodName string, methodBody func(test *Test, argsValues []reflect.Value)) { +func patch(methodName string, methodBody func(test *Test, argsValues []interface{})) { patchesMutex.Lock() defer patchesMutex.Unlock() patchPointersMutex.Lock() @@ -144,6 +145,7 @@ func patch(methodName string, methodBody func(test *Test, argsValues []reflect.V var methodPatch *mpatch.Patch var err error methodPatch, err = mpatch.PatchMethodWithMakeFunc(method, func(in []reflect.Value) []reflect.Value { + argIn := createArgs(in[1:]) t := (*testing.T)(unsafe.Pointer(in[0].Pointer())) if t == nil { instrumentation.Logger().Println("testing.T is nil") @@ -161,7 +163,7 @@ func patch(methodName string, methodBody func(test *Test, argsValues []reflect.V instrumentation.Logger().Printf("test struct for %v doesn't exist\n", t.Name()) return nil } - methodBody(test, in[1:]) + methodBody(test, argIn) return nil }) logOnError(err) diff --git a/instrumentation/testing/testing_test.go b/instrumentation/testing/testing_test.go index 9eb56e74..0929ff02 100644 --- a/instrumentation/testing/testing_test.go +++ b/instrumentation/testing/testing_test.go @@ -1,6 +1,7 @@ package testing import ( + "fmt" "go.undefinedlabs.com/scopeagent/reflection" "sync" "testing" @@ -67,10 +68,10 @@ func TestLoggerPatcher(t *testing.T) { wg := sync.WaitGroup{} for i := 0; i < 1000; i++ { wg.Add(1) - go func() { + go func(x int) { defer wg.Done() - t.Log("Hello world") - }() + t.Log(fmt.Sprintf("Hello world %d", x)) + }(i) } wg.Wait() UnpatchTestingLogger() From 3a3ac115da8e4ab7d0604f849e7b75590b5dcc95 Mon Sep 17 00:00:00 2001 From: Daniel Redondo Date: Fri, 3 Apr 2020 13:27:33 +0200 Subject: [PATCH 18/20] format fixes --- instrumentation/testing/logger.go | 15 ++++++--------- instrumentation/testing/testing_test.go | 5 +++-- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/instrumentation/testing/logger.go b/instrumentation/testing/logger.go index 1cb81736..7603de81 100644 --- a/instrumentation/testing/logger.go +++ b/instrumentation/testing/logger.go @@ -1,12 +1,13 @@ package testing import ( - "github.com/undefinedlabs/go-mpatch" "reflect" "sync" "testing" "unsafe" + "github.com/undefinedlabs/go-mpatch" + "go.undefinedlabs.com/scopeagent/instrumentation" "go.undefinedlabs.com/scopeagent/reflection" ) @@ -61,8 +62,7 @@ func patchErrorf() { patch("Errorf", func(test *Test, args []interface{}) { test.t.Helper() format := args[0].(string) - args = args[1:] - test.Errorf(format, args...) + test.Errorf(format, args[1:]...) }) } @@ -77,8 +77,7 @@ func patchFatalf() { patch("Fatalf", func(test *Test, args []interface{}) { test.t.Helper() format := args[0].(string) - args = args[1:] - test.Fatalf(format, args...) + test.Fatalf(format, args[1:]...) }) } @@ -93,8 +92,7 @@ func patchLogf() { patch("Logf", func(test *Test, args []interface{}) { test.t.Helper() format := args[0].(string) - args = args[1:] - test.Logf(format, args...) + test.Logf(format, args[1:]...) }) } @@ -109,8 +107,7 @@ func patchSkipf() { patch("Skipf", func(test *Test, args []interface{}) { test.t.Helper() format := args[0].(string) - args = args[1:] - test.Skipf(format, args...) + test.Skipf(format, args[1:]...) }) } diff --git a/instrumentation/testing/testing_test.go b/instrumentation/testing/testing_test.go index 0929ff02..47bd05c0 100644 --- a/instrumentation/testing/testing_test.go +++ b/instrumentation/testing/testing_test.go @@ -2,10 +2,11 @@ package testing import ( "fmt" - "go.undefinedlabs.com/scopeagent/reflection" "sync" "testing" "time" + + "go.undefinedlabs.com/scopeagent/reflection" ) func TestLogBufferRegex(t *testing.T) { @@ -75,7 +76,7 @@ func TestLoggerPatcher(t *testing.T) { } wg.Wait() UnpatchTestingLogger() - if time.Since(tm) > time.Second { + if time.Since(tm) > 2*time.Second { t.Fatal("Test is too slow") } } From e880c46a9156025f44196310232b324096918bbe Mon Sep 17 00:00:00 2001 From: Daniel Redondo Date: Fri, 3 Apr 2020 13:30:49 +0200 Subject: [PATCH 19/20] test fix --- instrumentation/testing/testing_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/testing/testing_test.go b/instrumentation/testing/testing_test.go index 47bd05c0..d9dae4d5 100644 --- a/instrumentation/testing/testing_test.go +++ b/instrumentation/testing/testing_test.go @@ -85,7 +85,7 @@ func TestIsParallelByReflection(t *testing.T) { t.Parallel() tm := time.Now() wg := sync.WaitGroup{} - for i := 0; i < 10000; i++ { + for i := 0; i < 1000; i++ { wg.Add(1) go func() { defer wg.Done() From 7615f60c856d4fc35d9a1edda4ff27ae8b510eb3 Mon Sep 17 00:00:00 2001 From: Daniel Redondo Date: Fri, 3 Apr 2020 13:57:22 +0200 Subject: [PATCH 20/20] Fix util tests --- instrumentation/testing/testing.go | 2 +- instrumentation/testing/util.go | 5 ++- instrumentation/testing/util_test.go | 67 ++++++++++------------------ 3 files changed, 28 insertions(+), 46 deletions(-) diff --git a/instrumentation/testing/testing.go b/instrumentation/testing/testing.go index 47f1f7fe..16a7ca30 100644 --- a/instrumentation/testing/testing.go +++ b/instrumentation/testing/testing.go @@ -2,7 +2,6 @@ package testing import ( "context" - "go.undefinedlabs.com/scopeagent/tracer" "reflect" "regexp" "runtime" @@ -20,6 +19,7 @@ import ( "go.undefinedlabs.com/scopeagent/reflection" "go.undefinedlabs.com/scopeagent/runner" "go.undefinedlabs.com/scopeagent/tags" + "go.undefinedlabs.com/scopeagent/tracer" ) type ( diff --git a/instrumentation/testing/util.go b/instrumentation/testing/util.go index ba5c209a..974f71bb 100644 --- a/instrumentation/testing/util.go +++ b/instrumentation/testing/util.go @@ -11,7 +11,10 @@ import ( ) func getPackageAndName(pc uintptr) (string, string) { - funcFullName := runtime.FuncForPC(pc).Name() + return splitPackageAndName(runtime.FuncForPC(pc).Name()) +} + +func splitPackageAndName(funcFullName string) (string, string) { lastSlash := strings.LastIndexByte(funcFullName, '/') if lastSlash < 0 { lastSlash = 0 diff --git a/instrumentation/testing/util_test.go b/instrumentation/testing/util_test.go index 0f2ee2dd..27d16cfc 100644 --- a/instrumentation/testing/util_test.go +++ b/instrumentation/testing/util_test.go @@ -1,70 +1,49 @@ package testing import ( - _ "fmt" - _ "runtime" - _ "testing" + "fmt" + "runtime" + "testing" - _ "go.undefinedlabs.com/scopeagent/ast" + "go.undefinedlabs.com/scopeagent/ast" ) -/* -func TestGetFuncName(t *testing.T) { - cases := map[string]string{ - "TestBase": "TestBase", - "TestBase/Sub01": "TestBase", - "TestBase/Sub 02": "TestBase", - "TestBase/Sub/Sub02": "TestBase", - "TestBase/Sub/Sub02/Sub03": "TestBase", - "TestBase/Sub/Sub02/Sub03/S u b 0 4": "TestBase", +func TestSplitPackageAndName(t *testing.T) { + cases := map[string][]string{ + "pkg.TestBase": {"pkg", "TestBase"}, + "pkg.TestBase.func1": {"pkg", "TestBase.func1"}, + "github.com/org/proj.TestBase": {"github.com/org/proj", "TestBase"}, } for key, expected := range cases { t.Run(key, func(t *testing.T) { - value := getFuncName(key) - if value != expected { - t.Fatalf("value '%s', expected: '%s'", value, expected) + pkg, fname := splitPackageAndName(key) + if pkg != expected[0] { + t.Fatalf("value '%s', expected: '%s'", pkg, expected[0]) + } + if fname != expected[1] { + t.Fatalf("value '%s', expected: '%s'", fname, expected[1]) } }) } } -func TestGetPackageName(t *testing.T) { +func TestGetTestCodeBoundaries(t *testing.T) { var pc uintptr func() { pc, _, _, _ = runtime.Caller(1) }() testName := t.Name() - packName := getPackageName(pc, testName) - subTestName := "" - t.Run("sub-test", func(t *testing.T) { - subTestName = t.Name() - }) - packName02 := getPackageName(pc, subTestName) + pkg, fname, bound := getPackageAndNameAndBoundaries(pc) - if testName != "TestGetPackageName" { - t.Fatalf("value '%s' not expected", testName) - } - if subTestName != "TestGetPackageName/sub-test" { - t.Fatalf("value '%s' not expected", testName) + if pkg != "go.undefinedlabs.com/scopeagent/instrumentation/testing" { + t.Fatalf("value '%s' not expected", pkg) } - if packName != "go.undefinedlabs.com/scopeagent/instrumentation/testing" { - t.Fatalf("value '%s' not expected", packName) + if fname != testName { + t.Fatalf("value '%s' not expected", fname) } - if packName != packName02 { - t.Fatalf("value '%s' not expected", packName02) - } -} - -func TestGetTestCodeBoundaries(t *testing.T) { - var pc uintptr - func() { pc, _, _, _ = runtime.Caller(1) }() - testName := t.Name() - - actualBoundary := getTestCodeBoundaries(pc, testName) boundaryExpected, _ := ast.GetFuncSourceForName(pc, testName) calcExpected := fmt.Sprintf("%s:%d:%d", boundaryExpected.File, boundaryExpected.Start.Line, boundaryExpected.End.Line) - if actualBoundary != calcExpected { - t.Fatalf("value '%s' not expected", actualBoundary) + if bound != calcExpected { + t.Fatalf("value '%s' not expected", bound) } } -*/