From 3ca01f4bc352e36e9d80f87d80b7de57d967fb7a Mon Sep 17 00:00:00 2001 From: Mike Auclair Date: Mon, 24 Jun 2024 16:39:18 +0000 Subject: [PATCH 1/6] Stop querying for stack frames multiple times on CallerInfo() --- assert/assertions.go | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/assert/assertions.go b/assert/assertions.go index 104a0c936..cf4d80fe0 100644 --- a/assert/assertions.go +++ b/assert/assertions.go @@ -212,19 +212,23 @@ the problem actually occurred in calling code.*/ func CallerInfo() []string { var pc uintptr - var ok bool var file string var line int var name string callers := []string{} - for i := 0; ; i++ { - pc, file, line, ok = runtime.Caller(i) - if !ok { - // The breaks below failed to terminate the loop, and we ran off the - // end of the call stack. - break - } + pcs := []uintptr{} + n := runtime.Callers(0, pcs) + if n == 0 { + return []string{} + } + frames := runtime.CallersFrames(pcs[:n]) + + for { + frame, more := frames.Next() + pc = frame.PC + file = frame.File + line = frame.Line // This is a huge edge case, but it will panic if this is the case, see #180 if file == "" { @@ -263,6 +267,10 @@ func CallerInfo() []string { isTest(name, "Example") { break } + + if !more { + break + } } return callers From 4a90eff4ae12f3ed4bc9331e2f8ba368317fe86e Mon Sep 17 00:00:00 2001 From: Mike Auclair Date: Mon, 24 Jun 2024 16:50:27 +0000 Subject: [PATCH 2/6] fix --- assert/assertions.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/assert/assertions.go b/assert/assertions.go index cf4d80fe0..02dc37487 100644 --- a/assert/assertions.go +++ b/assert/assertions.go @@ -217,8 +217,8 @@ func CallerInfo() []string { var name string callers := []string{} - pcs := []uintptr{} - n := runtime.Callers(0, pcs) + pcs := make([]uintptr, 50) + n := runtime.Callers(1, pcs) if n == 0 { return []string{} } From 28e0be50927fb120f1275d34c648d0abbd166810 Mon Sep 17 00:00:00 2001 From: Mike Auclair Date: Mon, 24 Jun 2024 18:11:07 +0000 Subject: [PATCH 3/6] refill stack frame buffer after it's exhausted --- assert/assertions.go | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/assert/assertions.go b/assert/assertions.go index 02dc37487..1013f601b 100644 --- a/assert/assertions.go +++ b/assert/assertions.go @@ -24,6 +24,8 @@ import ( "github.com/stretchr/testify/assert/yaml" ) +const stackFrameBufferSize = 10 + //go:generate sh -c "cd ../_codegen && go build && cd - && ../_codegen/_codegen -output-package=assert -template=assertion_format.go.tmpl" // TestingT is an interface wrapper around *testing.T @@ -217,8 +219,14 @@ func CallerInfo() []string { var name string callers := []string{} - pcs := make([]uintptr, 50) - n := runtime.Callers(1, pcs) + pcs := make([]uintptr, stackFrameBufferSize) + offset := 1 + n := runtime.Callers(offset, pcs) + maybeMore := true + if n < stackFrameBufferSize { + maybeMore = false + } + if n == 0 { return []string{} } @@ -269,8 +277,22 @@ func CallerInfo() []string { } if !more { - break + // We know we already have less than a buffer's worth of frames + if !maybeMore { + break + } + offset += stackFrameBufferSize + n = runtime.Callers(offset, pcs) + if n < stackFrameBufferSize { + maybeMore = false + } + + if n == 0 { + break + } + frames = runtime.CallersFrames(pcs[:n]) } + } return callers From 176474a4c9bcd6b4d1daf36d902dd773171fb7c6 Mon Sep 17 00:00:00 2001 From: Mike Auclair Date: Mon, 24 Jun 2024 20:42:26 +0000 Subject: [PATCH 4/6] cleanup --- assert/assertions.go | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/assert/assertions.go b/assert/assertions.go index 1013f601b..461182210 100644 --- a/assert/assertions.go +++ b/assert/assertions.go @@ -222,10 +222,7 @@ func CallerInfo() []string { pcs := make([]uintptr, stackFrameBufferSize) offset := 1 n := runtime.Callers(offset, pcs) - maybeMore := true - if n < stackFrameBufferSize { - maybeMore = false - } + maybeMore := n == stackFrameBufferSize if n == 0 { return []string{} @@ -283,13 +280,12 @@ func CallerInfo() []string { } offset += stackFrameBufferSize n = runtime.Callers(offset, pcs) - if n < stackFrameBufferSize { - maybeMore = false - } - if n == 0 { break } + + maybeMore = n == stackFrameBufferSize + frames = runtime.CallersFrames(pcs[:n]) } From 7f10816c9395d22ff32f384f79d07d8ef6feb698 Mon Sep 17 00:00:00 2001 From: Mike Auclair Date: Tue, 25 Jun 2024 11:50:49 +0000 Subject: [PATCH 5/6] review feedback --- assert/assertions.go | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/assert/assertions.go b/assert/assertions.go index 461182210..9edddc784 100644 --- a/assert/assertions.go +++ b/assert/assertions.go @@ -222,11 +222,12 @@ func CallerInfo() []string { pcs := make([]uintptr, stackFrameBufferSize) offset := 1 n := runtime.Callers(offset, pcs) - maybeMore := n == stackFrameBufferSize if n == 0 { return []string{} } + + maybeMore := n == stackFrameBufferSize frames := runtime.CallersFrames(pcs[:n]) for { @@ -273,21 +274,22 @@ func CallerInfo() []string { break } - if !more { - // We know we already have less than a buffer's worth of frames - if !maybeMore { - break - } - offset += stackFrameBufferSize - n = runtime.Callers(offset, pcs) - if n == 0 { - break - } + if more { + continue + } + // We know we already have less than a buffer's worth of frames + if !maybeMore { + break + } + offset += stackFrameBufferSize + n = runtime.Callers(offset, pcs) + if n == 0 { + break + } - maybeMore = n == stackFrameBufferSize + maybeMore = n == stackFrameBufferSize - frames = runtime.CallersFrames(pcs[:n]) - } + frames = runtime.CallersFrames(pcs[:n]) } From cfee2346d77ee99757847d055944f73c22cd5c4e Mon Sep 17 00:00:00 2001 From: Mike Auclair Date: Tue, 17 Dec 2024 18:18:56 +0000 Subject: [PATCH 6/6] review feedback --- assert/assertions.go | 102 +++++++++++++++++++------------------------ 1 file changed, 46 insertions(+), 56 deletions(-) diff --git a/assert/assertions.go b/assert/assertions.go index 9edddc784..9024239a4 100644 --- a/assert/assertions.go +++ b/assert/assertions.go @@ -221,75 +221,65 @@ func CallerInfo() []string { callers := []string{} pcs := make([]uintptr, stackFrameBufferSize) offset := 1 - n := runtime.Callers(offset, pcs) - - if n == 0 { - return []string{} - } - - maybeMore := n == stackFrameBufferSize - frames := runtime.CallersFrames(pcs[:n]) for { - frame, more := frames.Next() - pc = frame.PC - file = frame.File - line = frame.Line + n := runtime.Callers(offset, pcs) - // This is a huge edge case, but it will panic if this is the case, see #180 - if file == "" { + if n == 0 { break } - f := runtime.FuncForPC(pc) - if f == nil { - break - } - name = f.Name() - - // testing.tRunner is the standard library function that calls - // tests. Subtests are called directly by tRunner, without going through - // the Test/Benchmark/Example function that contains the t.Run calls, so - // with subtests we should break when we hit tRunner, without adding it - // to the list of callers. - if name == "testing.tRunner" { - break - } + frames := runtime.CallersFrames(pcs[:n]) - parts := strings.Split(file, "/") - if len(parts) > 1 { - filename := parts[len(parts)-1] - dir := parts[len(parts)-2] - if (dir != "assert" && dir != "mock" && dir != "require") || filename == "mock_test.go" { - callers = append(callers, fmt.Sprintf("%s:%d", file, line)) + for { + frame, more := frames.Next() + pc = frame.PC + file = frame.File + line = frame.Line + + // This is a huge edge case, but it will panic if this is the case, see #180 + if file == "" { + break } - } - // Drop the package - segments := strings.Split(name, ".") - name = segments[len(segments)-1] - if isTest(name, "Test") || - isTest(name, "Benchmark") || - isTest(name, "Example") { - break - } + f := runtime.FuncForPC(pc) + if f == nil { + break + } + name = f.Name() + + // testing.tRunner is the standard library function that calls + // tests. Subtests are called directly by tRunner, without going through + // the Test/Benchmark/Example function that contains the t.Run calls, so + // with subtests we should break when we hit tRunner, without adding it + // to the list of callers. + if name == "testing.tRunner" { + break + } - if more { - continue + parts := strings.Split(file, "/") + if len(parts) > 1 { + filename := parts[len(parts)-1] + dir := parts[len(parts)-2] + if (dir != "assert" && dir != "mock" && dir != "require") || filename == "mock_test.go" { + callers = append(callers, fmt.Sprintf("%s:%d", file, line)) + } + } + + // Drop the package + segments := strings.Split(name, ".") + name = segments[len(segments)-1] + if isTest(name, "Test") || + isTest(name, "Benchmark") || + isTest(name, "Example") { + break + } + if !more { + break + } } // We know we already have less than a buffer's worth of frames - if !maybeMore { - break - } offset += stackFrameBufferSize - n = runtime.Callers(offset, pcs) - if n == 0 { - break - } - - maybeMore = n == stackFrameBufferSize - - frames = runtime.CallersFrames(pcs[:n]) }