spring-projects / spring-framework

Spring Framework
https://spring.io/projects/spring-framework
Apache License 2.0
56.55k stars 38.12k forks source link

TestContext to become subclassable [SPR-5428] #10102

Closed spring-projects-issues closed 11 years ago

spring-projects-issues commented 15 years ago

Grzegorz Olędzki opened SPR-5428 and commented

org.springframework.test.context.TestContext declares a package-protected constructor (the only constructor there). It would be better to make it protected instead - so that one can subclass it in his/her own package.

The same would apply to methods there - why not make them protected instead of private: loadApplicationContext() contextKeyString()


Affects: 2.5.6

Issue Links:

spring-projects-issues commented 15 years ago

Sam Brannen commented

Hi Grzegorz,

The visibility of TestContext's constructor and methods is intentionally limited in order to hide the implementation details of the context cache, etc.

Developers typically should have no need to instantiate or subclass TestContext. Rather, developers will typically use a TestContextManager directly, which in turns instantiates and configures the TestContext that it manages.

What concrete use case do you have which prompted you to raise this JIRA issue?

Moreover, why would you want protected access to loadApplicationContext() and contextKeyString()?

Regards,

Sam

spring-projects-issues commented 15 years ago

Grzegorz Olędzki commented

Hi Sam,

The use case is to have a different way of application contexts being constructed for tests. In our setup there was a need for using parent link between containers instead of setting up a different container based on the sum of file locations.

To be more precise, follow the example:

@ContextConfiguration(locations={"fileA.xml"})
class TestA {
}

@ContextConfiguration(locations={"fileB.xml"})
class TestB extends TestA {
}

Using standard SpringJUnit4ClassRunner would cause two separate application contexts to be created:

The approach we prefer is to have:

It seems to me that it's easier doable being able to subclass TestContext, TestContextManager and SpringJUnit4ClassRunner. I am not quite sure now about contextKeyString() method, but loadApplicationContext() seems still reasonable.

Regards, Grzegorz

spring-projects-issues commented 11 years ago

Piotr Findeisen commented

Hi,

I've just stumbled upon that.

I wanted to mock (Mockito) TestContext to be able to test my class extending AbstractTestExecutionListener - following crazy idea of TTD, applied to the test utilities themselves. All would be fine if only methods in TestContext were not final.

I've ended up with not-so readable reflection-based workaround (fest-reflect saved my life), instead of straight-forward mock

MergedContextConfiguration mergedContextConfiguration = new MergedContextConfiguration(testClass,
        new String[] { "I wish SPR-5428 was resolved" },
        new Class[0], new String[0],
        null);

Class<?> contextCacheClass = Class.forName("org.springframework.test.context.ContextCache");
Object contextCache = Reflection.constructor()
        .in(contextCacheClass)
        .newInstance();

Reflection.field("contextMap")
        .ofType(new TypeRef<Map<MergedContextConfiguration, ApplicationContext>>() {
        })
        .in(contextCache)
        .get().put(mergedContextConfiguration, applicationContext);

TestContext testContext = Reflection.constructor()
        .withParameterTypes(Class.class, contextCacheClass)
        .in(TestContext.class)
        .newInstance(testClass, contextCache);

Reflection.field("mergedContextConfiguration")
        .ofType(MergedContextConfiguration.class)
        .in(testContext)
        .set(mergedContextConfiguration);

Reflection.method("updateState")
        .withParameterTypes(Object.class, Method.class, Throwable.class)
        .in(testContext)
        .invoke(testInstance, testMethod, null);

All this hackish code should not be necessary.

spring-projects-issues commented 11 years ago

Piotr Findeisen commented

For me, inability (since I would not call my workaround "an ability") to test listeners is a Bug, and not with a minor priority.

Could you please therefore raise the priority a bit? Or perhaps I should report a separate issue for that (since my problem is not 100% equal to the original one)?

spring-projects-issues commented 11 years ago

Sam Brannen commented

Piotr Findeisen, there's no need to create a new issue since your wish is already addressed in #12348. So feel free to watch that issue instead of this one for testability of listeners.

Regards,

Sam

spring-projects-issues commented 11 years ago

Sam Brannen commented

In light of the fact that both #10284 and #12348 have been resolved since this issue was raised, I am resolving this issue as Won't Fix.

However, if you feel that there is still a need for elements of this issue in spite of recent changes in the TestContext framework, please provide feedback here.

Thanks!

Sam