Often times we want to use interfaces that exist in the stdlib

Since counterfeiter requires you to point it at the source file which has the interface definition, this is pretty much impossible to do programatically, especially in go:generate comments

This would be a huge win.

package race_test

import (
    "github.com/yourcode/yourcodefakes"
    . "github.com/onsi/ginkgo"
)

var _ = Describe("data race", func() {
    It("has a race condition", func() {
        myHandler := new(yourcodefakes.FakeHandler)

        go func() {
            for {
                myHandler.SomeMethod()
            }
        }()

        myHandler.SomeMethodStub = func() string {
            return "some string"
        }
    })
})

Is there a preferred way to change the stub without causing a race condition? We experimented with exposing the mutex for SomeMethod by manually modifying the generated counterfeiter fakes, and locking when reassigning SomeMethodStub, eg:

                myHandler.SomeMethodMutex.Lock()
        myHandler.SomeMethodStub = func() string {
            return "some string"
        }
                myHandler.SomeMethodMutex.Unlock()

This fixes the race condition but obviously this requires editing the fake (non-ideal).

If we hid the re-assignment of the stub into a method, we could Lock/Unlock it in that function instead.

// This file was generated by counterfeiter
package yourcodefakes

import (
    "sync"

    "yourcode"
)

type FakeHandler struct {
    someMethodStub        func() string
    someMethodMutex       sync.RWMutex
    someMethodArgsForCall []struct{}
    someMethodReturns     struct {
        result1 string
    }
    invocations      map[string][][]interface{}
    invocationsMutex sync.RWMutex
}

func (fake *FakeHandler) SomeMethod() string {
    fake.someMethodMutex.Lock()
    fake.someMethodArgsForCall = append(fake.someMethodArgsForCall, struct{}{})
    fake.recordInvocation("SomeMethod", []interface{}{})
    fake.someMethodMutex.Unlock()
    if fake.someMethodStub != nil {
        return fake.someMethodStub()
    } else {
        return fake.someMethodReturns.result1
    }
}

func (fake *FakeHandler) SomeMethodCalls(stub func() string) {
      fake.someMethodMutex.Lock()
      defer fake.someMethodMutex.Unlock()

      fake.someMethodStub = stub
}

...

When you have a vendored library in your project and you use a fully qualified import path to generate a fake, counterfeiter won't search for it in the vendored directory.

E.g.:

mkdir -p vendor/foo/bar/baz/
echo "package baz\ntype Baz interface {\nBaz() string\n}\n" > vendor/foo/bar/baz/baz.go
counterfeiter foo/bar/baz.Baz

Package 'foo/bar/baz' not found on GOPATH

We do not have counterfeiter under our PATH env var. Instead, the location is determined by a GOTOOLSPATH env var. Until recently, the following go:generate directive worked fine:

//go:generate ${GOTOOLSPATH}/bin/counterfeiter ...

However, with v6.2.3, this line no longer works. It seems counterfeiter is a complete no-op and doesn't return any errors. If I add it to PATH explicitly, then the following works as expected.

//go:generate counterfeiter ...

I've been using counterfeiter a lot lately. One thing I notice is that once I generate fakes for two or more packages I'm likely to want to use fakes from multiple packages in a single test. This often means I have to alias my imports...

import (
  "net/http"
  "os"

  dbFakes   "github.com/tjarratt/foo/database/fakes"
  httpFakes "github.com/tjarratt/foo/http/fakes"
)

This kind of sucks because existing tooling doesn't have a great answer to aliased packages (I'm looking at you, vim, emacs, Intelli-J and Sublime), and you constantly have to remember if you can use the fakes package name as is, or if you need to alias it.

What if (hear me out here), we followed the standard library model, and put our fakes into a "__fakes" package? For example, "net/http" has a "net/http/httptest" package that exposes some convenient test helpers. I believe that pivotal-golang/lager does this as well. This pattern seems to be emergent, and convenient since the _fakes package name is much less likely to collide, but still captures the same spirit of the standard library.

For existing code using the fakes package, we could write a "go fix" script that automatically rewrites your code and puts the source files in the (new) correct package. That should be a relatively painless way of dealing with the breakage for current users. (Tip of the hat to @robdimsdale for suggesting this).

I would like to issue a PR to counterfeiter that changes fakes to xyzfakes where xyz is the name of the package being faked.

Thoughts? Concerns? This is a breaking change, so I wanted to put this out here and solicit feedback first.

The counterfeiter CLI allows you to specify -o and --fake-name flags. But it should also allow you to define the package name and path that is generated, for example:

$ counterfeiter github.com/my/package Something -p github.com/my/package/tests/fakes
Wrote `FakeSomething` to `github.com/my/package/tests/fakes/fake_something.go`

An additional complexity here is that I would like to be able to export the fakes to the same package as the interface definition. Reason: currently it is not possible to use counterfeiter-generated fakes in the package defining the interface (e.g. in unit tests) because this results in a cyclic dependency; allowing fakes to reside in the same package fixes this issue.

$ counterfeiter github.com/my/package Something -p github.com/my/package
Wrote `FakeSomething` to `github.com/my/package/fake_something.go`

Fixes #166.

@joefitzgerald According to the usage message from counterfeiter, the -generate flag is supposed to cause it to evaluate all and only the //counterfeiter:generate directives in the current package.

-generate Identify all //counterfeiter:generate directives in .go file in the current working directory and generate fakes for them. You can pass arguments as usual.

  NOTE: This is not the same as //go:generate directives
  (used with the 'go generate' command), but it can be combined with
  go generate by adding the following to a .go file:

  # runs counterfeiter in generate mode
  //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 -generate

Consequently, the existing behavior of trying to also include //go:generate counterfeiter directives in the same bulk request is counter to the documented behavior. Additionally, running counterfeiter without the -generate flag should not trigger any such bulk request logic (the legacy behavior).

As such, I have untangled the legacy logic from the -generate logic, which simplifies things greatly. In short:

1. If the -generate flag was passed, collect all //countefeiter:generate directives, and execute them.
2. Else, only generate what was specifically requested via the command line arguments.

I was trying to figure out if I had the latest version of counterfeiter and was imaging that I could run something along the lines of

counterfeiter -v

Maybe I get a git sha back of the latest commit? A friendly version number would be nice but maybe I am asking too much?

Hi,

I think using the import path would be handier than the filesystem path to locate the interface.

It can be looked up converted to a fs path fairly easy by using the go/build package.

// findRelImport tries to find the import string in the $GOPATH
// and constructs a filepath relative to the current wroking directory
func findRelImport(imp string) string {
    p, err := build.Default.Import(imp, "", build.FindOnly)
    checkPanic(err)

    cwd, err := os.Getwd()
    checkPanic(err)

    p.Dir, err = filepath.Rel(cwd, p.Dir)
    checkPanic(err)

    return p.Dir
}

Having this made moving and renaming packages much more comfortable with other tools but I guess we should have both options for compatibility and flexibility.

I can whip up a preliminary PR if you agree.

This causes an import cycle:

package counterfeit

//go:generate counterfeiter . Fooer

type Fooer interface {
	Foo() bool
}

func Bar(f Fooer) string {
	if f.Foo() {
		return "foo"
	} else {
		return "bar"
	}
}

package counterfeit

import (
	"temp/counterfeit/counterfeitfakes"
	"testing"
)

func TestBar(t *testing.T) {
	fake := &counterfeitfakes.FakeFooer{}

	Bar(fake)
}

This is caused by the interface check at the bottom of fake_fooer.go:

var _ counterfeit.Fooer = new(FakeFooer)

One solution is to change the test file's package to counterfeit_tests, but that doesn't work if I want to test unexported functions.

#8 is similar, but that one is solvable by renaming the import. This isn't.

Is this check really necessary though? If you try to use the fake in a test, and it no longer satisfies the interface, it'll fail to compile anyway (well, except if you're doing something tricky with interface{}). Could you maybe add an option to disable this check?

Given a type aliased function such as

type RequestFactory func(api.Filter, map[string]interface{}) (*http.Request, error)

a valid counterfeit/spy can be generated that looks like

package fakes

import (
    "net/http"
    "sync"

    "github.com/cloudfoundry-incubator/diego-enabler/api"
    "github.com/cloudfoundry-incubator/diego-enabler/commands"
)

type FakeRequestFactory struct {
    Stub        func(api.Filter, map[string]interface{}) (*http.Request, error)
    mutex       sync.RWMutex
    argsForCall []struct {
        arg1 api.Filter
        arg2 map[string]interface{}
    }
    returns     struct {
        result1 *http.Request
        result2 error
    }
}

func (fake *FakeRequestFactory) Spy(arg1 api.Filter, arg2 map[string]interface{}) (*http.Request, error) {
    fake.mutex.Lock()
    fake.argsForCall = append(fake.argsForCall, struct {
        arg1 api.Filter
        arg2 map[string]interface{}
    }{arg1, arg2})
    fake.mutex.Unlock()
    if fake.Stub != nil {
        return fake.Stub(arg1, arg2)
    } else {
        return fake.returns.result1, fake.returns.result2
    }
}

func (fake *FakeRequestFactory) CallCount() int {
    fake.mutex.RLock()
    defer fake.mutex.RUnlock()
    return len(fake.argsForCall)
}

func (fake *FakeRequestFactory) ArgsForCall(i int) (api.Filter, map[string]interface{}) {
    fake.mutex.RLock()
    defer fake.mutex.RUnlock()
    return fake.argsForCall[i].arg1, fake.argsForCall[i].arg2
}

func (fake *FakeRequestFactory) Returns(result1 *http.Request, result2 error) {
    fake.Stub = nil
    fake.returns = struct {
        result1 *http.Request
        result2 error
    }{result1, result2}
}

var _ commands.RequestFactory = new(FakeRequestFactory).Spy

This spy can be used in a very similar way as other spies:

var fakeRequestFactory *fakes.FakeRequestFactory

BeforeEach(func() {
  fakeRequestFactory = new(fakes.FakeRequestFactory)
})

It("does stuff", func() {
  fakeRequestFactory.Returns(new(http.Request), errors.New("BOOM"))

  results := someFuncWhichUsesIt(fakeRequestFactory.Spy)

  Expect(fakeRequestFactory.CallCount()).To(Equal(1))
})

==> Testing counterfeiter
==> /home/linuxbrew/.linuxbrew/Cellar/counterfeiter/6.5.0_1/bin/counterfeiter -p os 2>&1
Writing `Os` to `osshim/os.go`... 
/home/linuxbrew/.linuxbrew/opt/go/libexec/src/os/file.go:246:27: StringData not declared by package unsafe
==> Testing counterfeiter (again)
==> /home/linuxbrew/.linuxbrew/Cellar/counterfeiter/6.5.0_1/bin/counterfeiter -p os 2>&1
Writing `Os` to `osshim/os.go`... 
/home/linuxbrew/.linuxbrew/opt/go/libexec/src/os/file.go:246:27: StringData not declared by package unsafe
Error: counterfeiter: failed

Refs https://github.com/Homebrew/homebrew-core/pull/117643

Hi,

We just found the WhiteSource violation golang.org/x/tools-v0.1.12 in @github.com/maxbrunsfeld/counterfeiter/v6 dependency: https://www.mend.io/vulnerability-database/CVE-2020-11022

Can you please the safe github.com/maxbrunsfeld/counterfeiter/v6 version?

Thank you, Yael

counterfeiter fakes include a line at the bottom which assigns a pointer to the fake to a variable declared as type of interface being mocked. This is a great way to ensure that the counterfeit actually implements the specified interface so that the compiler will generate an error when the interface changes, but it means that you cannot implement a test that depends on the counterfeit implementation within the package itself, because it forces a dependency on that package into packagefakes package.

This means that every time we run go generate, we must also run a script which strips that line out of each generated file. There should be a command line option to skip adding that line to the counterfeit, or which specifies the file to add it to. If I want to detect that the interface is no longer satisfied, I can add such an assignment myself, rather than having the code generator break all test code that adheres to the common pattern of putting tests in the same package as the code being tested (in order to gain access to non-exported fields and methods, which can greatly simplify testing). But it would be even better to have the generator add the line to a specified file so that we can generate those checks in the package being tested rather than in the packagefakes package where it causes an import cycle.

First of all, love your library. I was wondering if you plan on supporting the go 1.18 generics in the near future.

I'm happy to help if you give me some pointers on guidance on how you would approach this

This PR implements the feature discussed in #191. I did my best to add the feature without making invasive changes, but I did have to restructure how flags are parsed. I'm definitely open to feedback on how I can improve the commit or the feature itself. Here are some specific questions I have:

• Is -fake-name-template a good name for the flag?
• Is text/template the right approach here, or would a simple regex, or a prefix + suffix flag be better?
• If text/template works here, is TargetName alright as the field available in the template, or would another name be better. Perhaps just Name?

/cc @joefitzgerald @maxbrunsfeld

Suppose, MyInterface returns different values in different use cases. This can be handled by DoThingsReturnsOnCall(callCount), where we can initialise the fake interface by specifying what the return value should be on each call 0,1,2.. But how can we handle the case, when there is no return value, and the arguments are itself altered as a side effect by the function?

DoThingsCalls(stub) can be used to fake by passing stub to perform that specific side effect. But, what if similar to above, the function has different side effects on different calls? There should be something on similar lines: DoThingsCallsOnCall(callCount, stub) , where we can specify different stubs while initialising, so that different side effects can be performed on each callCount 0,1,2..

If the above is supported via any other functionality available today, please do let know.