counterfeiter does not work via go generate if outside PATH

[rittneje]

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 ...

[joefitzgerald]

The expansion of environments happens via go's generate functionality and unfortunately counterfeiter plays no role in resolution of itself.

I would recommend using the approach described in the README to invoke counterfeiter, described in detail here: https://github.com/maxbrunsfeld/counterfeiter#using-counterfeiter

This will result in addition of a tools.go file and your generate directives changing to look like this:

//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 <the arguments you use here>

Using this approach, you won't need counterfeiter in your path or a tools path, and instead can use go.mod to control the version of counterfeiter you and the other members of your team use.

[rittneje]

@joefitzgerald This is definitely a bug with counterfeiter itself. I can tell from the debug output of go generate that it is properly executing the command with expansion. But then counterfeiter does nothing.

Based upon the recent release notes, my suspicion is that the fix for #137 (or something similar to it) is responsible. Why is counterfeiter even parsing go:generate directives? That makes no sense.

[joefitzgerald]

@rittneje have you tried to use the approach described in the README? This will solve your issues.

counterfeiter parses go:generate directives as an optimization to speed up invocation when there are multiple generate directives in a package.

I will write a test that replicates your issue and look into a fix, as you are correct that the generate parsing logic should be able to handle this scenario.

[rittneje]

We have installed counterfeiter as a pre-built binary inside a docker container. I don't want to have to save the source code in the image or download it on the fly, so using go run as suggested will not work.

[joefitzgerald]

Are you using a -generate flag in your invocation of counterfeiter? Could you post a full generate directive for my test case?

[joefitzgerald]

Also, could you please post the error output (if any) in the case of the failed invocation?

[rittneje]

No, we are not using that flag. We have the following forms:

// local interface
//go:generate $GOTOOLSPATH/bin/counterfeiter . MyInterface

// vendored interface
//go:generate $GOTOOLSPATH/bin/counterfeiter mypackage/vendor/github.com/aws/aws-sdk-go/service/dynamodb/dynamodbiface.DynamoDBAPI

// standard library interface
//go:generate $GOTOOLSPATH/bin/counterfeiter os.FileInfo

On occasion, we also use the -o flag and/or -fake-name flag.

There is no output at all from the failed invocation. go generate also exits with a successful status code (i.e., 0).

[joefitzgerald]

If you are not using the -generate flag, counterfeiter is not doing parsing of the generate directives. Have you confirmed that the following results in different output?

//go:generate $GOTOOLSPATH/bin/nonexistent

[michaelbeaumont]

I'm running into this issue as well. If I have:

//go:generate counterfeiter -o fakes/fargate_client.go . FargateManager

everything works, if I have:

//go:generate "${GOBIN}/counterfeiter" -o fakes/fargate_client.go . FargateManager

it exits successfully but generates nothing.

EDIT: And yes, with a non existent file I get:

pkg/eks/fargate.go:18: running "/home/mike/go/bin/counterfeiters": fork/exec /home/mike/go/bin/counterfeiters: no such file or directory

[rittneje]

@joefitzgerald From tracing through the code, the bug was indeed caused by the changes in #137.

First, main calls command.Detect to get a list of "invocations".

command.Detect will return invocations(cwd, generateMode), because invokedByGoGenerate() is true.

invocations will loop through all the files, calling open(cwd, gofiles[i], generateMode) on each and appending the resulting invocation lists.

open will call matchForString for each line in the file.

Because of the regex changes in #137, matchForString now requires your go:generate directive to be one of the following:

1. go run github.com/maxbrunsfeld/counterfeiter/v6
2. gobin -m -run github.com/maxbrunsfeld/counterfeiter/v6
3. counterfeiter
4. counterfeiter.exe
   Since ${GOTOOLSPATH}/bin/counterfeiter doesn't match any of these, matchForString will return nil.

Consequently, open will return an empty slice, as will invocations, and command.Detect. Thus the for loop up in main will do nothing at all.

---

Now, in terms of fixing this bug, I think counterfeiter should be amended not to have any of this fancy optimization logic under go generate. This is because you are actually violating how go generate is supposed to work. For example, I can pass a specific file path to go generate, and only directives in that file will be executed. Or I can pass a regex via -run, and only matching directives will be executed. But it seems in both cases counterfeiter will go behind my back and attempt to regenerate the entire package.

If someone does want to generate all the package fakes at once, they should exclusively use counterfeiter:generate directives, and then invoke counterfeiter itself with some flag that indicates they want that behavior (instead of the classic "one interface at a time" behavior). In short:

//go:generate counterfeiter . MyInterface will exclusively generate a fake for MyInterface, no matter what directives exist in the rest of the package.
//go:generate counterfeiter -package (or some similar flag) will generate fakes for all counterfeiter:generate directives in the package, ignoring all go:generate directives (even if they are go:generate counterfeiter directives).

I would like to point out that if someone is using the "classic" go:generate directives, I don't think your optimization accomplishes anything, because if there are N directives in the package, counterfeiter is going to be invoked N times by go generate. In fact, it probably makes it worse, since you will actually generate all N fakes N times, for a total of N^2 generations.