Rob Winch opened SPR-12031 and commented

Status Quo

The Spring TestContext Framework (TCF) makes it very easy to run tests that share the same configuration within a test class hierarchy or even across a test suite. However, it does not support annotating a method with @ContextConfiguration.

Proposals

  1. It would be nice if the TCF would allow @ContextConfiguration on a test method. This would allow for easily testing various permutations of configuration.
  2. Support throw-away contexts (i.e., contexts that are loaded for a single test method but not stored in the context cache), perhaps via a separate issue that serves as a building block for this issue.
  3. See #18295.

Issue Links: - #19600 Support Verifying Errors with @ContextConfiguration at method level ("is depended on by") - #13977 Support @ActiveProfiles at method level - #18951 Support @TestPropertySource at method level - #18295 Support 'throw-away' context for a test class

Referenced from: commits https://github.com/spring-projects/spring-framework-issues/commit/73db0e2ca5fc017caa8914e393d36d039a43b480

7 votes, 14 watchers

Comment From: spring-projects-issues

Sam Brannen commented

Dave Syer made a similar suggestion in the comments for #10284.

Since JIRA's permalink functionality seems to be broken, I am including Dave's comments here:

I have a suggestion: what about extending @ContextConfiguration to be an optional method-level annotation? The meaning would be: create a new context if there is none, or use the existing type-level value as a parent otherwise. I would also suggest the default should be to have an implicit @DirtiesContext applying only to the child context (with an option to switch it off I guess), otherwise the context cache could get quite large with no great benefit.

This would also be a great feature for integration testing where you want to override a handful of beans just for the test to provide a stubbed or alternative environment, or where you want to test multiple profiles in one go, without having to reset system properties.

Comment From: spring-projects-issues

Sam Brannen commented

Hi guys,

A few brainstorming questions...

  1. How would you foresee contexts loaded at the test level interacting with those loaded at the class level, in terms of inheritance and overriding of configuration locations and classes, TestExecutionListeners, etc.
  2. How do you imagine method-level contexts interacting with context hierarchies (i.e. via @ContextHierarchy)?
  3. Should method-level contexts be cached at all?
  4. How should a default @Configuration class be detected for an empty method-level declaration of @ContextConfiguration?

Cheers,

Sam

Comment From: spring-projects-issues

Thomas Darimont commented

Continuing the discussions from #12387:

@1: I'd just let them override bean definitions... - w.r.t. TestExecutionListeners: What would be some use cases where users wanted to customize TELs (indirectly via config classes) on test method level?

@2: How about using the method level java config as an overlay / decorator for the top of the context hierarchy stack? This would enable you to replace / override beans but would still behave in an expected way. Perhaps we need another annotation here than @ContextConfiguration if we wouldn't want to allow all that CC allows on method level...

@3. Should method-level contexts be cached at all? I think caching them is possible but could lead to subtible bugs or at least unexpected behaviour. I think it would be okay to not cache those in the first iteration.

@4. Perhaps based on some convention?

class SomeTests{

  @Test
  @ContextConfiguration
  public void test_with_awkward_but_descriptive_name{...}
}

Should lookup something like config Test_with_awkward_but_descriptive_nameConfig{} or a SomeTests_test_with_awkward_but_descriptive_name.xml

That the latter could be quite handy IMHO - the former as well if you could specifiy a the test class to derive in a sane way ;-) - perhaps via an annotation based qualifier - which would also be more robust against refactorings than a test method name based convention.

Comment From: spring-projects-issues

Sam Brannen commented

Hi Thomas,

Thanks for the valuable feedback!

I won't have any time to work on this issue for quite some time (perhaps months since it's too late for 4.2), but that doesn't mean it is forgotten. After all, it's still in my Waiting for Triage list. ;)

Regards,

Sam

Comment From: spring-projects-issues

Sam Brannen commented

In the interim, if you are interested in seeing something like this implemented in spring-test, feel free to vote for this issue.

Comment From: spring-projects-issues

Rob Winch commented

Sam Brannen I would really love to be able to clean up up my tests. Any chance we could see this feature soon?

If not, I don't suppose you have a good idea of how to work around this limitation? I'm fine if the workaround does not support caching.

Comment From: spring-projects-issues

Sam Brannen commented

Hi Rob Winch,

I don't foresee this getting implemented in the very near future, perhaps in Spring Framework 4.3 at the earliest since it requires quite a lot of changes to the internals of the TCF (to do it right and make it a first-class feature).

In the interim, however, it should be possible with custom TestContext and TestContextBootstrapper implementations.

Warning: I have not tried this, but I believe the following should work...

  1. Create a subclass of DefaultTestContext.
  2. The constructor in your subclass will have to track the MergedContextConfiguration in its own instance field since mergedContextConfiguration is a private field in DefaultTestContext.
  3. Introduce new logic for creating an instance of MergedContextConfiguration for the current test method and set the parent of that MCC to the MCC passed into your constructor. The parent part is optional, depending on your needs of course.
  4. Override the getApplicationContext() and markApplicationContextDirty() methods, essentially by copying the existing code from the corresponding overridden methods but using your custom method-level MCC instead of the standard class-level MCC.
  5. Create a subclass of AbstractTestContextBootstrapper (or preferably a subclass of DefaultTestContextBootstrapper or WebTestContextBootstrapper) which overrides the buildTestContext() method and returns an instance of the custom TestContext you created in step # 1 above.
  6. Register your custom TestContextBootstrapper (e.g., via @BootstrapWith).

I suppose the biggest obstacle is that @ContextConfiguration is currently declared with @Target(ElementType.TYPE), meaning that you cannot declare it on a test method. As a consequence, to build your MergedContextConfiguration in step # 1 you will have to find another way to declare and obtain the necessary metadata (e.g., via a custom method-level annotation that mimics @ContextConfiguration, providing likely a subset of its features).

Does that make sense?

Is that enough for you to go on?

If you make any progress based on these tips, I'd be delighted to take a look at it. ;)

Cheers,

Sam

Comment From: spring-projects-issues

Rob Winch commented

Sam Brannen Thanks for the detailed writeup! This does make sense. I will take a look into implementing this soon.

Comment From: spring-projects-issues

Sam Brannen commented

You're welcome Rob Winch, and... may the source be with you! ;)

Comment From: spring-projects-issues

Rob Winch commented

Thanks to your tips I was able to get a good start on this! I have a few questions though.

  • Is there an easy way to disable DependencyInjectionTestExecutionListener.prepareTestInstance from trying to inject dependencies? NoDefaultForContextConfigurationMethodContextJavaConfigTests illustrates this issue along with some additional comments.
  • Is there a good way to signal that the inject dependencies for the test method? Right now I needed to override DefaultMethodTestContext to ensure the context is injected at the method level. The reason for this is that the test method is not available until beforeTestMethod, but it doesn't appear the context will be injected at this point since prepareTestInstance preforms injectDependencies and that removes the REINJECT_DEPENDENCIES_ATTRIBUTE attribute.

Comment From: spring-projects-issues

Sam Brannen commented

Hi Rob Winch,

Glad to hear you're making progress! :)

Thanks to your tips I was able to get a good start on this! I have a few questions though.

Is there an easy way to disable DependencyInjectionTestExecutionListener.prepareTestInstance from trying to inject dependencies?

No, not currently. prepareTestInstance() always injects dependencies.

Is there a good way to signal that the inject dependencies for the test method?

Nothing other than what you already hinted at: REINJECT_DEPENDENCIES_ATTRIBUTE.

That's how both DirtiesContextTestExecutionListener and DirtiesContextBeforeModesTestExecutionListener instruct the DependencyInjectionTestExecutionListener to perform DI before test methods. This feature was added for TestNG, since TestNG does not reinstantiate the test class between methods (in contrast to JUnit).

So one option would be to implement a custom TestExecutionListener that sets the REINJECT_DEPENDENCIES_ATTRIBUTE to true in its beforeTestMethod() method. Then order that TEL just before the DITEL.

Make sense?

Comment From: spring-projects-issues

Rob Winch commented

Sam Brannen Once again thank you for your response.

A follow up question that was in the javadoc, but not listed in the comments. Is there an easy way to easily include all default TestExecutionListeners, but replace DependencyInjectionTestExecutionListener with my custom one?

Thanks again!

Comment From: spring-projects-issues

Sam Brannen commented

Is there an easy way to easily include all default TestExecutionListeners, but replace DependencyInjectionTestExecutionListener with my custom one?

No, unfortunately not.

I guess you could say that's a missing feature.

With Spring Security (which I'm pretty sure you're familiar with ;) ), there's an option for replacing a filter in the filter chain; however, the TestContext framework only provides the ability to insert an additional TestExecutionListener... at least via first-class mechanisms.

There is, however, a hackish way to achieve this goal. I described it somewhere in an issue for Spring Boot (in case you want to search for it). In summary: you could override AbstractTestContextBootstrapper.getDefaultTestExecutionListenerClasses(), delegate to super.getDefaultTestExecutionListenerClasses(), and then manually filter the default listeners to your heart's content.

- Sam

Comment From: spring-projects-issues

Dave Syer commented

Here's some code that works for me: https://github.com/spring-projects/spring-framework-issues/pull/110. It would be great to see support for this pattern in spring-test.

Comment From: spring-projects-issues

Sam Brannen commented

Thanks for sharing your working example, Dave!

Since both you and Rob have made progress in this area, I'll go ahead and move this issue to the 4.3 backlog for more immediate consideration.

Comment From: spring-projects-issues

Juergen Hoeller commented

I'm afraid we have to move this to the backlog at this point since we'll have to do an ultimate feature freeze for 4.3 tomorrow.

Comment From: spring-projects-issues

Rob Winch commented

Sam Brannen This might be implied (but just in case it isn't), can this feature also include a default context to look up (similar to how test classes have)? For example, if the method was:

@ContextConfiguration
public void doThings() {

}

The default contexts it would search for might be somepackage/SomeTest-doThings-context.xml and somepackage/SomeTest$DoThingsConfig.class.

I'm not suggesting this specific naming convention needs to be used. However, I think some sort of convention will save a lot of typing and more importantly allow the tests to have some consistency in how the contexts are associated with the method.

Comment From: spring-projects-issues

Sam Brannen commented

Rob Winch,

See my initial brainstorming question #4 (scroll up):

  • How should a default @Configuration class be detected for an empty method-level declaration of @ContextConfiguration?

So, yeah, default configuration detection is certainly worth considering, and your proposal seems reasonable. ;)

Though... having the default @Configuration class for a method declared in the enclosing class would unfortunately break backwards compatibility since the current algorithm for empty class-level declarations of @ContextConfiguration finds all nested, static @Configuration classes. In other words, Spring would somehow have to know which one to pick when.

Comment From: spring-projects-issues

Rob Winch commented

Thanks for the response. Sorry I missed the comment.

Would it be reasonable to say that if any method is annotated, the defaults for class level are disabled?

Comment From: spring-projects-issues

Sam Brannen commented

Would it be reasonable to say that if any method is annotated, the defaults for class level are disabled?

Yeah, I suppose that would be a feasible way to do it.

Since @ContextConfiguration on a method was never previously supported, this would be an opt-in feature anyway and could therefore safely introduce different semantics.

Comment From: spring-projects-issues

Sergei Ustimenko commented

Sam Brannen Hi Sam, I started to implement described behavior of

@ContextConfiguration

and planning to reuse

@Sql

approach. Just want to know if this ticket still needed.

Comment From: spring-projects-issues

Sam Brannen commented

Yes, this ticket is still needed. I am working on this feature in the 5.0 time frame, but feel free to give it a try yourself if you like.

Comment From: spring-projects-issues

Sergei Ustimenko commented

Sam Brannen Ok, after some investigation the least painful way to add such functionality, as for me, is to make some modifications: * AbstractTestContextBootstrapper. There, as I see it, should be the method like AbstractTestContextBootstrapper#buildTestContext(Method) which will create TestContext with appropriately merged ApplicationContext under the hood. This should be enough to onboard @ActiveProfiles and other Type-level annotations in the future. * Also there should be done some modifications around MetaAnnotationUtils in accordance to add method that will have the same semantics as MetaAnnotationUtils#findAnnotationDescriptorForTypes but will act on method level.

In the very end changes will affect TestContextManager as well. There in the beforeTestMethod most likely would be some additions to handle situation, when we have method annotated with @ContextConfiguration or any other context-affecting annotation.

    public void beforeTestMethod(Object testInstance, Method testMethod) throws Exception {
        String callbackName = "beforeTestMethod";
        prepareForBeforeCallback(callbackName, testInstance, testMethod);
        TestContext ctx = handleMethodLevelContextAffectingAnnotation(getTestContext());

        for (TestExecutionListener testExecutionListener : getTestExecutionListeners()) {
            try {
                testExecutionListener.beforeTestMethod(ctx);
            }
            catch (Throwable ex) {
                handleBeforeException(ex, callbackName, testExecutionListener, testInstance, testMethod);
            }
        }
    }

    private TestContext handleMethodLevelContextAffectingAnnotation(TestContext testContext) {
        if (... annotation on method level ...) {
            TestContext mergedTestContext = copyTestContext(testContext);
            //... do merging here
            return mergedTestContext;
        }

        return testContext;
    }

What do you think about that? Any suggestions? Maybe you will tell me how you would implement such changes.

Comment From: spring-projects-issues

Sergei Ustimenko commented

Sam Brannen You can check out first iteration of working prototype there. It enables a lot of code duplications and I'm planning to refactor it a bit. But still tests for @ContextConfiguration and @ContextHierarchy are passing.

Comment From: spring-projects-issues

Sam Brannen commented

Sergei Ustimenko, I do not currently have time to review your work, but I am glad to hear that you are making progress.

Comment From: spring-projects-issues

Sergei Ustimenko commented

Sam Brannen PR opened here. Please take a look when you will have time. Thanks!

Comment From: spring-projects-issues

Sergei Ustimenko commented

Want to mention, that in fact that I'm not comfortable with so big changes in one commit, I can divide those changes into two commits/PRs. One for @ContextConfiguration and another one for @ContextHierarchy if this will come in handy to ease review process and to keep different logical parts in different commits, though this is parts of one jira ticket.

Comment From: spring-projects-issues

Sam Brannen commented

Sergei Ustimenko, thanks for the PR and the offer to split up the commits!

For the time being, since we are not yet sure if your proposal is the route we want to go, I would suggest that you not put much more effort into it until we have had time to review it in detail.

Please keep in mind that we may choose to implement it differently. So it's best not to polish it too much until a decision has been made on the direction to go.

Cheers!

Sam

Comment From: spring-projects-issues

Sergei Ustimenko commented

Hi Sam Brannen

Thank you for your response! If you don't mind I'll keep this PR open until team's decision and will close it immediately if this is the wrong way to go. Also I'll glad to reimplement this PR if there is a chance. In any case I can put more effort to make it work with the latest approach.

Thanks, Sergei

Comment From: spring-projects-issues

Rob Winch commented

Sam Brannen It would be nice if it was easy to verify the configuration produced some sort of error. Perhaps this is a separate issue though?

Comment From: spring-projects-issues

Sam Brannen commented

Rob Winch,

It would be nice if it was easy to verify the configuration produced some sort of error. Perhaps this is a separate issue though?

Could you please expound on that? And yes... in a separate issue.

Thanks

Comment From: spring-projects-issues

Rob Winch commented

Thanks Sam Brannen I provided SPR-15034 to track verifying configuration errors