Skip to content

Conversation

@reject-i
Copy link

@reject-i reject-i commented May 2, 2025

What’s inside

  • .mockery.yaml – central config (mock-build-tags: "test", inpackage, with-expecter).
  • Replaced every //go:generate moq … with //go:generate mockery --name=….
  • Deleted legacy *_generated_mock.go / *_moq_test.go; added new mock_*.go (guarded by //go:build test).
  • Makefile targets mocks / generate_mocks
    • regenerate SSZ first
    • iterate over packages that contain //go:generate mockery and run go generate.
  • CI / local build: mocks stay out of production binary, are reproducible via make mocks.

Closes #852

@reject-i reject-i changed the title Switch mock generation from **moq** to **mockery** (build-tag test) markdown Копіювати Редагувати Switch mock generation from **moq** to **mockery** (build-tag test) markdown May 2, 2025
@ukorvl ukorvl requested a review from Un1oR May 2, 2025 19:06
@reject-i reject-i force-pushed the mockery-migration branch from d65e913 to 180c964 Compare May 5, 2025 14:04
@reject-i
Copy link
Author

reject-i commented May 5, 2025

  • switched mock generation from matryer/moq to mockery
  • added .mockery.yaml with build tag test, inpackage, with-expecter, package list & include-regex
  • removed //go:generate directives – generation is now driven solely by make mocks
  • ignored nil/**/mock_*.go and dropped committed auto-generated mocks
  • simplified mocks targets in Makefile & nil/Makefile.inc

@Un1oR @olegrok ready for review , thanks

.PHONY: $(root_relayer)/gen_l2_mocks
$(root_relayer)/gen_l2_mocks: $(root_relayer)/embed_l2_abi
cd $(root_relayer)/internal/l2 && go run github.com/matryer/moq -out l2_contract_generated_mock.go -rm -stub -with-resets . L2Contract
cd $(root_relayer)/internal/l2 && mockery --name=L2Contract
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you try to build a project?

.gitignore Outdated
.qodo
nil/**/mock_*.go

# mockery output (исключаем helper-тесты)
Copy link
Contributor

Choose a reason for hiding this comment

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

Usually we write comments in English even we use AI tools

.gitignore Outdated
.qodo
nil/**/mock_*.go

# mockery output (исключаем helper-тесты)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please write all comments in English

nil/internal/vm: {}
nil/services/rollup: {}

include-regex: '^(Client|DB|RwTx|StateDBReadOnly|L1BlockFetcher)$'
Copy link
Contributor

Choose a reason for hiding this comment

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

There are still plenty of types used for mock generation which are not covered here
For instance, check this Makefile.inc

Copy link
Contributor

Choose a reason for hiding this comment

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

As far as I see, mockery has a completely different API for assertions in comparison to moq
So, all existing tests should also be adjusted

@zadykian
Copy link
Contributor

zadykian commented May 5, 2025

Do we really need to migrate to mockery?
What benefits are we getting by adding a new build dependency and updating all existing tests?

Regarding the //go:build test annotation, it can be easily handled by a script

Script invocation looks like that:

//go:generate bash ../scripts/generate_mock.sh {InterfaceName}

@Un1oR
Copy link
Contributor

Un1oR commented May 6, 2025

@zadykian I'll answer as the author of the original proposal)

As far as I see, mockery is more actively maintained and developed than moq. Maybe it's not so important for a "just working" library, but still.
OK, we could add workaround for build tags, but why when it's already there out of the box?
I like the idea of being able to remove unnecessary stuff from Makefiles. The boilerplate for various code generation in them makes me sad.
Also mockery positions itself not only as a mock generator, but also as a framework for generating general purpose code using customizable templates. This could be interesting for replacing reflection with codegeneration in RawAPI, for example.
Well, and adding one dependency instead of another doesn't look like a big problem.
And test adaptation is part of the task. If it is accomplished, we don't have to worry about additional effort.

@reject-i
Copy link
Author

reject-i commented May 6, 2025

Should I apply all your suggestions?

@Un1oR
Copy link
Contributor

Un1oR commented May 30, 2025

@reject-i do you plan to correct the comments?

@reject-i
Copy link
Author

@Un1oR ready, thanks

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