search

uber-go / mock

GoMock is a mocking framework for the Go programming language.
Apache License 2.0
2.13k stars 109 forks source link

non-reproducible mocks due to recording of invocation arguments #80

Closed marten-seemann closed 1 year ago

marten-seemann commented 1 year ago

Actual behavior

In quic-go, I'm using go run go.uber.org/mock/mockgen [...] to generate mocks (example). This is a common pattern, and it has the nice property that mock generation becomes independent from the locally install version of mockgen, and instead the version defined in go.mod is used (the project contains a tools.go file behind a build flag that pins the version).

I then have a job on CI that runs go generate ./... and checks that all generated files are consistent. This job has proved invaluable, considering the number of times it has prevented us from accidentally getting the repo into an inconsistent state.

Expected behavior

I expected this job to succeed when using the latest (unreleased) version. However, due to #31, the invocation arguments are now recorded in the mock file. This now contains the path of a temporary directory:

// Code generated by MockGen. DO NOT EDIT.
// Source: github.com/quic-go/quic-go (interfaces: TokenStore)
//
// Generated by this command:
//
//  /var/folders/q0/b5ynf00142l7bl9sp8y098zr0000gn/T/go-build1159986194/b001/exe/mockgen -typed -package quic -self_package github.com/quic-go/quic-go -self_package github.com/quic-go/quic-go -destination mock_token_store_test.go github.com/quic-go/quic-go TokenStore

Obviously, this path differs between platforms and even between repeated invocations on the same machine. Generated mocks are effectively non-deterministic now.

I'm not sure I find #31 very useful to begin with, and in its current state this will cause quite a bit of pain in quic-go. We'd have to pipe the mockgen output through awk / grep to remove the additional lines introduced in #31.

I can see two ways to resolve this problem:

  1. Don't print the full path of the binary, i.e. just print mockgen instead of /var/folders/q0/b5ynf00142l7bl9sp8y098zr0000gn/T/go-build1159986194/b001/exe/mockgen.
  2. Hide this feature behind a config flag. No strong opinions regarding the default value of that flag.

Additional Information

Triage Notes for the Maintainers

liggitt commented 1 year ago

This is still an issue when some of the inputs (like the copyright file) are also in a tmp dir

We're seeing this in https://github.com/kubernetes/kubernetes/pull/120969#discussion_r1347632725 trying to adopt this

Could something like the following allow opting out of recording the invocation args?

writeInvocationComment     = flag.Bool("write_invocation_comment", true, "Writes the invocation used to generate the file as a comment if true.")