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

chore(all): mocks generation improved #3628

Merged
merged 15 commits into from
Jan 6, 2025

Conversation

qdm12
Copy link
Contributor

@qdm12 qdm12 commented Dec 31, 2024

Happy new year 🎉 🎆 🆕

Why this should be merged

💁 This PR is created because of similar PRs created on coreth (ava-labs/coreth#721) and subnet-evm (ava-labs/subnet-evm#1413)

ℹ️ Each commit can be separated in a different PR, and the few last commits can be dropped if necessary.

  • Local mockgen commands in the package where they are needed
  • Generate mocks from your IDE directly
  • Platform independent generation of mocks using go only
  • Simplify CI check for up-to-date mocks and to detect mocks without a source controlled generation command

How this works

  • New mocks_generate_test.go file per package where interface(s) need a corresponding mock(s) generated, containing only //go:generate commands for mock generation. Each command is relative to the current package directory.
  • Use //go:generate go run go.uber.org/mock/[email protected] to avoid requiring to pre-install mockgen
  • no shell script, just go command needed with go generate -run "mockgen" ./... - remove now unneeded scripts/mocks.gen.sh, scripts/mocks.mockgen.txt and scripts/mocks.mockgen.source.txt
  • Simplify CI mocks check, it only uses grep, xargs and git. This is now for CI only since the developer should mostly care about re-generating mocks and not detecting outdated mocks. The developer can still use the CI job inlined commands locally if needed.
  • Documentation mocks section updated in CONTRIBUTING.md and referenced in README.md
  • Remove unused generated mocks (if it's used in another repository, they should generate the mocks in their own repository ideally)
  • Rename local test only mock files (1) to mocks_test.go to reduce Go production code namespace pollution
  • mockgen upgraded from v0.4 to v0.5 in order to support generic code and drop the usage of the source mode mock generation (6 generate commands) in favor of the reflect mode
  • mock files in the same directory are now combined into a single mock file "mocks.go" to reduce the number of mockgen calls to make full re-generation a bit faster removed to reduce 4000+ lines of diffs, will do later eventually

How this was tested

Need to documented in RELEASES.md?

Not really

@qdm12 qdm12 force-pushed the qdm12/mocks-generation-simplified branch 2 times, most recently from f9864be to a4b393a Compare December 31, 2024 15:43
@qdm12 qdm12 force-pushed the qdm12/mocks-generation-simplified branch from a4b393a to 79c6a89 Compare December 31, 2024 15:58
@qdm12 qdm12 force-pushed the qdm12/mocks-generation-simplified branch from 79c6a89 to 44eaab4 Compare December 31, 2024 16:10
@qdm12 qdm12 marked this pull request as ready for review December 31, 2024 17:04
@qdm12 qdm12 force-pushed the qdm12/mocks-generation-simplified branch from 44eaab4 to 89a41e4 Compare January 2, 2025 16:30
@qdm12 qdm12 force-pushed the qdm12/mocks-generation-simplified branch from 89a41e4 to a35197a Compare January 2, 2025 18:45
@StephenButtolph
Copy link
Contributor

Once linting passes this lgtm

qdm12 added 13 commits January 3, 2025 19:10
- Rely only on `grep`, `xargs` and `git`
- Does not rely on `mapfile` which is *sigh* still not there on macos default bash 3.2
- Use `git diff --exit-code` to print diffs
- Simplify script
- No need to check twice for outdated mocks
- Only the CI should really check for outdated mocks, the developer should only care about re-generating mocks
- One less script / more cross platform mocks generation
- requires a `tools.go` blank importing golang.org/x/mod/semver in order to have the `golang.org/x/mod` dependency satisfied for mockgen v0.5
@qdm12 qdm12 force-pushed the qdm12/mocks-generation-simplified branch from d347ff5 to ccfdecd Compare January 3, 2025 18:10
@StephenButtolph StephenButtolph merged commit 8fd2d7b into master Jan 6, 2025
22 checks passed
@StephenButtolph StephenButtolph deleted the qdm12/mocks-generation-simplified branch January 6, 2025 09:18
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 this pull request may close these issues.

4 participants