-
Notifications
You must be signed in to change notification settings - Fork 14
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
Skipped assertions in Jetty 12 tests #197
Comments
That comes from using the wrong (not recommended) initialization of weld with Jetty 12. Specifically the environment is using either a Fallback or DecoratingListener approach to initialize Weld. There are many ways to initalize Weld with Jetty. Take this example from Jetty 11.0.x The only one that should be used is the one labelled as If you look at that same example, but for Jetty 12, you'll see that options for |
So you're basically saying that you can no longer use In that case that looks like a fault in the container test setup, namely in the web.xml file - https://github.com/arquillian/arquillian-container-jetty/blob/master/jetty-embedded-12-ee10/src/test/resources/in-container-web.xml#L21 Either way, it means that part of the |
Use the following code as an example of what the Weld folks recommend. // Jetty setup telling Jetty's CdiDecoratingListener how to operate.
context.setInitParameter(CdiServletContainerInitializer.CDI_INTEGRATION_ATTRIBUTE, CdiDecoratingListener.MODE);
// jetty setup for layer between Weld and Jetty.
context.addServletContainerInitializer(new CdiServletContainerInitializer());
// weld setup, the Weld recommended way to use initialize weld. (this is supposed to be true for all containers)
context.addServletContainerInitializer(new org.jboss.weld.environment.servlet.EnhancedListener()); Note: the ee11 changes to CDI are going to change things again. |
I am a Weld folk and I am just trying to figure out where we stand with servlet integration and what needs to be updated :) @joakime based on what you said, I think the Arq. adapter should then look like this -> #198 That's passing with zero changes to Weld itself. We could also update Weld's
If so, then the integration can be simplified like so -> weld/core#2959
How exactly? |
Yes I agree with the changes in #198 and the proposed removal of the legacy bits.
The Lets look at the example from Jetty 9.4.x // Enable Weld + CDI
context.setInitParameter(CdiServletContainerInitializer.CDI_INTEGRATION_ATTRIBUTE, CdiDecoratingListener.MODE);
context.addBean(new ServletContextHandler.Initializer(context, new CdiServletContainerInitializer()));
context.addBean(new ServletContextHandler.Initializer(context, new org.jboss.weld.environment.servlet.EnhancedListener())); And also in Jetty 12 (ee-10) // Enable Weld + CDI
context.setInitParameter(CdiServletContainerInitializer.CDI_INTEGRATION_ATTRIBUTE, CdiDecoratingListener.MODE);
context.addServletContainerInitializer(new CdiServletContainerInitializer());
context.addServletContainerInitializer(new org.jboss.weld.environment.servlet.EnhancedListener()); Just using that technique is compatible across many major versions of Jetty. As for removing the Weld side legacy features, that seems very similar to the past issues. We have no issues with that.
There has been a slow process to push some of the responsibilities of the CDI implementations (weld, owb, etc) onto the various containers (servlet, websocket, etc). See thread "[servlet-dev] New home for HttpServletRequest injection requirements" in https://www.eclipse.org/lists/servlet-dev/msg00638.html While the changes in ee11 are minor, they are impacting Jetty as we have more and more tasks down in Jetty that we need to do for CDI. |
That seems to have gotten there as part of weld/core@c5b20b0
If those changes need to be reflected in Weld integration code please do reach out so that we can fix that :)
I see, so essentially the same as what you have in full blown app servers. Thanks for the information and your feedback. |
I noticed that tests for Jetty 12 adapter have Weld-related exclusions such as this one - https://github.com/arquillian/arquillian-container-jetty/blob/master/jetty-embedded-12-ee10/src/test/java/org/jboss/arquillian/container/jetty/embedded_12_ee10/JettyEmbeddedInContainerTestCase.java#L119-L121
This should be because you are using empty
beans.xml
which in CDI 4.0+ translates to discovery modeannotated
and only beans with bean defining annotations will be discovered.In other words, if you add
@Dependent
annotation onto your bean class (here) it will be discovered. Alternatively, you could providebeans.xml
which explicitly declaresall
discovery mode. Both approaches should get you to previous behavior.That being said, doing so gave me a new stack that's not Weld related and that someone here (@joakime or @olamy) might know more about:
The text was updated successfully, but these errors were encountered: