gojuno / minimock

Powerful mock generation tool for Go programming language
MIT License
577 stars 38 forks source link

Use "go run" in the generated go:generate #47

Open deadok22 opened 4 years ago

deadok22 commented 4 years ago

Running minimock without -g flag produces a go:generate line that looks like this:

//go:generate minimock -i InterfaceName ...

minimock can resolve to any command on the system go:generate is run on (most likely, older versions of minimock). Let's change the generated command to go run github.com/gojuno/minimock/v3/cmd/minimock so that the version from the module's dependencies is used.

That way it'll play nicely with the recommended tool dependency tracking approach

hexdigest commented 4 years ago

Hi @deadok22

As usual thanks for the issue! I was thinking about this approach but the problem with go run is that it tries to compile minimock every time. Even though there's now module cache I don't think it's very effective way of calling the tool. In some projects I have >20 mocks. What also bothers me is the fact that I think it's actually a breaking change for some users and if we're going to make this behavior default then it should be a new major version which I want to avoid.

I also faced this problem but with tool dependency tracking approach that you suggest you can actually install all required tools in a ./bin/ dir relative to the root of your project and redefine PATH in a Makefile (or whatever build tool you use) or just simply call concrete minimock version directly, i.e. ./bin/minimock from a Makefile or by placing //go:generate instructions into the special file like:

$ cat generate.go

//go:generate ./bin/minimock -g -i io.Writer -o ./buffer

This is the approach I actually ended up with. I have make tools job in my Makefiles which installs all tools with respect to module dependencies (and other tools like protoc etc) to get fully reproducible code generation. And make generate job which actually calls go generate with PATH env var modified:

export GOBIN := $(PWD)/bin
export PATH := $(GOBIN):$(PATH)

There are also tools like gvm that allow you to manage your env vars depending on the "packagesets" which can also help.

I'll think on making this optional via some new flag but my general thought on that problem is that while we obviously can make this change and use go run it's a developer responsibility to set up the environment correctly because at the end of the day we use many tools not only minimock and have same issues with versions.

WDYT?

deadok22 commented 4 years ago

@hexdigest thanks for sharing your thought process. It all makes sense as ultimately are after reproducible builds / code generation.

The approach with makefiles would definitely work, provided you and your team agree on using makefiles, maintain those makefiles properly, and make sure the build tools are runnable on all operating systems. That can't be achieved in all organizations and it doesn't scale well if you have hundreds of projects as that's yet another way of versioning things.

The concerns about recompilation are legit, but that may be acceptable for some, if not most users. Going with an option for generating go run invocations seems reasonable to me.

chiluk commented 7 months ago

Would it be possible to instead check the line and leave whatever invocation mechanism was used as the go:generate line? That way each preference would be satisfied.

Forcing one mechanism over the other doesn't make sense, when minimock shouldn't care how it's called. Instead it should attempt to preserve how it was called as much as possible.

hexdigest commented 6 months ago

Hey guys,

Another possible solution to this problem could be introducing a flag to check the version, i.e.:

//go:generate minimock -i InterfaceName -check-version=3.3.1 ...

This way we can make sure that the older version of this command won't be used. I can also make sure that in case if current version of minimock produces code that is backward incompatible with the one that was used to generate file initially it will return an error message instead of rewriting the file.

WDYT?