Junit5: Annotation lookup on @Nested constructor parameter fails if test class is compiled with JDK 8

Created on 27 Mar 2018 · 52Comments · Source: junit-team/junit5

Bug Report

Version: junit-jupiter-api:5.1.0

Given the following Extension which supports a custom @Parameterized annotation:

public class TestExtension implements ParameterResolver {

    @Override
    public boolean supportsParameter(ParameterContext parameterContext, ExtensionContext extensionContext) throws ParameterResolutionException {
        return parameterContext.getParameter().isAnnotationPresent(Parameterized.class);
    }

    @Override
    public Object resolveParameter(ParameterContext parameterContext, ExtensionContext extensionContext) throws ParameterResolutionException {
        return new Object();
    }

    @Target(PARAMETER)
    @Retention(RUNTIME)
    public @interface Parameterized {
    }

}

And the following usage of the extension, by annotating the constructor parameters:

@ExtendWith(TestExtension.class)
class ExampleTest {

    private final Object bar;

    ExampleTest(@TestExtension.Parameterized Object bar) {
        this.bar = bar;
    }

    @Test
    void parameter_annotation_in_root_class_does_not_throw() {
        assertNotNull(this.bar);
    }

    @Nested
    class NestedClass {
        private final Object foo;

        NestedClass(@TestExtension.Parameterized Object foo) {
            this.foo = foo;
        }

        @Test
        void parameter_annotation_in_nested_class_does_not_throw() {
            assertNotNull(this.foo);
        }
    }
}

Then when executing this class file, the test defined in ExampleTest will pass, but the test in NestedClass will fail with the following exception:

/usr/lib/jvm/java-1.8.0-openjdk-amd64/bin/java -Dvisualvm.id=1767311736474 -ea -Didea.test.cyclic.buffer.size=1048576 -javaagent:/opt/idea-IU-173.3727.127/lib/idea_rt.jar=33845:/opt/idea-IU-173.3727.127/bin -Dfile.encoding=UTF-8 -classpath /opt/idea-IU-173.3727.127/lib/idea_rt.jar:/opt/idea-IU-173.3727.127/plugins/junit/lib/junit-rt.jar:/opt/idea-IU-173.3727.127/plugins/junit/lib/junit5-rt.jar:/usr/lib/jvm/java-1.8.0-openjdk-amd64/jre/lib/charsets.jar:/usr/lib/jvm/java-1.8.0-openjdk-amd64/jre/lib/ext/cldrdata.jar:/usr/lib/jvm/java-1.8.0-openjdk-amd64/jre/lib/ext/dnsns.jar:/usr/lib/jvm/java-1.8.0-openjdk-amd64/jre/lib/ext/icedtea-sound.jar:/usr/lib/jvm/java-1.8.0-openjdk-amd64/jre/lib/ext/jaccess.jar:/usr/lib/jvm/java-1.8.0-openjdk-amd64/jre/lib/ext/localedata.jar:/usr/lib/jvm/java-1.8.0-openjdk-amd64/jre/lib/ext/nashorn.jar:/usr/lib/jvm/java-1.8.0-openjdk-amd64/jre/lib/ext/sunec.jar:/usr/lib/jvm/java-1.8.0-openjdk-amd64/jre/lib/ext/sunjce_provider.jar:/usr/lib/jvm/java-1.8.0-openjdk-amd64/jre/lib/ext/sunpkcs11.jar:/usr/lib/jvm/java-1.8.0-openjdk-amd64/jre/lib/ext/zipfs.jar:/usr/lib/jvm/java-1.8.0-openjdk-amd64/jre/lib/jce.jar:/usr/lib/jvm/java-1.8.0-openjdk-amd64/jre/lib/jsse.jar:/usr/lib/jvm/java-1.8.0-openjdk-amd64/jre/lib/management-agent.jar:/usr/lib/jvm/java-1.8.0-openjdk-amd64/jre/lib/resources.jar:/usr/lib/jvm/java-1.8.0-openjdk-amd64/jre/lib/rt.jar:/home/tim/Projects/mockito/subprojects/junit-jupiter/out/test/classes:/home/tim/Projects/mockito/subprojects/junit-jupiter/out/production/classes:/home/tim/Projects/mockito/out/production/classes:/home/tim/.gradle/caches/modules-2/files-2.1/org.junit.jupiter/junit-jupiter-api/5.1.0/370218fbc7ce9cf0600c4b5db400bccdf0c01a48/junit-jupiter-api-5.1.0.jar:/home/tim/.gradle/caches/modules-2/files-2.1/org.assertj/assertj-core/2.9.0/5c5ae45b58f12023817abe492447cdc7912c1a2c/assertj-core-2.9.0.jar:/home/tim/.gradle/caches/modules-2/files-2.1/org.junit.platform/junit-platform-launcher/1.1.0/ba098edde4e59cacd9225e238ea3ad9c946684ab/junit-platform-launcher-1.1.0.jar:/home/tim/.gradle/caches/modules-2/files-2.1/org.junit.jupiter/junit-jupiter-engine/5.1.0/c54b96b465bb5b49c7708d734a4180dd95422754/junit-jupiter-engine-5.1.0.jar:/home/tim/.gradle/caches/modules-2/files-2.1/net.bytebuddy/byte-buddy/1.8.0/f7c50fcf1fab4fa3e148ecf6b329f01f733ed427/byte-buddy-1.8.0.jar:/home/tim/.gradle/caches/modules-2/files-2.1/net.bytebuddy/byte-buddy-agent/1.8.0/feacb6818aaad11abc792a86a587e4ee5af3008c/byte-buddy-agent-1.8.0.jar:/home/tim/.gradle/caches/modules-2/files-2.1/org.objenesis/objenesis/2.6/639033469776fd37c08358c6b92a4761feb2af4b/objenesis-2.6.jar:/home/tim/.gradle/caches/modules-2/files-2.1/org.junit.platform/junit-platform-engine/1.1.0/2596bd4d909e7ea8d029272db4338075445d731b/junit-platform-engine-1.1.0.jar:/home/tim/.gradle/caches/modules-2/files-2.1/org.junit.platform/junit-platform-commons/1.1.0/d6aa21029f9cedfb4cc8a9e9efc0bd97987205b8/junit-platform-commons-1.1.0.jar:/home/tim/.gradle/caches/modules-2/files-2.1/org.apiguardian/apiguardian-api/1.0.0/3ef5276905e36f4d8055fe3cb0bdcc7503ffc85d/apiguardian-api-1.0.0.jar:/home/tim/.gradle/caches/modules-2/files-2.1/org.opentest4j/opentest4j/1.0.0/6f09c598e9ff64bf0ce2fa7e7de49a99ba83c0b4/opentest4j-1.0.0.jar com.intellij.rt.execution.junit.JUnitStarter -ideVersion5 -junit5 org.mockitousage.ExampleTest

org.junit.jupiter.api.extension.ParameterResolutionException: Failed to resolve parameter [java.lang.Object arg1] in executable [org.mockitousage.ExampleTest$NestedClass(org.mockitousage.ExampleTest,java.lang.Object)]

    at org.junit.jupiter.engine.execution.ExecutableInvoker.resolveParameter(ExecutableInvoker.java:221)
    at org.junit.jupiter.engine.execution.ExecutableInvoker.resolveParameters(ExecutableInvoker.java:174)
    at org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:82)
    at org.junit.jupiter.engine.descriptor.NestedClassTestDescriptor.instantiateTestClass(NestedClassTestDescriptor.java:77)
    at org.junit.jupiter.engine.descriptor.ClassTestDescriptor.instantiateAndPostProcessTestInstance(ClassTestDescriptor.java:195)
    at org.junit.jupiter.engine.descriptor.ClassTestDescriptor.lambda$testInstanceProvider$0(ClassTestDescriptor.java:185)
    at org.junit.jupiter.engine.descriptor.ClassTestDescriptor.lambda$testInstanceProvider$1(ClassTestDescriptor.java:189)
    at java.util.Optional.orElseGet(Optional.java:267)
    at org.junit.jupiter.engine.descriptor.ClassTestDescriptor.lambda$testInstanceProvider$2(ClassTestDescriptor.java:188)
    at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.prepare(TestMethodTestDescriptor.java:81)
    at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.prepare(TestMethodTestDescriptor.java:58)
    at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor$NodeExecutor.prepare(HierarchicalTestExecutor.java:89)
    at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor$NodeExecutor.execute(HierarchicalTestExecutor.java:74)
    at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor$NodeExecutor.lambda$executeRecursively$2(HierarchicalTestExecutor.java:120)
    at java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:184)
    at java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:175)
    at java.util.Iterator.forEachRemaining(Iterator.java:116)
    at java.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:1801)
    at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:481)
    at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471)
    at java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:151)
    at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:174)
    at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
    at java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:418)
    at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor$NodeExecutor.lambda$executeRecursively$3(HierarchicalTestExecutor.java:120)
    at org.junit.platform.engine.support.hierarchical.SingleTestExecutor.executeSafely(SingleTestExecutor.java:66)
    at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor$NodeExecutor.executeRecursively(HierarchicalTestExecutor.java:108)
    at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor$NodeExecutor.execute(HierarchicalTestExecutor.java:79)
    at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor$NodeExecutor.lambda$executeRecursively$2(HierarchicalTestExecutor.java:120)
    at java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:184)
    at java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:175)
    at java.util.Iterator.forEachRemaining(Iterator.java:116)
    at java.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:1801)
    at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:481)
    at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471)
    at java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:151)
    at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:174)
    at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
    at java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:418)
    at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor$NodeExecutor.lambda$executeRecursively$3(HierarchicalTestExecutor.java:120)
    at org.junit.platform.engine.support.hierarchical.SingleTestExecutor.executeSafely(SingleTestExecutor.java:66)
    at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor$NodeExecutor.executeRecursively(HierarchicalTestExecutor.java:108)
    at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor$NodeExecutor.execute(HierarchicalTestExecutor.java:79)
    at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor$NodeExecutor.lambda$executeRecursively$2(HierarchicalTestExecutor.java:120)
    at java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:184)
    at java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:175)
    at java.util.Iterator.forEachRemaining(Iterator.java:116)
    at java.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:1801)
    at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:481)
    at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471)
    at java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:151)
    at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:174)
    at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
    at java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:418)
    at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor$NodeExecutor.lambda$executeRecursively$3(HierarchicalTestExecutor.java:120)
    at org.junit.platform.engine.support.hierarchical.SingleTestExecutor.executeSafely(SingleTestExecutor.java:66)
    at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor$NodeExecutor.executeRecursively(HierarchicalTestExecutor.java:108)
    at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor$NodeExecutor.execute(HierarchicalTestExecutor.java:79)
    at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:55)
    at org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine.execute(HierarchicalTestEngine.java:43)
    at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:170)
    at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:154)
    at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:90)
    at com.intellij.junit5.JUnit5IdeaTestRunner.startRunnerWithArgs(JUnit5IdeaTestRunner.java:65)
    at com.intellij.rt.execution.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:47)
    at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:242)
    at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:70)
Caused by: java.lang.ArrayIndexOutOfBoundsException: 1
    at java.lang.reflect.Parameter.getDeclaredAnnotations(Parameter.java:305)
    at java.lang.reflect.Parameter.declaredAnnotations(Parameter.java:342)
    at java.lang.reflect.Parameter.getAnnotation(Parameter.java:287)
    at java.lang.reflect.AnnotatedElement.isAnnotationPresent(AnnotatedElement.java:258)
    at org.mockito.junit.jupiter.TestExtension.supportsParameter(TestExtension.java:17)
    at org.junit.jupiter.engine.execution.ExecutableInvoker.lambda$resolveParameter$0(ExecutableInvoker.java:185)
    at java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:174)
    at java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:193)
    at java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:175)
    at java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1380)
    at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:481)
    at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471)
    at java.util.stream.StreamSpliterators$WrappingSpliterator.forEachRemaining(StreamSpliterators.java:312)
    at java.util.stream.Streams$ConcatSpliterator.forEachRemaining(Streams.java:743)
    at java.util.stream.Streams$ConcatSpliterator.forEachRemaining(Streams.java:742)
    at java.util.stream.Streams$ConcatSpliterator.forEachRemaining(Streams.java:742)
    at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:481)
    at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471)
    at java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:708)
    at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
    at java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:499)
    at org.junit.jupiter.engine.execution.ExecutableInvoker.resolveParameter(ExecutableInvoker.java:186)
    ... 66 more


Process finished with exit code 255

As such, resolution of a parameter in constructor of a @Nested test class fails, while the same parameter resolved on the root class constructor works just fine.

I encountered this issue while testing out the capabilities of the MockitoExtension I was developing.

Deliverables

[x] Introduce convenience methods for looking up annotations on parameters in ParameterContext.
[x] Document in User Guide.
[x] Document in Release Notes.

Jupiter extensions bug enhancement

Source

TimvdLippe

All 52 comments

I should note that this exception is not thrown for annotations on parameters of test methods defined in a nested test class, nor does it throw on annotations on fields in a nested class.

TimvdLippe on 27 Mar 2018

Thanks for raising the issue.

We'll look into it!

sbrannen on 27 Mar 2018

Thanks to your example, I am able to reproduce this behavior against master.

So that gives us a sound foundation to track down the bug.

Cheers!

sbrannen on 27 Mar 2018

Well, I've got some good news and some bad news.

It took me quite a bit of debugging... that lead me to believe it was actually a bug in the JDK.

That turned out to be true.

Good News: it's been fixed!
Bad News: it's only been fixed in JDK 9+. 😞

Related JDK bugs:

The place where it actually blows up is in java.lang.reflect.Parameter.getDeclaredAnnotations(), which looks like this:

    public Annotation[] getDeclaredAnnotations() {
        return executable.getParameterAnnotations()[index];
    }

The problem is that JDK 8 compiles the wrong annotation arrays into the byte code. So there's an "off by one" error when indexing the array returned by the executable (i.e., the constructor for the _inner_ class in the example in this issue).

Through a bit of debugging, I determined that the annotation is _present_ on the invisible (synthetic) first argument to the constructor for the inner class which is actually the parameter for the outer instance.

Crazy!

So with that in mind, I'm afraid we'll have to close this bug report and leave it as is.

sbrannen on 27 Mar 2018

Or....

@junit-team/junit-lambda / @TimvdLippe ...

Anyone have any ideas for a robust way to handle this?

sbrannen on 27 Mar 2018

By the way, here's the technical explanation for the bug in JDK 8 (copied from the above linked bug issue):

Root cause is javac generates RuntimeVisibleParameterAnnotations without an annotation slot for the synthetic/mandated parameters.

sbrannen on 27 Mar 2018

@sbrannen Thanks for the detailed investigation! The options are limited I fear, but maybe the following would work?

Introduce a new interface AnnotatedParameterResolver that extends ParameterResolver and instead has a resolveForAnnotation that can return the @interface class that we want to resolve for. (The examples thus far all look for annotations, so it would be nice to abstract this logic away). Then in the logic of handling the annotation, JUnit could take care of this bug, by correctly searching for the annotation in the annotations array.

Essentially this would interface would receive a Class it would manually look in the annotations on parameterContext.getParameter() to see if it is required.

TimvdLippe on 27 Mar 2018

FWIW, I actually tested the example in this issue against JDK 9.0.4, and the nested test passes.

sbrannen on 27 Mar 2018

Yep, I totally agree that the options are limited for an issue like this (i.e., a bug in the compiled byte code).

sbrannen on 27 Mar 2018

@TimvdLippe, I'm not so sure we'd want to introduce a new extension API just to handle this.

But... I am _considering_ adding an additional method to ParameterContext that performs the annotation lookup for the extension.

sbrannen on 27 Mar 2018

But... if we do address this somehow, extension authors would have to be made aware of the problem, and it would never be intuitive to be _forced_ to use the convenience method _only_ when resolving a constructor parameter for a @Nested test class that was compiled on JDK 8.

sbrannen on 27 Mar 2018

TBH, I don't even know how easy it would be for JUnit to check if the test class was compiled on JDK 8.

sbrannen on 27 Mar 2018

OK, I think I have a means for determining if the byte code is messed up.

I added the following to the if (outerInstance != null) block within ExecutableInvoker.resolveParameters(Executable, Optional<Object>, Object, ExtensionContext, ExtensionRegistry).

System.err.format("%d: %s --> %s%n", 0, parameter, Arrays.toString(parameter.getAnnotations()));
System.err.format(">>> executable.getParameters().length: %d%n", executable.getParameters().length);
System.err.format(">>> executable.getParameterAnnotations().length: %d%n", executable.getParameterAnnotations().length);

And these are the results...

JDK 8

0: final org.junit.ExampleTest this$0 --> [@org.junit.TestExtension$Parameterized()]
>>> executable.getParameters().length: 2
>>> executable.getParameterAnnotations().length: 1

JDK 9+

0: final org.junit.ExampleTest this$0 --> []
>>> executable.getParameters().length: 2
>>> executable.getParameterAnnotations().length: 2

So it looks like comparing the lengths of the parameters and parameterAnnotations arrays might be a viable option.

sbrannen on 27 Mar 2018

If we do that and then do this within the for-loop...

Parameter parameter = (outerInstance == null) ? parameters[i] : parameters[i - 1];

... the test then passes on JDK 8.

Except... that we have to comment out the invocation of validateResolvedType(...) since it now checks the resolved type against the Parameter for the synthetic argument for the enclosing instance.

Which means we'd have to _hack_ that algorithm as well... so that it actually checks the resolved type against the i + 1 Parameter object.

Starting to sound _very hacky_ to me......

sbrannen on 27 Mar 2018

Of course, we wouldn't actually want to provide a ParameterResolver the Parameter for the enclosing instance.

I was just investigating if we could even find the correct information and make it available to extension authors via a _convenience method_ that does not use the Parameter annotation lookup APIs directly.

sbrannen on 27 Mar 2018

Update: changed label from bug to enhancement since this is not due to a bug in JUnit.

sbrannen on 27 Mar 2018

FYI: I verified that the SpringExtension is naturally affected by this as well...

https://github.com/spring-projects/spring-framework/commit/75f70b269ed89294330758eb61ee8d7f218c9af6

sbrannen on 27 Mar 2018

_in progress_ / _team discussion_

sbrannen on 27 Mar 2018

On a positive note, the following is a means for _fixing_ / _addressing_ the issue within the ParameterResolver implementation itself.

public boolean supportsParameter(ParameterContext parameterContext, ExtensionContext extensionContext) {
    Executable executable = parameterContext.getDeclaringExecutable();

    Parameter parameter = parameterContext.getParameter();
    // Take into account a bug in javac in JDK 8:
    if (executable.getParameters().length == executable.getParameterAnnotations().length + 1) {
        parameter = executable.getParameters()[parameterContext.getIndex() - 1];
    }

    return parameter.isAnnotationPresent(Parameterized.class);
}

Making that change causes all tests in the provided example to pass! 😜

sbrannen on 27 Mar 2018

Oh that is great news! Would be awesome if that could be included (as it does not seem to be an overly complicated hack :thinking: )

TimvdLippe on 27 Mar 2018

yeah.... that's my current line of thinking...

adding that _hack_ to some new convenience methods in the ParameterContext API.

sbrannen on 27 Mar 2018

FYI: commit e1e477cd5d3ce699f2b0f5df2687cf76eff5c7ba introduces a comparable test case within the JUnit Jupiter test suite.

Thus any _fix_ for this issue can be verified by removing the @DisabledOnJre(JAVA_8) declaration on the newly introduced nested test class (AnnotatedConstructorParameterNestedTestCase).

sbrannen on 27 Mar 2018

Would be awesome if that could be included (as it does not seem to be an overly complicated hack)

I'd love to make this transparent to end users.

Unfortunately, I don't see how that is possible since java.lang.reflect.Parameter is a final class. 😞

sbrannen on 27 Mar 2018

Oh I was thinking of something in ParameterContext. Let me test that out tomorrow and potentially open a PR

TimvdLippe on 27 Mar 2018

Let me test that out tomorrow and potentially open a PR

Thanks for the offer!

But... I've already found the solution; the issue is assigned to me; and it's labeled _in progress_.

So, in other words... I'm on it! 😉

sbrannen on 28 Mar 2018

Oh I was thinking of something in ParameterContext.

Yes, of course: that's the only option since it cannot be done transparently (i.e., with zero changes to existing implementations).

sbrannen on 28 Mar 2018

OK, here's my current Proposal.

Additions to ParameterContext API:

boolean isAnnotated(Class<? extends Annotation> annotationType);

<A extends Annotation> Optional<A> findAnnotation(Class<A> annotationType);

<A extends Annotation> List<A> findRepeatableAnnotations(Class<A> annotationType);

Corresponding implementation in DefaultParameterContext:

@Override
public boolean isAnnotated(Class<? extends Annotation> annotationType) {
    return AnnotationUtils.isAnnotated(getAnnotatedElement(), annotationType);
}

@Override
public <A extends Annotation> Optional<A> findAnnotation(Class<A> annotationType) {
    return AnnotationUtils.findAnnotation(getAnnotatedElement(), annotationType);
}

@Override
public <A extends Annotation> List<A> findRepeatableAnnotations(Class<A> annotationType) {
    return AnnotationUtils.findRepeatableAnnotations(getAnnotatedElement(), annotationType);
}

private AnnotatedElement getAnnotatedElement() {
    Executable executable = getDeclaringExecutable();

    // Take into account a bug in javac in JDK 8:
    if (executable instanceof Constructor //
            && isInnerClass(executable.getDeclaringClass()) //
            && (executable.getParameters().length == executable.getParameterAnnotations().length + 1)) {
        return executable.getParameters()[this.index - 1];
    }

    return this.parameter;
}

That seems to do the job.

@TimvdLippe and @junit-team/junit-lambda, Thoughts?

sbrannen on 28 Mar 2018

That looks great. One question: Is there a particular reason to name it isAnnotated rather than isAnnotationPresent as used in the Parameter API?

TimvdLippe on 28 Mar 2018

Food for thought:

I'm thinking we might want to expose getAnnotatedElement() to extension authors as well so that they can use their own approach for finding annotations, etc. (e.g., like Spring might want to do).

But if we do that, we'll have to very carefully document exactly what that annotated _element_ is.

Other options for the name might be something like getEffectiveAnnotatedElement() or getEffectiveAnnotatedParameter().

Thoughts?

sbrannen on 28 Mar 2018

Is there a particular reason to name it isAnnotated rather than isAnnotationPresent as used in the Parameter API?

Yes. isAnnotated carries the same semantics as ReflectionUtils.isAnnotated which provides support for finding meta-annotations, etc.

And isAnnotationPresent does not carry (or implement) those semantics.

sbrannen on 28 Mar 2018

👍1

Related issues

assertEquals(expected, actual, delta) should allow a delta of zero

lessthanoptimal · 4Comments

Tests in default package do not get picked up via classpath scanning

otrosien · 4Comments

Method `Assertions.assertThrows(expectedType, executable, message)` doesn't check the contents of the exception message.

borisilic · 4Comments

Review experimental APIs and decide which to promote in 5.7

marcphilipp · 3Comments

Support disabling cleanup for @TempDir

solter · 4Comments