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

[junit5] Injecting an @ArquillianResource which depends on the container being started results in the resource not being found #661

Open
jamezp opened this issue Jan 11, 2025 · 4 comments

Comments

@jamezp
Copy link
Collaborator

jamezp commented Jan 11, 2025

Issue Overview

Given a manual mode test where a @BeforeEach or potentially a @BeforeAll method is responsible for starting the container, method parameters resolved for @ArquillianResource methods might not be available.

The issue seems to be the TestRunnerAdaptor.before() is fired in the beforeEach() of the ArquillianExtension. However, the test method seems to already be resolved and there is an attempt to lookup the method parameters before the container is started.

An example test looks like:

@ArquillianTest
public class ManualModeTest {

    @ArquillianResource
    private ContainerController controller;

    @BeforeEach
    public void start() {
        if (!controller.isStarted("default")) {
            conroller.start("default");
        }
    }

    @Test
    public void inject(@ArquillianResource URL url) {
    }
}

Given the above method the test with throw an exception like:

java.lang.RuntimeException: All Providers for type class java.net.URL returned a null value: [org.jboss.arquillian.container.test.impl.enricher.resource.URLResourceProvider@6d514259]
	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:580)
	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:580)
	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:580)
	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:580)
	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:580)
	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:1596)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)

I need to write a test/reproducer still. I've been able to reproduce it in WildFly and did find a fix for the specific WildFly issue. However, the fix seems to break other cases.

What is likely needed is the ArquillianExtension should implement the BeforeTestExecutionCallback and that's where the parameters should be set for test methods. Observing the Before event is likely too soon for test methods, but might be needed for @BeforeEach methods.

The issue I'm currently seeing when implementing the BeforeTestExecutionCallback is the URL is not available in the the active Arquillian contexts. This seems to be resolved from the ProtocolMetaData which is not available in the TestScoped context. The ProtocolMetaData is set after a deployment is done as it comes from the DeployableContainer.deploy() method.

This follows up on #312.

@starksm64
Copy link
Member

As part of the 2.0 work I really want to look at switching to a static, deterministic set of lifecycle events and look into issues around how early the arquillian.xml descriptor is parsed. This would mean replacing the EventTestRunnerAdaptor with some ContractTestRunnerAdaptor. We should also look at all of the available Junit5 callbacks to see if there are other we should integrate with as well as how testcontainers integrates.

In terms of the current issue, I remember I had to add more information to the contain/protocol metadata to have access to it in one of the test artifact callbacks. We still have this issue(#587) about how the contract between the DeployableContainer and Protocol is not clean, and this is relevant to this issue as well because the issue is about wanting access to the ProtocolMetaData without having to call the DeployableContainer#deploy(Archive) method.

So @jamezp , what does wildfly need to be able to generate the URL for a deployment? What I'm suggesting in the #587 issue is that we have a new method on either the Protocol interface, or something like a new ProtocolContainerAdapter interface that supports:

interface ProtocolContainerAdapter {
  ProtocolMetaData prepareDeployment(ContainerConfiguration cc, ProtocolConfiguration pc, Archive testArchive); 
}

and this would be called during the DeploymentGenerator processing, which is part of the Junit5 BeforeAllCallback phase and the Arquillian GenerateDeployment event handling.

@jamezp
Copy link
Collaborator Author

jamezp commented Jan 13, 2025

I do think that #587 makes sense. In this specific case, it wouldn't work for the issue I'm seeing. I need to look at the JUnit documentation to understand why the callbacks are happening in the order I'm seeing them. Effectively though this is what I see.

The BeforeEachCallback.beforeEach() is being invoked on the test method not the method annotated with @BeforeEach. In the ArquillianExtension this attempts to lookup the parameters for an @ArquillianResource parameter, but the @BeforeEach method in the test has not yet been invoked. Therefore, the container hasn't even been started yet.

The simple fix seemed to be to change where the parameters were set by using the BeforeTestExecutionCallback.beforeTestExecution(). This ensures the @BeforeEach method has been invoked. However, this seemed to break an existing test where the container was managed. What I observed was I was missing an Arquillian Context which was able to provide the ProtocolMetaData which seems to be associated with the @DeploymentScoped. In the BeforeTestExecutionCallback.beforeTestExecution() the only scopes activated seemed to be ApplicationScoped, SuiteScoped, ClassScoped and TestScoped.

That said, it does seem the ProtocolMetaData should really be available in at least one of those scopes. Probably TestScoped as it's the most narrow and setup methods should be invoked before then.

@jamezp
Copy link
Collaborator Author

jamezp commented Jan 13, 2025

I've got a fix now which may or may not be correct. I'm going to run the full clustering test suite in WildFly first before I make any more changes. Then I'll see if I can write a test for this.

@jamezp
Copy link
Collaborator Author

jamezp commented Jan 14, 2025

For clarification the BeforeEachCallback.beforeEach() explicit states:

BeforeEachCallback defines the API for Extensions that wish to provide additional behavior to tests before an individual test and any user-defined setup methods (e.g., @BeforeEach methods) for that test have been executed.

That is where we attempt to resolve parameters is in a org.jboss.arquillian.test.spi.event.suite.Before event observer. In most cases this works fine when Arquillian controls the lifecycle of the container. However, for manual mode tests where the test itself controls the lifecycle, the even is triggered before the @BeforeEach method is invoked.

The simple solution seems to be to use the BeforeTestExecutionCallback.beforeTestExecution() to set the parameters. However, we don't have the DeploymentContextImpl activated which is where the ProtocolMetaData is stored which is needed to resolve a URI or URL for @ArquillianResource injection.

It's not clear to me how to correctly activate a context or if that is correct. Potentially it could be done in the TestExension and ContainerTestExtension. However, that also might not be correct.

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

No branches or pull requests

2 participants