Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
marten-seemann opened this issue Sep 11, 2023 · 1 comment · Fixed by #82
Closed

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

marten-seemann opened this issue Sep 11, 2023 · 1 comment · Fixed by #82
Assignees

Comments

@marten-seemann
Copy link
Contributor

marten-seemann commented Sep 11, 2023

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

  • gomock mode (reflect or source): both
  • gomock version or git ref: 837f20a
  • golang version: Go 1.21.1

Triage Notes for the Maintainers

@liggitt
Copy link

liggitt commented Oct 5, 2023

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 kubernetes/kubernetes#120969 (comment) 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.")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants