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

[312] Initial attempt to allow Arquillian to inject method parameters. #608

Merged
merged 2 commits into from
Aug 13, 2024

Conversation

jamezp
Copy link
Collaborator

@jamezp jamezp commented Aug 9, 2024

Short description of what this resolves:

Allows method parameters which are provided by Arquillian to be injected. The parameters are resolved from TestEnricher's.

Note that for in-container tests, a ParameterResolver outside of Arquillian does not work. In other words, you can't inject a method parameter like @TempDir final Path path because two resolvers will be found. One being the real resolver, the other being the ArquillianExtension. This seems to be due to the fact that the parameter resolver is executed twice for in-container tests. Once for invoking the test from client and once executing in the container itself.

To work around the above, I created a @Vetoed annotation which can be set on those parameters to allow Arquillian to ignore them. Note I don't know if using the same name as the CDI annotation is good or not, but I couldn't think of a better name :)

Changes proposed in this pull request:

  • Add a ParameterResolver for resolved method parameters which Arquillian can provide. For example a @ArquillianResource URI uri.
  • This works both on the client and in the container, with some limitations

Fixes #312

@jamezp jamezp force-pushed the issue312 branch 2 times, most recently from 06053d0 to d320658 Compare August 9, 2024 22:05
@jamezp jamezp marked this pull request as ready for review August 9, 2024 22:07
@jamezp jamezp force-pushed the issue312 branch 3 times, most recently from d4249a9 to d5402a1 Compare August 9, 2024 23:12
@jamezp jamezp requested a review from rhusar August 9, 2024 23:33
@jamezp jamezp marked this pull request as draft August 10, 2024 00:24
@jamezp
Copy link
Collaborator Author

jamezp commented Aug 10, 2024

I've moved this back to a draft as there is an issue. The issues is we need to ensure we get the correct parameters for a @BeforeEach and @AfterEeach method. The @BeforeAll and @AfterAll do not work, but I don't think that is a huge issue.

…ters. The parameters are resolved from TestEnrichers.

Signed-off-by: James R. Perkins <[email protected]>
@starksm64
Copy link
Member

It looks like the MethodParameterObserver#injectParameters needs to handle a TestEnricher throwing an exception and ignore that as in the Payara integration tests the org.jboss.arquillian.container.test.impl.enricher.resource.InitialContextProvider is throwing an exception:

java.lang.RuntimeException: All Providers for type interface javax.naming.Context returned a null value: [org.jboss.arquillian.container.test.impl.enricher.resource.InitialContextProvider@5792c08c]
	at org.jboss.arquillian.test.impl.enricher.resource.ArquillianResourceTestEnricher.lookup(ArquillianResourceTestEnricher.java:126)
	at org.jboss.arquillian.test.impl.enricher.resource.ArquillianResourceTestEnricher.resolve(ArquillianResourceTestEnricher.java:99)
	at org.jboss.arquillian.junit5.MethodParameterObserver.injectParameters(MethodParameterObserver.java:67)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)
	at org.jboss.arquillian.core.impl.ObserverImpl.invoke(ObserverImpl.java:86)
	at org.jboss.arquillian.core.impl.EventContextImpl.invokeObservers(EventContextImpl.java:103)
	at org.jboss.arquillian.core.impl.EventContextImpl.proceed(EventContextImpl.java:90)
	at org.jboss.arquillian.container.test.impl.client.ContainerEventController.createContext(ContainerEventController.java:128)
	at org.jboss.arquillian.container.test.impl.client.ContainerEventController.createBeforeContext(ContainerEventController.java:114)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)
	at org.jboss.arquillian.core.impl.ObserverImpl.invoke(ObserverImpl.java:86)
	at org.jboss.arquillian.core.impl.EventContextImpl.proceed(EventContextImpl.java:95)
	at org.jboss.arquillian.test.impl.TestContextHandler.createTestContext(TestContextHandler.java:116)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)
	at org.jboss.arquillian.core.impl.ObserverImpl.invoke(ObserverImpl.java:86)
	at org.jboss.arquillian.core.impl.EventContextImpl.proceed(EventContextImpl.java:95)
	at org.jboss.arquillian.test.impl.TestContextHandler.createClassContext(TestContextHandler.java:83)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)
	at org.jboss.arquillian.core.impl.ObserverImpl.invoke(ObserverImpl.java:86)
	at org.jboss.arquillian.core.impl.EventContextImpl.proceed(EventContextImpl.java:95)
	at org.jboss.arquillian.test.impl.TestContextHandler.createSuiteContext(TestContextHandler.java:69)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)
	at org.jboss.arquillian.core.impl.ObserverImpl.invoke(ObserverImpl.java:86)
	at org.jboss.arquillian.core.impl.EventContextImpl.proceed(EventContextImpl.java:95)
	at org.jboss.arquillian.core.impl.ManagerImpl.fire(ManagerImpl.java:134)
	at org.jboss.arquillian.core.impl.ManagerImpl.fire(ManagerImpl.java:106)
	at org.jboss.arquillian.test.impl.EventTestRunnerAdaptor.before(EventTestRunnerAdaptor.java:115)
	at org.jboss.arquillian.junit5.ArquillianExtension.beforeEach(ArquillianExtension.java:63)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

@jamezp
Copy link
Collaborator Author

jamezp commented Aug 12, 2024

Yeah, I'm going to have a look at that now. It's interesting it passes in WildFly, but not Payara. I'll look to see if they override the InitialContext provider or if I'm just missing a test dependency or something simple like that.

@jamezp
Copy link
Collaborator Author

jamezp commented Aug 12, 2024

Maybe we should just ignore this test on Payara TBH. In Wildfly the context is initialized in the CommonDeployableContainer. In Payara it's done via the org.jboss.arquillian.testenricher:arquillian-testenricher-initialcontext is is only observed in a remote container. What is happening is when JUnit 5 tries to resolve the parameter, it's not happening in the container and therefore the InitialContext is never set.

I don't really see this as a huge deal because there are much better ways to lookup objects these days without the javax.naming.Context.

…ara. There is an issue with where/when the InitialContext producer is initialized for JUnit 5 ParameterResolvers.

Signed-off-by: James R. Perkins <[email protected]>
@jamezp jamezp marked this pull request as ready for review August 12, 2024 21:10
@Documented
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.PARAMETER)
public @interface Vetoed {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about DelegateToJunit, JunitParam or something that indicates it is relying on a Junit ParameterResolver

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not a bad idea. Just a way to say "don't let Arquillian attempt to resolve this". I think that makes a lot of sense.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I filed #610 as a follow for this.

@starksm64 starksm64 merged commit ff8a556 into arquillian:main Aug 13, 2024
16 checks passed
@jamezp jamezp deleted the issue312 branch August 13, 2024 00:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JUnit 5 Method Parameter Injection
2 participants