[
https://issues.apache.org/jira/browse/SLING-8312?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16807633#comment-16807633
]
Julian Sedding commented on SLING-8312:
---------------------------------------
bq. [...] SlingContext/OsgiContext (without Impl) depend on the JUnit version
they are compiled against (JUnit 4 / Junit 5) [...]
Yes, that's a shame. I suppose it would be easier if the {{*Context}} classes
were framework agnostic and hooked into the testing frameworks using
composition rather than inheritance (i.e. an {{ExternalResource}} rule that
manages the {{*Context}}'s life-cycle and provides access to it). But we're
always wiser in hindsight.
bq. [...] plugins should always declared against
OsgiContextImpl/SlingContextImpl [...]
I didn't know that. It seems very unintuitive that you *should* use the
{{*Impl}} class rather than the non-{{Impl}}. I agree though, that the
non-{{Impl}} classes provide no benefit (i.e. API) to the user, because they
only serve to hook into the test frameworks.
bq. i've not idea how this could be solved better in the generics expression
We could use signatures in the *Builder classes like the following (note the
generic {{<? super SlingContextImpl>}}):
{code:java}
public final @NotNull SlingContextBuilder plugin(@NotNull ContextPlugin<? super
SlingContextImpl> @NotNull ... plugin) {
plugins.addPlugin(plugin);
return this;
}
{code}
This snippet would automatically suggest {{new
AbstractContextPlugin<SlingContextImpl>()}} in Intellij (didn't try in
Eclipse). Furthermore, the signature only allows plugins for
{{SlingContextImpl}} and its super classes, thus preventing compilation of code
that leads to the above runtime error.
Unfortunately this change would not be backwards compatible. I expect most
people use {{OsgiContext}}/{{SlingContext}} without the {{*Impl}}. The
signatures of all their plugins would need to be adjusted.
Just using the generic {{<? super SlingContext>}} would improve the situation,
but IDEs would encourage users to build plugins against this class, which you
say is wrong/undesirable. It would also restrict compilation of code using
{{ContextPlugin<OsgiContext>}} as an argument to
{{SlingContextBuilder.plugin()}}.
So I don't know where best to go from here. It's basically a trade-off between
improving the API and usability for new code vs backwards compatibility for
older code. WDYT?
PS: regarding the use of generics in API design, there is a [nice explanation
on stack
exchange|https://stackoverflow.com/questions/2723397/what-is-pecs-producer-extends-consumer-super#2723538].
> Generics in OsgiContextBuilder allow compilation of code that cannot run
> ------------------------------------------------------------------------
>
> Key: SLING-8312
> URL: https://issues.apache.org/jira/browse/SLING-8312
> Project: Sling
> Issue Type: Bug
> Components: Testing
> Affects Versions: Testing OSGi Mock 2.4.6
> Reporter: Julian Sedding
> Priority: Minor
>
> While using the {{OsgiContextBuilder}} class, I noticed that e.g. when adding
> a {{ContextPlugin}}, my IDE's code completion created callback methods with
> an {{OsgiContextImpl}} argument. Since I want to initialize an
> {{OsgiContext}} that struck me as odd.
> Suspecting that the generics are not quite right/ideal, I experimented a bit
> more and noticed that I can write and compile the following code, which leads
> to a runtime exception (note the {{SlingContext}} argument):
> {code:java}
> @Rule
> public OsgiContext context = new OsgiContextBuilder()
> .plugin(new AbstractContextPlugin<SlingContext>() {
> @Override
> public void beforeSetUp(@NotNull SlingContext context) throws
> Exception {
> // ... initialize context for test ...
> }
> })
> .build();
> {code}
> The exception is:
> {noformat}
> java.lang.RuntimeException: Before setup failed
> (com.foo.OsgiTest$1@5b464ce8):
> org.apache.sling.testing.mock.sling.junit.SlingContext cannot be cast to
> org.apache.sling.testing.mock.osgi.junit.OsgiContext
> at
> org.apache.sling.testing.mock.osgi.context.ContextPlugins.executeBeforeSetUpCallback(ContextPlugins.java:201)
> at
> org.apache.sling.testing.mock.sling.junit.SlingContext$1.before(SlingContext.java:145)
> at org.junit.rules.ExternalResource$1.evaluate(ExternalResource.java:46)
> at org.junit.rules.RunRules.evaluate(RunRules.java:20)
> at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
> at
> org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
> at
> org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
> at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
> at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
> at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
> at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
> at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
> at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
> at
> org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:369)
> at
> org.apache.maven.surefire.junit4.JUnit4Provider.executeWithRerun(JUnit4Provider.java:275)
> at
> org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:239)
> at
> org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:160)
> at
> org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:373)
> at
> org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:334)
> at
> org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:119)
> at
> org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:407)
> Caused by: java.lang.ClassCastException:
> org.apache.sling.testing.mock.sling.junit.SlingContext cannot be cast to
> org.apache.sling.testing.mock.osgi.junit.OsgiContext
> at com.foo.OsgiTest$1.beforeSetUp(ResourceCopierTest.java:18)
> at
> org.apache.sling.testing.mock.osgi.context.ContextPlugins.executeBeforeSetUpCallback(ContextPlugins.java:198)
> ... 20 more
> {noformat}
> IMHO it would make sense to address this in a way that
> - the IDE suggests the "correct" {{*Context}} class (i.e. {{OsgiContext}} for
> {{OsgiContextBuilder}}, {{SlingContext}} for {{SlingContextBuilder}}
> - a {{SlingContextBuilder}} can consume plugins written for an
> {{OsgiContext}} (not currently possible because {{SlingContext extends
> OsgiContextImpl}} (and not {{OsgiContext}}
> - only runnable code can be compiled
> cc [[email protected]]
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
