Mixing Java and Spring annotations.

[slowikps]

Whenever one mix Java and Spring annotations:

• org.springframework.beans.factory.annotation.Autowired
• javax.annotation.Resource

and create a test:

public class MyTest {

    @Tested
    private TestedClass testedClass;

    @Injectable
    @Mocked
    private SomeService annotatedWithAutowired

    @Injectable
    @Mocked
    private SomeService annotatedWithResource
}

class TestedClass {
    @Autowired
    private SomeService1 annotatedWithAutowired;

    @Resource
    private SomeService2 annotatedWithResource;
}

only fields with Java annotations are injected to @Tested object.

What I think need to be fixed/redesign is a discardFieldsNotAnnotatedWithJavaxInjectIfAtLeastOneIsAnnotated method inside mockit.internal.expectations.injection.FieldInjection class

[rliesenfeld]

JMockit does it this way by design, based on the assumption that the code under test would either use standard CDI/Java EE annotations, or proprietary (Spring, Guice) annotations, but not both at the same time.

In this case, is there a reason for the SUT to use both @Resource and @Autowired? Shouldn't it just use @Autowired, or (ideally) the standard javax.inject.Inject annotation rather than @Autowired? Note that Spring does support @Inject.

[slowikps]

Hi @rliesenfeld, Thanks for answer. I ideal world it make perfect sense, but I think that there are many projects which mix especially those two annotations @Resource and @Autowired. Then when one upgrade jmockit version it is really hard to figure out why some of dependencies are not injected and there are so many NullPointerExceptions in tests which were working before. I would like to be able to somehow turn off this functionality, or at least see some hints in my logs.

[rliesenfeld]

Support will be added for @Autowired.

[slowikps]

Hi, Thanks for replaying to my comment, but I think this Enhancement can not be closed as of yet. Two problems: 1) This implementations is very restrictive. It is throwing java.lang.IllegalStateException when one not provide all fields marked as @Injectable, @Autowired... for tested service. I think this should be logged only and exceptions should not be thrown. This can break a lot of legacy tests. 2) If you are adding support to @Autowired annotation support for org.springframework.beans.factory.annotation.Value annotation should be added as well.

Thanks, Paweł

[rliesenfeld]

Hi,

You're right about @Value; I will add support for it too.

About the IllegalStateException, I think this is necessary to avoid unexpected NullPointerExceptions from annotated injection points left uninitialized. Simply printing a warning message to the console will just get ignored by most users. If an existing test breaks, the user can fix it by adding a suitable @Injectable field or parameter. I think this is the right approach because DI containers don't allow required dependencies to remain null either, throwing an exception at startup for any unresolved dependency; also, this prevents a test that passes now to start failing with an NPE months later, when someone changes the code to use the dependency in some new place.

[slowikps]

Hi Rogério, I see your point, but for me it is a bit too much that I need to provide all dependencies. With this requirements unittests are very similar to integration tests, so they loose a bit of unittess beauty.

Furthermore lets consider this example

class A { 
  @Autowired Service1 s1
  @Autowired Service2 s2
  @Autowired Service3 s3
}

Lets assume that A is nicely covered by unit tests. Then we have B:

class B extends A { 
  @Autowired Service4 s4
}

I would like to test B in isolation, but in order to do this I need to provide s1 and s2 and s3.

Does it make any sense for you?

Regards, Paweł

[rliesenfeld]

Most classes with injected dependencies shouldn't have more than three or four of them, so it shouldn't be that difficult to declare the necessary @Injectable's. Classes with more than four dependencies are asking to be broken up into smaller classes, so we can argue that the tests are encouraging a better design.

For the "B extends A" example, I would say that 1) such cases (where a class with its own tests has a subclass) tend to be rare; 2) testing B in isolation from A implies that A would be mocked away, and that doesn't sound good - either B should be tested as if it doesn't extend A, or object composition should have been used rather than inheritance (then it would be perfecly fine to mock A); and 3) a B instance will have all four dependencies when used in production, so why do differently from the tests?

[slowikps]

Ok, I am fine with this.

[pberlowski]

@rliesenfeld Hi Rogerio and thanks for looking into this. I have a few thoughts on the issue. I agree with the conclusion that all dependencies should be mocked into the tested class, however, I have a very important case in which I need to mix @Resource and @Autowired and that is a self-injection.

Spring will not autowire an instance of the object into itself unless that field is annotated with @Resource. We tend to stick to @Autowired (with possible @Qualifier - will you support that?) in most of our services, so that @Resource really stands out as self-injection. There's two major cases where do this: @Cacheable proxy and @Trace proxy.

Now, we've been through all the discussions about the pros and cons of this approach and we stuck to this pattern in certain applications. Our question is really if JMockit will support this.

Also, while I agree that mocking framework for Unit Tests should encourage good practices and discourage bad ones, my opinion is that is not its place to enforce them. By encumbering a framework like this with a role of a policeman, you'd be actually discouraging possible new users from adopting it into their legacy applications. There are other tools to enforce good coding practices, some of them exquisitely good.

[rliesenfeld]

@Resource was already supported, and now (the next release) it can be used in the same class where @Autowired is used; both annotations, plus @Inject, are supported.

I don't know of any use case which requires support for @Qualifier, so there isn't any. It shouldn't matter, though, as qualifiers affect only the implementation that would get injected in production, not the type of the mocked dependency used in tests.

[pberlowski]

Thanks, that confirms that everything we need is going out in the new release.