wrdv / testme-idea

TestMe IntelliJ IDEA Plugin - Auto generates Unit Test code
http://weirddev.com/testme
Other
157 stars 62 forks source link

Feature/add di mock chek #24

Closed huangliang992 closed 3 months ago

huangliang992 commented 4 months ago

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

yaronyam commented 4 months ago

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). The main issues with this PR - it's not enough to check annotations on a PsiField to deduct that it's under DI. Field can be impacted by:

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). for loggers and alike, we can avoid mocking if:

  1. initialized inline (for loggers these would typically be static fields)
  2. initialized in static block
  3. initialized in ctor - but their value is not passed as ctor arg

use case 1 - is relevant for most implementations - that can be tackled easily with relatively lower impact on existing functionality: either add Field#hasInitializer -> com.intellij.psi.impl.source.PsiFieldImpl#hasInitializer - and use it to avoid generation mock when set. alternatively avoid adding that Field to Type (a potential breaking change)- in case we're sure it won't be useful/relevant in any case for test code generation. one potential pitfall I can think of - setting a default value that is overridden when class initialized (by ctor, DI) - where u'd like to control it's behavior with a mock

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...)

huangliang992 commented 3 months ago

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。 Dingtalk_20240311100241

yaronyam commented 3 months ago

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. Avoid mocking a field if all these conditions are met:

  1. field has no direct DI annotation
  2. field does not have a setter
  3. class has DI annotation
  4. there is no class ctor with DI annotation.
huangliang992 commented 3 months ago

Okay, I'll think about how to handle this next

huangliang992 commented 3 months ago

can condition 4 be replaced with "there is no class ctor"?

yaronyam commented 3 months ago

yes, 4th condition can be widen as suggested

huangliang992 commented 3 months ago

I updated the code and added some itegration tests for the scenes

yaronyam commented 3 months ago

Released in version 6.3