Support constructor injection without @Autowired when using JUnit Jupiter in spring-test

[JaynLau]

Affects: 5.0 GA

---

In the current implementation, org.springframework.test.context.junit.jupiter.ParameterAutowireUtils.isAutowirable(Parameter parameter, int parameterIndex) checks elements annotated with @Autowired, @Qualifier and @Value only.

static boolean isAutowirable(Parameter parameter, int parameterIndex) {
    if (ApplicationContext.class.isAssignableFrom(parameter.getType())) {
        return true;
    }
    AnnotatedElement annotatedParameter = getEffectiveAnnotatedParameter(parameter, parameterIndex);
    return (AnnotatedElementUtils.hasAnnotation(annotatedParameter, Autowired.class) ||
            AnnotatedElementUtils.hasAnnotation(annotatedParameter, Qualifier.class) ||
            AnnotatedElementUtils.hasAnnotation(annotatedParameter, Value.class));
}

When the JUnit Jupiter test class like below, it will not work properly.

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.springframework.test.context.ContextConfiguration;
import lombok.RequiredArgsConstructor;

@ExtendWith(SpringExtension.class)
@ContextConfiguration(classes = ContextBean.class)
@RequiredArgsConstructor
class Junit5WithLombokTest {

    private final DataSource dataSource;
    
    @Test
    void test() {
        System.out.println("test-datasource: " + dataSource);
    }
}

The above code will produce an error below:

org.junit.jupiter.api.extension.ParameterResolutionException: No ParameterResolver registered for parameter [final javax.sql.DataSource dataSource] in constructor [public com.springtest.test.Junit5WithLombokTest(javax.sql.DataSource)]
    at org.junit.jupiter.engine.execution.ExecutableInvoker.resolveParameter(ExecutableInvoker.java:191)
    at org.junit.jupiter.engine.execution.ExecutableInvoker.resolveParameters(ExecutableInvoker.java:174)
    at org.junit.jupiter.engine.execution.ExecutableInvoker.resolveParameters(ExecutableInvoker.java:135)
    at org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:61)
    at org.junit.jupiter.engine.descriptor.ClassTestDescriptor.invokeTestClassConstructor(ClassTestDescriptor.java:345)
    at org.junit.jupiter.engine.descriptor.ClassTestDescriptor.instantiateTestClass(ClassTestDescriptor.java:290)
    at org.junit.jupiter.engine.descriptor.ClassTestDescriptor.instantiateTestClass(ClassTestDescriptor.java:281)
    at org.junit.jupiter.engine.descriptor.ClassTestDescriptor.instantiateAndPostProcessTestInstance(ClassTestDescriptor.java:269)
    at org.junit.jupiter.engine.descriptor.ClassTestDescriptor.lambda$testInstancesProvider$2(ClassTestDescriptor.java:259)
    at org.junit.jupiter.engine.descriptor.ClassTestDescriptor.lambda$testInstancesProvider$3(ClassTestDescriptor.java:263)
    ...

Why not check the parameter name or type whether exists in current ApplicationContext?

For example:

static boolean isAutowirable(ExtensionContext extensionContext,
                             ParameterNameDiscoverer nameDiscoverer,
                             Parameter parameter,
                             int parameterIndex) {
    ApplicationContext ctx = SpringExtension.getApplicationContext(extensionContext);
    
    String name = parameter.getName();
    if (ctx.containsBean(name)) {
        return true;
    }

    Executable executable = parameter.getDeclaringExecutable();

    String[] names = null;
    if (executable instanceof Constructor) {
        names = nameDiscoverer.getParameterNames((Constructor<?>) executable);
    } else if (executable instanceof Method) {
        names = nameDiscoverer.getParameterNames((Method) executable);
    }

    name = (null == names || names.length <= parameterIndex) ? null : names[parameterIndex];
    if (null != name && ctx.containsBean(name)) {
        return true;
    }

    try {
        if (null != ctx.getBean(parameter.getType())) {
            return true;
        }
    } catch (Exception ignore) {}

    return isAutowirable(parameter, parameterIndex);
}

[sbrannen]

Thanks for raising the issue!

It has actually been requested through various channels, so I think we should indeed take some action here.

Why not check the parameter name or type whether exists in current ApplicationContext?

I don't think we would implement this feature like that, since that would result in early (and potentially unnecessary) creation of the ApplicationContext.

I would rather make automatic autowiring of constructors an opt-in feature.

The challenge is coming up with a good way to allow users to easily opt into it.

Another option is to turn on this feature by default and provide a way to opt out of it, but that would potentially be a breaking change for some users -- for example, if a test class constructor previously declared an @Autowired parameter alongside something like TestInfo from JUnit Jupiter.

[sbrannen]

When the JUnit Jupiter test class like below, it will not work properly.

If you're using Lombok you can actually instruct Lombok to annotate the generated constructor as demonstrated in this blog.

Thus, the following should work for your use case.

@SpringJUnitConfig(ContextBean.class)
@RequiredArgsConstructor(onConstructor_ = @Autowired)
class Junit5WithLombokTest {

    private final DataSource dataSource;
    
    @Test
    void test() {
        System.out.println("test-datasource: " + dataSource);
    }
}

Let me know if that works for you.

[JaynLau]

Thanks for your reply.

Yes, @RequiredArgsConstructor(onConstructor_ = @Autowired) works for me. But it is ugly, I am sure most people don't like it, and the lombok project tells us to use it carefully.

Another implementation is to support Constructor always in SpringExtension.

@Override
public boolean supportsParameter(ParameterContext parameterContext, ExtensionContext extensionContext) {
    Parameter parameter = parameterContext.getParameter();
    int index = parameterContext.getIndex();
    Executable executable = parameter.getDeclaringExecutable();
    return ParameterAutowireUtils.isAutowirable(parameter, index) ||
            executable instanceof Constructor;
}

It works for me too. It checks the annotations always and fallback to check a constructor.

I think this implementation is better than that I proposed earlier.

You can also consider a better way to implement this feature.

[sbrannen]

For anyone interested, the current work on this feature can be seen in the following feature branch.

https://github.com/spring-projects/spring-framework/compare/issues/gh-22286

[odrotbohm]

Thanks, Sam. This looks great already! I'm wondering if we could find a better name for the annotation enabling this feature. @AutowireTestConstructor sounds rather instructional ("Do X to the class!") whereas conceptually I'd much rather see the annotation being in line with the stereotype annotations already present in the framework (@Controller, @Service etc.).

I know that the latter imply that the class annotated is actually becoming a Spring bean eventually which the test case is not. But that feels like an implementation detail leaking through. I still think there's beauty in the symmetry of:

• @Controller annotated classes with @RequestMapping (and meta-annotated special versions of that) methods
• @Service annotated classes with @Transactional methods
• @Repository annotated interfaces with @Query methods
• @NameToBeDiscussed annotated classes with @Test methods

On a quick thought @TestComponent, @IntegrationTest or even @ComponentTest come to mind. That latter would even subtley hint at the fact, that it's still something managed by JUnit but allows access to Spring managed components.

If the annotation is supposed to be used in combination with SpringExtension anyway, couldn't we just enforce this by meta-annotating it with @ExtendWith? Also, the test case class in the change doesn't actually declare @ExtendWith(SpringExtension.class)? 🤔

[sbrannen]

Also, the test case class in the change doesn't actually declare @ExtendWith(SpringExtension.class)? 🤔

That's because the test class is annotated with @SpringJUnitConfig which is meta-annotated with @ExtendWith(SpringExtension.class). 😉

[sbrannen]

Thanks, Sam. This looks great already! I'm wondering if we could find a better name for the annotation enabling this feature. @AutowireTestConstructor sounds rather instructional ("Do X to the class!") whereas conceptually I'd much rather see the annotation being in line with the stereotype annotations already present in the framework (@Controller, @Service etc.).

Interesting points regarding the naming. You are correct that it is a "verb" rather than a noun or adjective like most annotations in spring-test. In light of that, I think I might change it to @TestConstructor(autowire = true). That would better align with other annotations such as @TestPropertySource.

What do you think about that?

I'm not very keen on naming this annotation anything related to components, since the use of this annotation doesn't actually imply that the test class is dedicated to testing components per se.

I am also not very keen on naming it anything like @IntegrationTest since that seems more like the testing slice approach in Spring Boot Test, and didn't a previous version of Spring Boot actually have an annotation named exactly that? 😉

In general, this annotation is only being introduced in order to allow developers to override the global default flag for "automatic autowiring of test constructors". As such, I envision that this annotation will typically only be used in one-off scenarios or as a meta-annotation (perhaps in Spring Boot Test) to enforce a certain opinionated style of testing.

In other words...

I imagine that people like you and @sdeleuze will typically set spring.test.constructor.autowire = true in src/test/resources/spring.properties (akin to JUnit Jupiter's support for test instance lifecycle). In that case, you will typically have no use for @AutowireTestConstructor/@TestConstructor unless you want to accept a constructor argument from some other source for a given test class in your test suite (e.g., TestInfo or TestReporter from JUnit Jupiter).

Does that make more sense, now that I've given the rationale for the annotation and the global flag?

[odrotbohm]

tl;dr

I'd love to be able to get a Spring integration test case running with a single annotation that causes the test class to be handled as much as possible like a Spring component in the first place to keep the conceptual differences between test classes and production components as little as possible:

@SpringJUnitConfig
@AutowireTestConstructor
@TestInstance(Lifecycle.PER_CLASS)
// other annotations needed
@interface SpringTest { 
  // add aliases for attributes of above annotations
}

@SpringTest
@TestPropertySource(…)
class MyIntegrationTest {

  // Autowire constructor without explicit annotation
  // Singleton lifecycle by default
}

Details

I understand there are knobs to tweak the defaults and make this work globally. I still think it's suboptimal to leak so much implementation detail into the naming. Ultimately it is about the out of the box simplicity and consistency in the programming model.

Let's take a step back and forget everything we know about how Spring works internally. Now let me introduce Spring's component model to you:

1. You learn that, to make the container aware of a class, the default way is to annotate it. Either with @Component (as that's what triggers the detection) or a stereotype one that allows you to assign a role to a class.
2. An effect of that step is that the class can get access to other components declared through constructor parameters. Also, depending on the stereotype you assign, certain other functionality is enabled on methods of those components: @Controller allows @RequestMapping, @Service is usually where @Transactional goes etc. I know that especially the latter is not a direct connection (i.e. you need to do other things to enable this than just annotate the class / method). But there's conceptual consistency and simplicity in that model.

If I now move on to tests as classes with a special role in the code base, anything that deviates from that model ("Annotate the class to assign it a role, which then allows you to use role specific features on methods") adds cognitive load and feels like a seam between classes with the special test role and production code. There's no reason there should be a conceptual difference, except the stereotype. I might have to deal with specialties and learn about additional annotations, but by default there should be a way to get these tests running as close as possible to the way Spring handles components in terms of features and lifecycle.

The reason I think this is so important is that I think Spring Framework's strength is in the simplicity of the component model as it incentivizes good class design in the first place: thread-safe, immutable types, dependencies handed into classes via constructor parameters, container managed singleton instances by default. With JUnit 5 this is possible now as well. Making the test class work the exact same way would reduce the need to constantly context switch between "How does this work for an application component?" and "How does this work for a test class?". Effectively, they're types with slightly different roles in your application.

If JUnit has other defaults, that's fine. But with the new generation we have the ability to let Spring related integration tests work as close as possible to the Spring component model. In that light, would it make sense to configure @TestInstance(Lifecycle.PER_CLASS) for those tests as well, so that they behave more like a Spring managed singleton, too. You see how this moves away from activating a particular feature (autowiring a constructor) towards mimicking the Spring component model. That said, maybe the latter should rather be implemented in a separate annotation, which could be meta-annotated with the one we're currently discussing. Lots of options.

Also, the test case class in the change doesn't actually declare @ExtendWith(SpringExtension.class)? 🤔

That's because the test class is annotated with @SpringJUnitConfig which is meta-annotated with @ExtendWith(SpringExtension.class). 😉

The reason I was puzzled is this line in the Javadoc:

As of Spring Framework 5.2, this annotation is only supported in conjunction with the {@link org.springframework.test.context.junit.jupiter.SpringExtension SpringExtension} for use with JUnit Jupiter.

That makes me think, I need to use the annotation. I look at the test and don't see it anywhere. Using meta-annotations can sometimes make it a bit hard to follow where stuff comes from and more importantly what do I need to get things running. Would it be an option to annotate the annotation to be introduced with @SpringJUnitConfig right away? Then it wouldn't be necessary to hint at the annotation that usually doesn't appear anywhere (as it's most likely used as meta-annotation).

Alternatively, if @SpringJUnitConfig was annotated with @AutowireTestConstructor, the former is already close to what I was envisioning above, except the test lifecycle setting.

Wondering what @jhoeller thinks about this.

[sbrannen]

@odrotbohm, thanks for the detailed write up!

I now see exactly where you're going with this line of thought, and I would appreciate it if you would open a separate issue to address the introduction of a new composed annotation to meet your goals.

Rationale: this GitHub issue is dedicated to the mechanics of how to support automatic autowiring of test constructors via a global flag and an annotation for overriding the global flag, and we need this as a building block for your proposal in any case. As such, the two topics are orthogonal, and I'll go ahead and complete the tasks for this issue as-is.

Cheers,

Sam

[sbrannen]

Reopening to add corresponding documentation to the Reference Manual.

[sbrannen]

add corresponding documentation to the Reference Manual.

To be addressed in gh-22928.