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(scripts): simplify mocks generation #721

Merged
merged 15 commits into from
Jan 7, 2025
Merged

Conversation

qdm12
Copy link
Collaborator

@qdm12 qdm12 commented Dec 27, 2024

Why this should be merged

💁 Sibling PR for subnet-evm so both repositories would look similar in this aspect

  • Localized mockgen commands in the package where they are needed
  • Generate mocks from your IDE directly
  • Platform independent way of generating mocks
  • Check automatically mocks are kept up to date with their matching interfaces
  • Check automatically mocks all have a matching source controlled generation command
  • Remove license headers for generated mocks
  • Generating mocks section added in contributing document
  • Use same mockgen version as specified in go.mod

How this works

  • New mocks_generate_test.go file per package where mocks need to be 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/mockgen 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 and scripts/mocks.mockgen.txt
  • Add step to CI test job to check:
    • mocks are up to date, by re-generating all of them and running a git add --intent-to-add --all and git diff --exit-code
    • mocks each have a source controlled //go:generate generation command, by removing all of them before re-generating them
  • Documentation added to .github/CONTRIBUTING.md
  • Specify tools.go with anonymous import to allow using the same mockgen version automatically as the one specified in go.mod
    • Re-generate mocks with mockgen v0.5 (due to avalanchego upgrade on master branch)

How this was tested

Need to be documented?

Yes in .github/CONTRIBUTING.md

Need to update RELEASES.md?

Not really

@qdm12 qdm12 marked this pull request as ready for review December 27, 2024 11:48
@qdm12 qdm12 requested review from ceyonur, darioush and a team as code owners December 27, 2024 11:48
Copy link
Collaborator

@darioush darioush left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO the license header is not needed, otherwise it's a great simplification.

Copy link
Collaborator

@darioush darioush left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add the CI check to this PR as well and close 720.

precompile/contract/mocks.go Outdated Show resolved Hide resolved
precompile/contract/mocks.go Outdated Show resolved Hide resolved
@qdm12 qdm12 force-pushed the qdm12/mock-gen-script branch from b763d0c to 9f1d7cb Compare December 30, 2024 16:10
@darioush darioush enabled auto-merge (squash) December 30, 2024 16:26
@qdm12 qdm12 force-pushed the qdm12/mock-gen-script branch 2 times, most recently from 127bcbd to d11e682 Compare December 30, 2024 17:08
@darioush darioush disabled auto-merge December 30, 2024 17:26
@qdm12 qdm12 force-pushed the qdm12/mock-gen-script branch 2 times, most recently from 3d7c598 to 084214f Compare December 30, 2024 19:04
darioush
darioush previously approved these changes Dec 30, 2024
@qdm12 qdm12 force-pushed the qdm12/mock-gen-script branch 4 times, most recently from e98de07 to 994bf1c Compare December 31, 2024 12:52
@qdm12 qdm12 force-pushed the qdm12/mock-gen-script branch from 994bf1c to 81d16c3 Compare January 6, 2025 10:01
@qdm12 qdm12 changed the title chore(scripts): simplify mock.gen.sh to use go generate commands chore(scripts): simplify mocks generation Jan 6, 2025
@qdm12 qdm12 force-pushed the qdm12/mock-gen-script branch 4 times, most recently from b8238b6 to 9f6f6cd Compare January 6, 2025 16:44
@qdm12 qdm12 force-pushed the qdm12/mock-gen-script branch from 9f6f6cd to 8fce33b Compare January 7, 2025 10:17
@darioush darioush merged commit b16fc67 into master Jan 7, 2025
8 checks passed
@darioush darioush deleted the qdm12/mock-gen-script branch January 7, 2025 12:30
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.

3 participants