fix #944: provide the resolved parameters in the ExtensionContext #3929
Conversation
There was a problem hiding this comment.
Thanks for the PR! I'll bring this up with the team to discuss the high-level design before we get too deep.
I'm not sure, if this ordering could be inverted: first the parameter resolvers, then the
BeforeEachhandling.
Yes, I think we'd have to explicitly resolve the test method's parameters (and only those!) before calling any method-level extensions or lifecycle callbacks.
| @@ -135,6 135,7 @@ private static Object resolveParameter(ParameterContext parameterContext, Execut | |||
| ParameterResolver resolver = matchingResolvers.get(0); | |||
| Object value = resolver.resolveParameter(parameterContext, extensionContext); | |||
| validateResolvedType(parameterContext.getParameter(), value, executable, resolver); | |||
| recordResolvedParameter(extensionContext, value); | |||
There was a problem hiding this comment.
This would also record parameters for methods invoked via ExtensionContext.getExecutableInvoker(), right? I think we need to ensure that we only record them for the test method that's being executed, e.g. by checking executable equals extensionContext.getTestMethod().
| @@ -135,6 135,7 @@ private static Object resolveParameter(ParameterContext parameterContext, Execut | |||
| ParameterResolver resolver = matchingResolvers.get(0); | |||
| Object value = resolver.resolveParameter(parameterContext, extensionContext); | |||
| validateResolvedType(parameterContext.getParameter(), value, executable, resolver); | |||
| recordResolvedParameter(extensionContext, value); | |||
There was a problem hiding this comment.
Is there a benefit of recording these one-by-one rather than in a single go?
Overview
As (almost) suggested in #944, I've added a method
ExtensionContext#getResolvedParameters(andgetRequiredResolvedParameters) and an interfaceResolvedParameters. TheAbstractExtensionContextcan store and return those. TheParameterResolutionUtilsstore them, but I had to introduce another but internal interfaceResolvedParametersStoreto make this possible without hurting the visibility scopes betweendescriptorandexecution.The problem I ran into is that
TestMethodTestDescriptor#executecalls the parameter resolvers after allBeforeEachcallbacks and methods. So they can't see them, but that was the purpose of it all.I'm not sure, if this ordering could be inverted: first the parameter resolvers, then the
BeforeEachhandling.I hereby agree to the terms of the JUnit Contributor License Agreement.
Definition of Done
@APIannotations