Skip to content
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

spring-test allows you to reference configuration classes that aren't annotated with @Configuration [SPR-9051] #13690

Closed
spring-projects-issues opened this issue Jan 24, 2012 · 17 comments
Assignees
Labels

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Jan 24, 2012

Keith Donald opened SPR-9051 and commented

When a @ContextConfiguration test class references a config class missing a @Configuration annotation, @Bean dependencies are wired successfully but the bean lifecycle is not applied (no init methods are invoked, for example). Adding the missing @Configuration annotation solves the problem, however the problem and solution isn't obvious since wiring/injection appeared to work.

It would be better if an integration test failed-fast if you reference a config class with no @Configuration annotation.


Affects: 3.1 GA

Issue Links:

  • #14114 Missing @Configuration annotation will cause transaction not working ("is duplicated by")
  • #14061 Document @Bean 'lite' mode and annotated classes in the reference manual
  • #15002 Log warning when using inner-bean referenced without @Configuration
  • #15599 Throw exception when @ContextConfiguration#classes are not annotated with @Configuration
  • #14037 Improve documentation for @Bean 'lite' mode and annotated class support in the TestContext Framework ("is superseded by")

Referenced from: commits 500a4dd, 2017b24, 1cec0f9, 78c6d70

0 votes, 5 watchers

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Feb 8, 2012

Aleksandr Dubinsky commented

Also nested static @Configuration classes aren't loaded.

Noticed this myself today. This doesn't have consistent behavior one way or another. Sure, it lets you quickly swap in a stub class. But why have this inflexible one-trick pony? Just make it an error rather than fix the inconsistencies.

Loading

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Apr 16, 2012

Keith Donald commented

This is a pretty important issue to address from a user experience perspective. If you forget @Configuration on your test config class, things appear to "half work" and it's very difficult to debug what the hell is going on. As one example, if you forget @Configuration on a @Transactional test, transaction rollbacks will just not work even though the logging indicates otherwise. It's not obvious at all that the cause of a transaction not being rolled back is related to forgetting a @Configuration annotation on a config class.

Please enforce that @Configuration is present if it's going to lead to all kinds of silent undesirable behavior.

Loading

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Apr 16, 2012

Joris Kuipers commented

My code is actually relying on this behavior: I'm using inner config classes in my test code not annotated with @Configuration, to prevent the inner class from being picked up by component scanning. A workaround is to exclude inner classes during component scanning using a reg-exp based exclusion, but I'm not all that unhappy with the current behavior (although it's not properly documented): Spring supports @Bean on classes not annotated with @Configuration as a quick factory method, and this seems consistent with that feature to me.

Loading

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Apr 16, 2012

Keith Donald commented

I guess you haven't run into problems yet because the bean lifecycle is not invoked, consider yourself lucky.

Loading

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented May 11, 2012

Sam Brannen commented

Hi Aleksandr,

Also nested static @Configuration classes aren't loaded.

Nested static @Configuration classes are detected automatically as the default configuration class if you do not specify explicit locations or classes via @ContextConfiguration.

So if this is not the behavior you are experiencing, could you please put together a minimal working example project (or classes) that reproduces the unexpected behavior?

Thanks,

Sam

Loading

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented May 11, 2012

Sam Brannen commented

Hi guys,

Based on tests I just checked in, I cannot reproduce the claims raised in this issue.

Specifically, AnnotatedConfigClassesWithoutAtConfigurationTests demonstrates that @Bean methods in non-@Configuration classes are properly handled as annotated factory bean methods and that lifecycle callbacks in fact apply to such factory beans.

This is in line with Joris' comments. Specifically, classes that are declared via @ContextConfiguration(classes=...) are only required to be annotated classes. Annotated classes include annotated components (e.g., @Service, et al), @Configuration classes, and any class that contains @Bean annotated methods. The latter are effectively annotated factory bean methods.

Thus I concur with Joris that the JavaDoc and Reference Manual for this support is misleading: the documentation typically refers to configuration classes when it should in fact refer to (and explain) annotated classes. This will be addressed in a #14037.

@Keith: if you have a use case where the lifecycle callbacks are not honored, please raise a separate JIRA issue with an example that reproduces that.

Thanks,

Sam

Loading

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented May 11, 2012

Sam Brannen commented

Resolving this issue since I could not reproduce the claimed improper behavior.

Loading

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented May 11, 2012

Keith Donald commented

I provided a use case -- @Transactional test methods. I guess you're putting the onus on me to show you that not working concretely. There is a bug.

Loading

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented May 11, 2012

Sam Brannen commented

Hi Keith,

I'm not trying to put the onus on you per se; I'm just trying to ensure that the code works as expected. I believe that the tests I checked in demonstrate the expected behavior, but that of course may not cover enough use cases. That's why I'd like to see a failing use case so that I can assess if it's just a configuration error (perhaps due to a misunderstanding and/or misleading documentation) or rather a true bug in the framework.

This is a pretty important issue to address from a user experience perspective. If you forget @Configuration on your test config class, things appear to "half work" and it's very difficult to debug what the hell is going on.

Please enforce that @Configuration is present if it's going to lead to all kinds of silent undesirable behavior.

We certainly want to do the best we can to support the user experience, but enforcing that @Configuration is present is not an option since it is perfectly viable to declare annotation classes that are not @Configuration classes. So that is not an error. Perhaps we could warn the user (via a log statement), but I fear that the warning would be annoying to those who intentionally declared other types of annotated classes.

As one example, if you forget @Configuration on a @Transactional test, transaction rollbacks will just not work even though the logging indicates otherwise. It's not obvious at all that the cause of a transaction not being rolled back is related to forgetting a @Configuration annotation on a config class.

I'd really like to see at least a small code and configuration snippet to understand what your set up is there.

Thanks,

Sam

Loading

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented May 11, 2012

Keith Donald commented

It's a simple test to reproduce the problem I experienced. I assumed it might be a specific instance of a more general problem that had something to do with Spring bean lifecycle not being managed properly in a context text environment, but I may have been mistaken. In any case, there is a definite, easily reproducible specific problem (despite the fact I don't know what the cause is--I have not had a chance to investigate the code).

To reproduce:

Take a Transactional test that mutates the database:

@ContextConfiguration(classes=Spr9051TestsConfig.class)
@Transactional
@RunWith(value=SpringJUnit4ClassRunner.class)
public class Spr9051Tests {

    @Inject
    private JdbcTemplate jdbcTemplate;
    
    @Test
    public void testInsert() throws Exception {
       // do a insert 
    }

}

"testInsert" should do something like assert the row isn't there, then insert the row, then verify the row is there, then rollback. If the referenced Spr9051TestsConfig class is annotated with @Configuration, a rollback is performed as you expect. If @Configuration is NOT there, like it was in my case because I accidentally left it off, the rollback doesn't happen. Instead the data is committed and the next time you run the test it fails along with every other test related to the table (I'm running these tests against an external Postgres DB not an in-memory store). So yeah, it probably took me a hour or more to determine the cause of the non-rollback was due to a missing @Configuration annotation. In the process my hair turned a shade more of gray and I came close to throwing my computer out the window.

Is this enough or do you need me to go to Github and submit a project you can clone and run? I just verified the problem again in my local environment. I can do a Skype screen-share as well; whatever, I'm easy to reach.

Loading

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented May 15, 2012

Keith Donald commented

Just a bump. Please advise if you need any more info :)

Loading

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented May 15, 2012

Sam Brannen commented

Hi Keith,

Thanks for the details of the use case. I think that should be enough to go on for now, and I'll try to take a look at that today.

Cheers,

Sam

Loading

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented May 16, 2012

Sam Brannen commented

Hi guys,

Based on Keith's use case, I put together some tests that investigate the issue.

You can find details about my findings in the message of this commit.

These are the tests involved:

Pay attention to the JavaDoc and NOTEs in the code, especially with regard to the the data sources in TransactionalAnnotatedConfigClassesWithoutAtConfigurationTests.

Based on the assumption that Keith's use case is similar, my conclusion is that the use of such annotated classes in configuration lite mode is not a bug in Spring Core or the TestContext Framework but rather a configuration error.

Of course, as previously mentioned, this could potentially be a rather common configuration error, and #14037 will hopefully help to alleviate such errors in testing scenarios.

I'd also like to point out that similar configuration errors can occur in production code. So this scenario is not limited to integration tests.

@Keith: can you please confirm that the cause of the undesired behavior in your tests is in fact related to the use of multiple data sources (or similar), resulting from the implicit use of configuration lite mode?

Thanks!

Sam

Loading

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented May 16, 2012

Keith Donald commented

Ok, so because I forgot @Configuration my dataSource() @Bean method was used as a factory method not a bean method. I'm sure that's what was causing the problem in my case, then.

I agree forgetting @Configuration that leads to strange behavior will be a common error that's difficult to trace, especially without clear documentation and logging. I expect most people would think a @Bean to be treated as a @Bean whether or not there is a @Configuration annotation on the enclosing class. I'm not sure what the use cases are for "lite" mode. In any case it's not obvious what it means or when it's enabled.

Loading

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented May 16, 2012

Chris Beams commented

I never advocate the use of "lite" mode. It is literally there as an escape hatch for people who really don't want a CGLIB dependency. Beyond that, it is simply a less functional version of "full" mode. There are no functional use cases where it is an advantage.

Loading

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented May 18, 2012

Sam Brannen commented

FYI: I added tests to verify proper scoping support for @Bean methods in lite mode in the TCF. Details on GitHub.

Just to clarify: beans created in lite mode do in fact get scoped.

Loading

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented May 18, 2012

Sam Brannen commented

If you are following this issue, you may also be interested in #14061.

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants