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

Constructor parameter resolution #106

Closed

Conversation

kkurczewski
Copy link

Hello

I implemented idea in #104.

What I did:

  • added parameter target to annotation so mock annotations can be used on constructor parameters
  • I changed parser to accept both fields and constructor parameters
  • Spring dependency descriptor still requires field in order to work hence I try to map constructor param to field by type (constructor param names are cleared at the bytecode) - in other words, we read annotations from ctor param but under hood we use backing-field for actual binding, it will work as long there is 1:1 mapping, otherwise we ignore it (just as it is now)
  • added tests
  • fixed Intellij warnings

I already tested snapshot version locally, here's is a repository with example code:
https://github.com/kkurczewski/spring-mockk-constructor-injection

In this repository I also implemented SpringMockkExtension for JUnit5, it just barely one class but without it parameter resolver will throw errors.

This extension depends both on SpringMockk and Junit5 code, as original springmockk artifact doesn't bundle Junit5 extension this probably should be released as a separate artifact but maybe this is an overkill, not sure.

Currently `@MockkBean` and `@SpyBean` annotations are only supported via fields with `lateinit var` keyword. This change implements support for binding parameters via constructor Junit5 extension.
@olaisolheim
Copy link

Waiting for this. 👀

@jnizet
Copy link
Member

jnizet commented Jan 19, 2024

See my comment on #104. I see that you introduced various small refinements on the code to make it cleaner though. If you're willing to open another PR with just those changes, I would happily merge it.

@jnizet jnizet closed this Jan 19, 2024
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