Skip to content
This repository was archived by the owner on Jun 27, 2023. It is now read-only.

More info when do func has wrong signature #173

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
module github.com/golang/mock

require golang.org/x/tools v0.0.0-20190425150028-36563e24a262
require (
github.com/pkg/errors v0.8.1
golang.org/x/tools v0.0.0-20190425150028-36563e24a262
)
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
github.com/pkg/errors v0.8.1 h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I=
github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
Expand Down
43 changes: 42 additions & 1 deletion gomock/call.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package gomock
import (
"fmt"
"reflect"
"runtime"
"strconv"
"strings"
)
Expand Down Expand Up @@ -149,6 +150,27 @@ func (c *Call) Do(f interface{}) *Call {
vargs[i] = reflect.Zero(ft.In(i))
}
}
defer func() {
if r := recover(); r != nil {
errMsg, ok := r.(string)

// We only handle a very specific panic
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you see a use case where a user might want to panic instead of failing the test?

If not, WDYTA letting fmt.Sprint do some work here (https://play.golang.org/p/SJh50xjMu_8) and allow more than just strings?

// If it's not that one, then we "rethrow" the panic
// This allows users to use functions that panic in their tests
if !ok {
panic(r)
}
if !strings.Contains(errMsg, "reflect: Call using") &&
!strings.Contains(errMsg, "reflect.Set: value of") {
panic(r)
}
skipFrames := 8
stackTraceStr := "\n\n" + currentStackTrace(skipFrames)
funcPC := v.Pointer()
file, line := runtime.FuncForPC(funcPC).FileLine(funcPC)
c.t.Fatalf("%s (incorrect func args at %s:%d?)%+v", errMsg, file, line, stackTraceStr)
}
}()
v.Call(vargs)
return nil
})
Expand Down Expand Up @@ -239,6 +261,25 @@ func (c *Call) SetArg(n int, value interface{}) *Call {
case reflect.Slice:
setSlice(args[n], v)
default:
defer func() {
if r := recover(); r != nil {
errMsg, ok := r.(string)

// We only handle a very specific panic
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question as above.

// If it's not that one, then we "rethrow" the panic
// This allows users to use functions that panic in their tests
if !ok {
panic(r)
}
if !strings.Contains(errMsg, "reflect: Call using") &&
!strings.Contains(errMsg, "reflect.Set: value of") {
panic(r)
}
skipFrames := 8
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add some docs where 8 came from?

stackTraceStr := "\n\n" + currentStackTrace(skipFrames)
c.t.Fatalf("%s%+v", errMsg, stackTraceStr)
}
}()
reflect.ValueOf(args[n]).Elem().Set(v)
}
return nil
Expand Down Expand Up @@ -382,7 +423,7 @@ func (c *Call) matches(args []interface{}) error {

// Check that the call is not exhausted.
if c.exhausted() {
return fmt.Errorf("Expected call at %s has already been called the max number of times.", c.origin)
return fmt.Errorf("Expected call at %s has already been called the max number of times (%d).", c.origin, c.maxCalls)
}

return nil
Expand Down
43 changes: 43 additions & 0 deletions gomock/stack.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package gomock

import (
"bytes"
"fmt"
"strings"

"github.com/pkg/errors"
)

const skipFrames = 2
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add some docs around this magic number?


type stackTracer interface {
StackTrace() errors.StackTrace
}

func stackTraceStringFromError(err error, skipFrames int) string {
if err, ok := err.(stackTracer); ok {
return stackTraceString(err.StackTrace(), skipFrames)
}
return ""
}

func currentStackTrace(skipFrames int) string {
err := errors.New("fake error just to get stack")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use the runtime package instead and avoid the extra dependency?

if err, ok := err.(stackTracer); ok {
return stackTraceString(err.StackTrace(), skipFrames)
}
return ""
}

func stackTraceString(stackTrace errors.StackTrace, skipFrames int) string {
buffer := bytes.NewBufferString("")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: It doesn't look like we need to use the function bytes.NewBufferString("") as we're taking in an empty string. Maybe just bytes.Buffer{}?

for i := skipFrames + 1; i < len(stackTrace); i++ {
frame := stackTrace[i]
buffer.WriteString(fmt.Sprintf("%+v\n", frame))
filename := fmt.Sprintf("%s", frame)
if strings.Contains(filename, "_test.go") {
break
}
}
return buffer.String()
}