-
Notifications
You must be signed in to change notification settings - Fork 66
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
Feature/add di mock chek #24
Conversation
Hats off for the initiative & effort 🙏 however, I have some concerns for cases where mock will not be generated where it should and regarding impact on performance (extending the traversed object tree). Before I raise technical comments, need to decide we should continue with this effort. It depends on the motivation, which problems are we trying to solve, what effort we're willing to invest and what is the potential negative impact (performance, complexity, backward compatibility).
Other issues - that are less concerning (lower impact or easier to tackle):
We can tackle some of these issue, but not sure the effort and resulted complexity would pay off, if the main use case it to avoid mocking fields such loggers (where those are not initialized by constructor arguments/setters).
use case 1 - is relevant for most implementations - that can be tackled easily with relatively lower impact on existing functionality: use cases 2,3 - are more complex, but used less often, can be supported later when prioritized, by calling psiField.getContainingClass().getInitializers() - search in nested child expressions of each (there's some complexity when private methods called...) |
What I am considering is the general Spring Di scenario. According to the Spring convention, which is greater than the common way of writing configurations. Would it be more accurate that we mock field with annotations such as @autowire in classes with annotations such as @service or @component. And it won't effect other scenarios. Alternatively, can we allow users to choose the objects they need to mock and the testing methods through pop-up boxes. I found that other plugins do it this way, like SquareTest。 |
Would be great to support custom settings on demand when generating test. would also like to add other settings for selection. But anyway, default settings should be best suited for most use cases without going through additional setting form. I think following logic would avoid negative impact.
|
Okay, I'll think about how to handle this next |
can condition 4 be replaced with "there is no class ctor"? |
yes, 4th condition can be widen as suggested |
…into feature/add-di-mock-chek
…eature/add-di-mock-chek
I updated the code and added some itegration tests for the scenes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great to see additional test scenarios being covered. looks great 👍 posted some technical comments
src/main/java/com/weirddev/testme/intellij/template/context/DiClassAnnotationEnum.java
Outdated
Show resolved
Hide resolved
src/main/java/com/weirddev/testme/intellij/template/context/DiFieldAnnotationEnum.java
Outdated
Show resolved
Hide resolved
src/main/java/com/weirddev/testme/intellij/template/context/Field.java
Outdated
Show resolved
Hide resolved
src/main/java/com/weirddev/testme/intellij/template/context/Field.java
Outdated
Show resolved
Hide resolved
src/main/java/com/weirddev/testme/intellij/template/context/MockitoMockBuilder.java
Outdated
Show resolved
Hide resolved
src/main/java/com/weirddev/testme/intellij/template/context/MockitoMockBuilder.java
Show resolved
Hide resolved
Released in version 6.3 |
mock fields according to the dependency injection annotations of class and fields, if a class has annotation like @service, @controller, @component ..., mock the filed with annotation @Autowired or @resource