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

Improve extensibility of TestContext bootstrap and context caching mechanisms [SPR-12683] #17282

Closed
spring-issuemaster opened this issue Feb 3, 2015 · 12 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

commented Feb 3, 2015

Lari Hotari opened SPR-12683 and commented

Currently it's not possible to provide a TestContext or TestContextBootstrapper instance to TestContextManager. This restricts the reusability of it.

I'm currently working on GRAILS-11963 ("Use Spring TestContext Framework for initializing the test context") for Grails 3, and we would like to reuse as much as possible from the Spring TCF.

As a fix, there should be a new constructor that allows passing TestContext and TestContextBootstrapper instances to the TestContextManager - for example:

public TestContextManager(TestContextBootstrapper testContextBootstrapper, TestContext testContext)

Something should be done to the ContextCache contextCache field, since that is static and has package visibility. The ContextCache class isn't a public class either. Perhaps that could be separated from TestContextManager?


Affects: 4.1 GA

Issue Links:

  • #17281 Make resolveContextLoader() method in AbstractTestContextBootstrapper protected

Referenced from: commits 5cbe4b9, 9e6a5ae, c52a0cc, 129488c, 0392a88, 186abcb, c9d597f, e6c24f7

1 votes, 3 watchers

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 3, 2015

Lari Hotari commented

created #732 that adds the constructor.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 3, 2015

Lari Hotari commented

it looks like it would also help if DefaultCacheAwareContextLoaderDelegate, DefaultBootstrapContext and DefaultTestContext were public classes.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 3, 2015

Sam Brannen commented

This is related to #17281.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 24, 2015

David Erickson commented

I too would love to see improvement here, two particular things that would have been very useful to me recently:

  1. Ability to provide my own ContextCache implementation
  2. Ability to provide my own implementation of CacheAwareContextLoaderDelegate
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 15, 2015

Sam Brannen commented

Lari Hotari,

Can you please provide more information on why you need to supply your own TestContext and TestContextBootstrapper to the TestContextManager?

Is the @BootstrapWith mechanism not suitable for your use case?

What is the difference between the DefaultTestContext and your custom TestContext implementation?

Thanks in advance for feedback,

Sam

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 15, 2015

Sam Brannen commented

Lari Hotari,

Similarly, can you please explain why you would like for DefaultCacheAwareContextLoaderDelegate and DefaultBootstrapContext to be public?

Cheers,

Sam

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 15, 2015

Sam Brannen commented

David Erickson,

I too would love to see improvement here, two particular things that would have been very useful to me recently:

  1. Ability to provide my own ContextCache implementation
  2. Ability to provide my own implementation of CacheAwareContextLoaderDelegate

Can you please expound on the concrete use case you have in mind?

Specifically, what is it you wish you achieve that you cannot currently?

Thanks,

Sam

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 15, 2015

Sam Brannen commented

Lari Hotari & David Erickson,

The reason I'm requesting the aforementioned feedback from you is that I need insight into your concrete requirements in order to refactor and/or redesign the mechanisms in a manner that is best for the community and longevity of the Spring TestContext Framework.

Regards,

Sam

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 15, 2015

Lari Hotari commented

Sam, The use case I have is https://jira.grails.org/browse/GRAILS-11963 . Grails has it's own unit testing intrastructure and I was going to use TestContextManager for managing the test context.
We decided to postpone that work (GRAILS-11963) until these issues have been resolved.

I was trying to programmaticly use the TCF to instantiate and control a test context. It wasn't possible to programmaticly control and extend TCF to be used in Grails unit test runtime. I know it's not originally designed to be used like that but making small changes would have solved the problem I faced in my Grails use case.

I'm sorry I cannot recall more accurate details than what I've described in this the issue details of this issue #17282 and #17281.

I don't work on Grails as a full time paid developer any more so I won't be able to provide more feedback now.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 16, 2015

David Erickson commented

Sam Brannen,
The specific use case I am trying to enable is multi-threaded testing. What I'd like to have happen is when a test starts it grabs an instance of its app context from a pool of contexts sharing the same configuration (or creates a new one for the pool), grabs a database from a pool, runs the tests, then returns both. I need a pool of contexts per configuration because the Datasource must be different in each actively used app context. I'd ideally also like hooks that I can specify to do things when checking out a context from the pool, putting it back, etc etc. Does that make sense?

Thanks,
David

edit - I also have a hacked up prototype version of this if it would be useful, I dropped you an email on this a couple of months back.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 19, 2015

Sam Brannen commented

Resolved in the following commits:

Highlights:

  • Moved static ContextCache to DefaultCacheAwareContextLoaderDelegate.
  • Converted ContextCache to interface with default implementation.
    • ContextCache is now a public interface.
    • Introduced public DefaultContextCache implementation in the 'support' subpackage.
  • Introduced logStatistics() method in the ContextCache API and defined statistics logging category as a constant.
  • DefaultCacheAwareContextLoaderDelegate now delegates to ContextCache.logStatistics().
  • DefaultBootstrapContext and DefaultCacheAwareContextLoaderDelegate are now public classes in the 'support' subpackage.
  • Introduced getCacheAwareContextLoaderDelegate() in AbstractTestContextBootstrapper as an extension point for configuring custom ContextCache support.
  • Introduced reflection-based createBootstrapContext() utility method in BootstrapUtils; TestContextManager now delegates to BootstrapUtils in order to avoid package cycles.
  • DefaultTestContext is now a public class residing in the "support" subpackage.
  • Introduced buildTestContext() in TestContextBootstrapper.
    • Moved the responsibility of building a TestContext from the TestContextManager to a TestContextBootstrapper.
  • Introduced TestContextManager(TestContextBootstrapper) constructor.
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 19, 2015

Sam Brannen commented

Hi guys,

Please take a look at the recent commits related to this issue and let me know if these changes cover your use cases.

Thanks,

Sam

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.