-
Notifications
You must be signed in to change notification settings - Fork 38.5k
Add @SpringTest annotation to bootstrap integration with the type's lifecycle and features similar to standard Spring components #22924
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
Comments
Out of curiosity, why make |
Because singleton is the default scope for Spring components in general. Making this the default for tests, too, is just consistent with that. This creates the incentive to create stateless classes, or classes whose methods do not share state, which not only enables multithreaded execution much easier but is also more efficient as it avoids the need to create new instances of the class for each method invocation. There's no burden to manage state if you don't have any. I know that JUnit's default has been exactly the other way round, but I've always considered the reinstantiation a workaround to allow state in the class. Coming from a pure Spring mindset it feels odd that we teach users to write stateless, immutable components but then all of a sudden allow keeping state in the test classes. Especially beginners usually don't realize this difference clearly and then start to use the pattern of manipulating instance variables from within production components which leads to subtle, hardly testable problems. Aligning the testing model with the runtime component model makes problems like these more obvious and discoverable more easily. |
Big +1 from me on that one, this combination of annotations is exactly what I need for Kotlin developers that typically use non static |
Isn't that going a bit too far? The same way that field autowiring in a test class is okay but doing so in a spring component should definitely be avoided. Given that the constructor is going to be invoked for each test, there is no "state" per se. I personally like to setup the common infrastructure of a test in the constructor so that it is immutable. If you have a single component, it is easy enough to just initialize an attribute as the first line of your test but: a) this is something that we have to repeat for every test, b) it's getting more annoying if you have more than one thing to initialise. I guess there are arguments to be made to wrap that in a component or whatever but I think we should be a bit more pragmatic and keep the default of the test framework. |
How do you mean "too far"? Having to inject dependencies into fields was a necessity as previous JUnit generations didn't support constructors. You're also free to use field injection with tests, the annotation setup doesn't change that. It's all opt-in anyway. The annotation enables constructor injection, it doesn't enforce it. Just like
I am not sure I can follow. The per-class setting changes that to instantiate the test class only once. If you have an immutable test fixture that's valid for all tests you can still set that up in the constructor. That can then be augmented by additional setup done in the test methods. If you have common logic there, you can still extract methods which you call from some methods and not from others. It all just creates more incentives to work with immutable test classes, which is a good thing in general and – as indicated before – all of this is optional.
Again, I can't follow. It's an annotation that's optional to be used. When you use it, you opt into its features. If you don't want that, don't use the annotation, use the ones you'd like to apply. I think it's convenient – or: pragmatic – to have a one stop shop annotation that expresses: this test class behaves like a Spring component in terms of lifecycle. If I want the defaults, I get them by default anyway. Especially for beginners this removes a lot of cognitive overload because you don't even have to talk about lifecycles and the differences between them in Spring VS. JUnit. Someone creates an instance, calls all |
The reason @marcphilipp posed that question is that parallel execution is disabled by default in JUnit Jupiter for methods in test classes configured with per-class test instance lifecycle semantics. Excerpt from the JUnit 5 User Guide:
|
@odrotbohm Thanks for the detailed explanation! @sbrannen Thanks for providing additional background to what I wrote! |
@philwebb and @wilkinsona, do either of you have any input from the Boot side? Would Spring Boot be more suitable to host such an opinionated test annotation? |
Regardless of if Spring Boot or Spring Framework would be a better home, there are a few things about the annotation that make me uneasy. Whilst I can see that it is useful in some situations, the subtle differences between Currently @BootstrapWith(SpringBootTestContextBootstrapper.class)
@ExtendWith(SpringExtension.class)
public @interface SpringBootTest {
... This means that you can use it with JUnit 5 directly, or with JUnit 4 (as long as you also have The With regards to So to summarize:
The leaves us with an annotation that's basically just @SpringJUnitConfig
@interface SpringTest {
} |
Thanks for asking, Sam. I don't really have much to add to what Phil's already written above. However, I would like to specifically echo his feelings about |
@philwebb and @wilkinsona, thanks for chiming in and providing feedback. Much appreciated. I tend to share the same sentiments. FWIW, I may find that I personally use such a composed annotation for my own projects, and I can see how it would be beneficial in certain scenarios, especially for Kotlin developers. However, I am not yet convinced that this particular combination of configuration options would be suitable for mainstream Spring users. Generally speaking, the core Spring Framework is rarely (if ever) strongly opinionated in such matters. For example, we introduced In the end, it's easy enough to implement such a custom composed annotation on a per-project basis. So I'm inclined to omit such an annotation for the time being and let the community speak up over time to voice more interest in such a dedicated annotation. |
I'm a bit confused about the aspect of confusion. We teach newbies that the following declaration is the way we recommend writing Spring components: @Component // or more concrete stereotype
class MyComponent {
private final MyDependency dependency;
MyComponent(MyDependency dependency) {
this.dependency = dependency;
}
// business methods go here
} To an overwhelming degree that design suggestion does not have anything to do with Spring Framework, but general ideas of decent object-oriented design: immutability, exposing required dependencies on constructors. Spring is able to work with classes designed that way. In fact, it creates incentives to design the class that way, as following that convention avoids additional configuration (e.g. explicit annotations to document: I want to autowire the constructor). This is also understandable to newbie users. If we now move on and discuss, how you'd write a test class that's also subject to framework handling, I'd argue that anything but the same level of simplicity (a single annotation enabling the above described feature) has to pass the test of asking "Why does it need to be more complicated than that?". Why do I need to write this: @SpringJUnitConfig
@TestConstructor(autowire = true)
@TestInstance(Lifecycle.PER_CLASS)
class MyComponentTest {
} to get the same behavior as above? We can now start to argue different defaults in lifecycle and injecting behavior, legacy expectations created from using previous JUnit generations. The unspoilt mind of a newbie to the platform considers this leaky abstraction, implementation details and – to come back to the start - confusing. This topic has come up every time I teach Spring to university students and it's always been considered a seam in the otherwise very consistent component design story we have. Up until JUnit 4, I was able to argue that the framework itself doesn't allow unifying this into a single annotation. That has changed with JUnit 5. Nobody is asking for some defaults to change, but merely a mechanism just as simple as The annotation would allow to just skip an entire world of discussion as we can resort to "Just works like for
Of course you can have a custom annotation in your project to make this work. That still leaves me to explain why everyone has to bridge that conceptual gap. I'd love to see @jhoeller's opinion on that topic here as in our discussion at Spring I/O it felt he saw some appeal in something like this. |
Unless I'm misunderstanding what the annotation does, isn't it quite possible to write a test in the same way without using With regards to @ExtendWith(SpringExtension.class)
@TestConstructor(Autowire.REMAINING)
class SpringTest {
} You could then do:
and both parameters would be injected correctly. |
It is possible, of course. It just causes the class to be re-instantiated and the code in the constructor being run for each test method, which makes it something like As indicated before, the main intention is to align the way things work with production code. A constructor of a singleton Spring bean is called once, the instance is then used for all method invocations. There's simply no need to recreate the instance if you're working with immutable test classes. Not having to explain any differences is the value here. I like what's suggested in junit-team/junit5#1945. What I am looking for is the general ability to inject dependencies into the constructor. That in a context of test execution other injection candidates can be available is totally fine with me. Personally, I guess I'd prefer |
Indeed, that was just an example. Although the same problem exists there as well if you want to inject it along with a bean. |
@sbrannen Is there a way to switch |
Answering my own question, it seems like
That way all tests get the default we recommend and |
Why would we recommend a non-standard default for all tests? I think changing the lifecycle at all will confuse people and even more so when it's applied to plain unit tests that have nothing to do with Spring. |
I agree with Andy. If at all, a change in the overall defaults should probably a separate discussion. The dedicated annotation would allow us to explicitly declare the changes in default and the rationale behind it as well as allow the user to opt into those decision per test, so that she has control over whether a particular test class is supposed to follow those altered conventions and that tweak is documented right at the test class through the annotation. |
Unfortunately I just don't think we're going to reach a consensus on using
I still don't see any harm in recreating the test instance, and the "not having to explain any differences is the value here" works equally well when comparing a |
It's work done, that doesn't need to be done. A single instance used for all invocations is a great starting point to explain why it makes sense to use immutable classes in the first place. I'm puzzled to hear someone arguing it's just fine to create multiple instances of a class when one would just work fine.
Can you elaborate? My point was that there's way of writing production components we actively recommend. It's hard to explain, why there's no simple (or not an as simple) way to get integration test classes work the same way. "Here's the annotation that works with the same component model." is both consistent and an easy to understand answer, that doesn't spark any new questions. |
I don't disagree that it's extra work to create new test instances, or that a single instance might be technically better. What I want to focus on is the fact that we're deviating from the JUnit default, and I think the bar for that should be higher than if we were starting a new test framework from scratch.
Clearly it is fine to create multiple instances, because that's exactly what happens currently. You can still write immutable classes, you can still explain why it makes sense to do that, you can even change the setting yourself if you're so inclined.
I don't see why the fact that the test instance gets created once, or per-method really changes things here. I think it's perfectly fine for JUnit to remain in charge of the test lifecycle. I think we need a really really strong benefit to change from the default, and so far I've not been convinced that we've found one. Something else that we should consider is if we're going to want to offer support for annotations like |
Team Decision: After having put considerable thought into the proposal, debating the pros and cons and in spite of the benefits such a constellation provides, the core Spring Framework team has decided that we are not convinced that the introduction of such an opinionated test annotation would be the right choice for the core framework. As pointed out in this issue's description, it is already possible for developers to implement their own composed annotation. As such, we recommend that developers do exactly that if such a composed annotation meets their needs. |
As a follow up of #22286, this ticket is supposed to discuss the idea of a canonical annotation to be used with test classes that constitute Spring integration tests and configure JUnit to treat the class like a Spring component as much as possible. In detail, this includes:
This could effectively look something like this (name TBD):
The goal is to minimalize the conceptual differences between a Spring component in production code and a integration test class.
The text was updated successfully, but these errors were encountered: