matryer / moq

Interface mocking tool for go generate
http://bit.ly/meetmoq
MIT License
1.96k stars 126 forks source link

Q. `--with-calls` option to generate functions to track and inspect the calls made to a mock #192

Open dnwe opened 1 year ago

dnwe commented 1 year ago

Currently moq will always generate the call tracking and inspection functionality as part of its template. Personally I prefer to have moq just generate the func callers. Is this a usecase that you'd like to support with a PR to add a cmdline flag opt-out/opt-in, or would you prefer not to have the complexity?

Something along the lines of this:

diff --git a/internal/template/template.go b/internal/template/template.go
index 9e2672c..04eb642 100644
--- a/internal/template/template.go
+++ b/internal/template/template.go
@@ -93,6 +93,7 @@ type {{.MockName}}
    // {{.Name}}Func mocks the {{.Name}} method.
    {{.Name}}Func func({{.ArgList}}) {{.ReturnArgTypeList}}
 {{end}}
+{{- if $.WithCalls -}}
    // calls tracks calls to the methods.
    calls struct {
 {{- range .Methods}}
@@ -105,9 +106,12 @@ type {{.MockName}}
        }
 {{- end}}
    }
+{{end}}
+{{- if $.WithCalls -}}
 {{- range .Methods}}
    lock{{.Name}} {{$.Imports | SyncPkgQualifier}}.RWMutex
 {{- end}}
+{{end}}
 }
 {{range .Methods}}
 // {{.Name}} calls {{.Name}}Func.
@@ -123,6 +127,7 @@ func (mock *{{$mock.MockName}}
        panic("{{$mock.MockName}}.{{.Name}}Func: method is nil but {{$mock.InterfaceName}}.{{.Name}} was just called")
    }
 {{- end}}
+{{- if $.WithCalls -}}
    callInfo := struct {
        {{- range .Params}}
        {{.Name | Exported}} {{.TypeString}}
@@ -135,6 +140,7 @@ func (mock *{{$mock.MockName}}
    mock.lock{{.Name}}.Lock()
    mock.calls.{{.Name}} = append(mock.calls.{{.Name}}, callInfo)
    mock.lock{{.Name}}.Unlock()
+{{- end}}
 {{- if .Returns}}
    {{- if $.StubImpl}}
    if mock.{{.Name}}Func == nil {
@@ -156,7 +162,7 @@ func (mock *{{$mock.MockName}}
    mock.{{.Name}}Func({{.ArgCallList}})
 {{- end}}
 }
-
+{{- if $.WithCalls -}}
 // {{.Name}}Calls gets all the calls that were made to {{.Name}}.
 // Check the length with:
 //
@@ -200,6 +206,7 @@ func (mock *{{$mock.MockName}}) ResetCalls() {
    mock.lock{{.Name}}.Unlock()
    {{end -}}
 }
+{{- end}}
 {{end -}}
 {{end -}}
 `
diff --git a/internal/template/template_data.go b/internal/template/template_data.go
index 2a3caeb..0ac6039 100644
--- a/internal/template/template_data.go
+++ b/internal/template/template_data.go
@@ -16,6 +16,7 @@ type Data struct {
    Mocks           []MockData
    StubImpl        bool
    SkipEnsure      bool
+   WithCalls       bool
    WithResets      bool
 }

diff --git a/main.go b/main.go
index 89adb3d..0fffdeb 100644
--- a/main.go
+++ b/main.go
@@ -22,6 +22,7 @@ type userFlags struct {
    formatter  string
    stubImpl   bool
    skipEnsure bool
+   withCalls  bool
    withResets bool
    remove     bool
    args       []string
@@ -38,6 +39,8 @@ func main() {
    flag.BoolVar(&flags.skipEnsure, "skip-ensure", false,
        "suppress mock implementation check, avoid import cycle if mocks generated outside of the tested package")
    flag.BoolVar(&flags.remove, "rm", false, "first remove output file, if it exists")
+   flag.BoolVar(&flags.withCalls, "with-calls", true,
+       "generate functions to track and inspect the calls made to a mock")
    flag.BoolVar(&flags.withResets, "with-resets", false,
        "generate functions to facilitate resetting calls made to a mock")

@@ -89,6 +92,7 @@ func run(flags userFlags) error {
        Formatter:  flags.formatter,
        StubImpl:   flags.stubImpl,
        SkipEnsure: flags.skipEnsure,
+       WithCalls:  flags.withCalls,
        WithResets: flags.withResets,
    })
    if err != nil {
diff --git a/pkg/moq/moq.go b/pkg/moq/moq.go
index e8a2975..4f5191b 100644
--- a/pkg/moq/moq.go
+++ b/pkg/moq/moq.go
@@ -28,6 +28,7 @@ type Config struct {
    Formatter  string
    StubImpl   bool
    SkipEnsure bool
+   WithCalls  bool
    WithResets bool
 }

@@ -82,10 +83,11 @@ func (m *Mocker) Mock(w io.Writer, namePairs ...string) error {
        Mocks:      mocks,
        StubImpl:   m.cfg.StubImpl,
        SkipEnsure: m.cfg.SkipEnsure,
+       WithCalls:  m.cfg.WithCalls,
        WithResets: m.cfg.WithResets,
    }

-   if data.MocksSomeMethod() {
+   if data.MocksSomeMethod() && m.cfg.WithCalls {
        m.registry.AddImport(types.NewPackage("sync", "sync"))
    }
    if m.registry.SrcPkgName() != m.mockPkgName() {
breml commented 1 year ago

I like this proposal. For me it is the same. In most test cases I only use the func callers. Sometimes I check for the number of calls for a particular method (len of calls). I can not remember a case, where I actually inspected the actual calls struct it self.

In regards to backwards compatibility / non breaking changes, I feel the flag would need to be "opt-out", e.g. something like --omit-calls.

endigma commented 1 year ago

I think this could be better as a runtime config option similar to adding hooks? We have some cases where we want/don't want counting for the same mock in different contexts.