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

CDI Test failures with GlassFish 7 #23961

Closed
11 tasks done
arjantijms opened this issue May 31, 2022 · 16 comments · Fixed by #23971, #23978, #23970, #23976 or #23981
Closed
11 tasks done

CDI Test failures with GlassFish 7 #23961

arjantijms opened this issue May 31, 2022 · 16 comments · Fixed by #23971, #23978, #23970, #23976 or #23981
Labels
ee10-tck EE 10 TCK failures

Comments

@arjantijms
Copy link
Contributor

arjantijms commented May 31, 2022

Running the CDI TCK (using the runner in GlassFish) there are 11 failures as of today:

[ERROR] Failures:

  • EjbInjectionTest>Arquillian.run:138->testPassivationOfEjbs:70 » EJB java.rmi.RemoteException: CORBA BAD_OPERATION 0 No; nested exception is:
    org.omg.CORBA.BAD_OPERATION: The delegate has not been set! vmcid: 0x0 minor code: 0 completed: No
  • AmbiguousInjectionIntoNonContextualComponentTest>Arquillian.arquillianBeforeClass:99 » Runtime Expected exception of type jakarta.enterprise.inject.spi.DeploymentException during deployment of DEFAULT, but no exception was thrown.
  • UnsatisfiedInjectionIntoNonContextualComponentTest>Arquillian.arquillianBeforeClass:99 » Runtime Expected exception of type jakarta.enterprise.inject.spi.DeploymentException during deployment of DEFAULT, but no exception was thrown.
  • InjectionPointTest>Arquillian.arquillianBeforeClass:99 » Runtime Expected exception of type jakarta.enterprise.inject.spi.DefinitionException during deployment of DEFAULT, but no exception was thrown.
  • CDIProviderInitTest>Arquillian.run:138->testAccessingBeanManager:103 expected [0] but found [1]
  • CDIProviderRuntimeTest>Arquillian.run:138->testGetBeanManager:76 expected [1] but found [0]
  • CDIProviderRuntimeTest>Arquillian.run:138->testLookup:90 » UnsatisfiedResolution WELD-001334: Unsatisfied dependencies for type Bravo with qualifiers

Fixed by Weld update:

  • ContainerLifeCycleEventRuntimeInvocationTest>Arquillian.run:138->testAfterBeanDiscoveryEventFails:224 » NullPointer
  • ContainerLifeCycleEventRuntimeInvocationTest>Arquillian.run:138->testAfterBeanDiscoveryEventFails:224 » NullPointer

See jakartaee/cdi#624 (comment)

Fixed by CDI TCK update:

  • PersistenceContextInjectionTest>Arquillian.run:138->testBeanTypesAndBindingTypesOfPersistenceContext:105
  • PersistenceContextInjectionTest>Arquillian.run:138->testBeanTypesOfPersistenceUnit:116

See jakartaee/cdi-tck#380

[ERROR] Tests run: 1837, Failures: 11, Errors: 0, Skipped: 6

Tests can be run from [glassfish]/appserver/tests/tck/cdi using:

mvn clean install

Individual tests can be run via

mvn clean install -Dit.test=ExcludeFiltersTest#testExcludeSystemPropertyActivator

Tests can be debugged via:

mvn clean install -Dit.test=ExcludeFiltersTest#testExcludeSystemPropertyActivator -Dglassfish.suspend

@gurunrao gurunrao added the ee10-tck EE 10 TCK failures label Jun 1, 2022
@dmatej dmatej reopened this Jun 7, 2022
@dmatej dmatej pinned this issue Jun 7, 2022
@dmatej
Copy link
Contributor

dmatej commented Jun 7, 2022

https://docs.google.com/spreadsheets/d/18BARERS-smPDv6vfK-73k5Gy3OrA2mUvBamwL7iGA3Q/edit#gid=1305635927
Yet 13 tests.

[ERROR]   ContainerEventTest>Arquillian.run:138->testProcessInjectionTargetEventFiredForMessageDrivenBean:75 expected object to not be null
[ERROR]   ContainerLifeCycleEventRuntimeInvocationTest>Arquillian.run:138->testAfterBeanDiscoveryEventFails:224 » NullPointer
[ERROR]   ContainerLifeCycleEventRuntimeInvocationTest>Arquillian.run:138->testAfterBeanDiscoveryEventFails:224 » NullPointer

[ERROR]   ExcludeFiltersTest>Arquillian.run:138->testExcludeSystemPropertyActivator:107->assertTypeIsExcluded:112 expected [true] but found [false]
[ERROR]   EjbInjectionTest>Arquillian.run:138->testPassivationOfEjbs:70 » EJB java.rmi.RemoteException: CORBA BAD_OPERATION 0 No; nested exception is: org.omg.CORBA.BAD_OPERATION: The delegate has not been set!  vmcid: 0x0  minor code: 0  completed: No

[ERROR]   PersistenceContextInjectionTest>Arquillian.run:138->testBeanTypesAndBindingTypesOfPersistenceContext:105
[ERROR]   PersistenceContextInjectionTest>Arquillian.run:138->testBeanTypesOfPersistenceUnit:116

[ERROR]   AmbiguousInjectionIntoNonContextualComponentTest>Arquillian.arquillianBeforeClass:99 » Runtime Expected exception of type jakarta.enterprise.inject.spi.DeploymentException during deployment of _DEFAULT_, but no exception was thrown.
[ERROR]   UnsatisfiedInjectionIntoNonContextualComponentTest>Arquillian.arquillianBeforeClass:99 » Runtime Expected exception of type jakarta.enterprise.inject.spi.DeploymentException during deployment of _DEFAULT_, but no exception was thrown.
[ERROR]   InjectionPointTest>Arquillian.arquillianBeforeClass:99 » Runtime Expected exception of type jakarta.enterprise.inject.spi.DefinitionException during deployment of _DEFAULT_, but no exception was thrown.

[ERROR]   CDIProviderInitTest>Arquillian.run:138->testAccessingBeanManager:102 expected [0] but found [1]
[ERROR]   CDIProviderRuntimeTest>Arquillian.run:138->testGetBeanManager:76 expected [1] but found [0]
[ERROR]   CDIProviderRuntimeTest>Arquillian.run:138->testLookup:90 » UnsatisfiedResolution WELD-001334: Unsatisfied dependencies for type Bravo with qualifiers

@manovotn
Copy link

manovotn commented Jun 8, 2022

ContainerLifeCycleEventRuntimeInvocationTest

This was Weld failure indeed, 5.0.0.SP2 is syncing with Central now and it should fix the issue.

PersistenceContextInjectionTest

This isn't Weld related, it's the TCKs asserting bean types of EntityManager and not counting on a JPA change that added AutoCloseable as their type. You'll need new TCK version for that (which I think Scott works on already), the fix for it was already merged.

@starksm64
Copy link
Member

CDI TCK 4.0.4 is promoted:
https://download.eclipse.org/jakartaee/cdi/4.0/cdi-tck-4.0.4-dist.zip

@dmatej
Copy link
Contributor

dmatej commented Jun 8, 2022

6 remaining failures :)

[ERROR] Failures:
[ERROR]   AmbiguousInjectionIntoNonContextualComponentTest>Arquillian.arquillianBeforeClass:99 » Runtime Expected exception of type jakarta.enterprise.inject.spi.DeploymentException during deployment of _DEFAULT_, but no exception was thrown.
[ERROR]   UnsatisfiedInjectionIntoNonContextualComponentTest>Arquillian.arquillianBeforeClass:99 » Runtime Expected exception of type jakarta.enterprise.inject.spi.DeploymentException during deployment of _DEFAULT_, but no exception was thrown.
[ERROR]   InjectionPointTest>Arquillian.arquillianBeforeClass:99 » Runtime Expected exception of type jakarta.enterprise.inject.spi.DefinitionException during deployment of _DEFAULT_, but no exception was thrown.
[ERROR]   CDIProviderInitTest>Arquillian.run:138->testAccessingBeanManager:102 expected [0] but found [1]
[ERROR]   CDIProviderRuntimeTest>Arquillian.run:138->testGetBeanManager:76 expected [1] but found [0]
[ERROR]   CDIProviderRuntimeTest>Arquillian.run:138->testLookup:90 » UnsatisfiedResolution WELD-001334: Unsatisfied dependencies for type Bravo with qualifiers
[INFO]

@dmatej
Copy link
Contributor

dmatej commented Jun 8, 2022

I have another bug in Weld, probably - I am able to hack and workaround it in GlassFish, but it would be quite nasty. It relates to InjectionPointTest, but maybe affects others too, I did not try it yet. Here InjectionTargetFactory catches DefinitionException (expected in the test) and wraps it into the IllegalArgumentException. Then GlassFish's StandardWrapper then wraps it to ServletException, ServletContext just logs it and the test fails. All those wrappings are not necessary and could be simply avoided (and logging), then the test would pass.

https://github.com/weld/core/blob/be7382b01c4a56c54f92873c1c2ebf0445714bfe/impl/src/main/java/org/jboss/weld/manager/InjectionTargetFactoryImpl.java#L83

Nasty hack I tried and test passed - I'm unwrapping the cause until I find the DefinitionException in causes, then just rethrowing it.

EDIT: Absolutely the same applies to AmbiguousInjectionIntoNonContextualComponentTest and DeploymentException.

@starksm64
Copy link
Member

You will have to install an org.jboss.arquillian.container.spi.client.container.DeploymentExceptionTransformer that unwraps the exceptions and register it via a org.jboss.arquillian.core.spi.LoadableExtension. This is an example from the WildFly cdi tck runner in the core profile:

https://github.com/jakartaredhat/wf-core-tck-runner/blob/main/cdi-tck/src/main/java/org/jboss/wildfly/tck/cdi/WildFlyExceptionTransformer.java
https://github.com/jakartaredhat/wf-core-tck-runner/blob/main/cdi-tck/src/main/java/org/jboss/wildfly/tck/cdi/WildFlyExtension.java

@dmatej
Copy link
Contributor

dmatej commented Jun 9, 2022

There already is such extension, I can change it, but ... that is rather hacking the test, because then if the specification expects DeploymentException and DefinitionException, in fact it is not thrown neither from GlassFish server nor from Weld, it is just a cause deep inside the stacktrace. The real-world user then cannot expect that. Is it really the correct solution for this?

@manovotn
Copy link

manovotn commented Jun 9, 2022

Quickly looking at the code, the IAE is (probably) thrown because the method, and it's validation, are used from several different places in which definition exception isn't required or doesn't make much sense.

This is an example from the WildFly cdi tck runner in the core profile:

Yea, same thing is in Weld repo for TCK execution.

The real-world user then cannot expect that. Is it really the correct solution for this?

This is an error case. The hypothetical real-world user won't intend to write a bean that blows up their application. Instead, they just need to get a sensible message for why it failed. Whether that's straight up definition exception or wrapped in IAE doesn't matter that much IMO.
So while we could look into why the IAE is actually there and try to improve it, I don't think it's anyhow blocking.

@dmatej
Copy link
Contributor

dmatej commented Jun 9, 2022

It is an error case tested by CDI TCK we try to hack.
Also any impl using Weld must use similar hack to pass the CDI TCK. That smells bad. Nevermind, I will try to fix that directly in GlassFish, seems the fix doesn't break any other TCK tests, so at least we would see if there is some controversion in tests/specs.
It might be important for embedded servers, but the only used on production I know is Jetty (and recently we fixed embedded GF).

The message is alright, but that is not tested by the CDI TCK.

@dmatej
Copy link
Contributor

dmatej commented Jun 9, 2022

I found yet another "unwrapping" solution already present in GlassFish. So you can take this discussion just as something to consider, but there is no acute action required from Weld team.

@tnagao7
Copy link
Member

tnagao7 commented Jun 10, 2022

CDIProviderRuntimeTest

The beans.xml file in a JAR file generated by CDIProviderRuntimeTest is different by TCK versions as follows. This might be the cause.

In cdi-tck 3.0, bean-discovery-mode="all" is specified:

<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<beans xmlns="http://xmlns.jcp.org/xml/ns/javaee" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" bean-discovery-mode="all" version="1.1" xsi:schemaLocation="http://xmlns.jcp.org/xml/ns/javaee http://xmlns.jcp.org/xml/ns/javaee/beans_1_1.xsd">
  <alternatives>
    <class>org.jboss.cdi.tck.tests.lookup.manager.provider.runtime.Bravo</class>
  </alternatives>
</beans>

In cdi-tck 4.0.4, bean-discovery-mode="annotated" is specified:

<beans version="3.0" bean-discovery-mode="annotated">
<alternatives>
<class>org.jboss.cdi.tck.tests.lookup.manager.provider.runtime.Bravo</class>
</alternatives>
</beans>

EDIT: In "annotated" bean discovery mode, BravoLocator is not regarded as bean. This causes that a different bean manager is returned by getBeanManager().

@tnagao7
Copy link
Member

tnagao7 commented Jun 13, 2022

CDIProviderInitTest

A possible cause is that the root BDA (RootBeanDeploymentArchive) has a reference to beans.xml in WAR.
Since a class implementing Marker is written in beans.xml, the bean manager for root BDA returns an instance, resulting in assertion failure of this line:

        assertEquals(MarkerObtainerNonBda.getBeans(Marker.class).size(), 0);

Removing the reference to beans.xml from the root BDA (that is, reverting #23944) will resolve the failure of CDIProviderInitTest, though it will lead to other tests (e.g. EnterpriseInterceptorBindingResolutionTest) failing again...

@tnagao7
Copy link
Member

tnagao7 commented Jun 13, 2022

In failed tests CDIProviderRuntimeTest and EnterpriseInterceptorBindingResolutionTest, it seems that an inappropriate bean manager is returned by the ClassNameToBeanManager.findBeanManager method.
The bean manager returned by the method is decided by whether its caller class is recognized in BDAs, but that decision seems to be currently incorrect.

Either of the following options may resolve failures of these two tests:

Option 1 fixes TCK to generate beans.xml that is semantically equivalent to CDI 3.0 TCK.
In PR jakartaee/cdi-tck#224, a semantically incompatible change to beans.xml is introduced: the bean-discovery-mode attribute in beans.xml generated by TCK was changed from bean-discovery-mode="all" to bean-discovery-mode="annotated" (as mentioned here).
This causes some classes to become unrecognized in BDAs, and thus an inappropriate bean manager is returned.
If this change of attribute is unintentional, TCK should be fixed.

Option 2 fixes GlassFish to override the BeanDeploymentArchive.getKnownClasses method, which is called from ClassNameToBeanManager.findBeanManager.
Since the method is not overridden in BeanDeploymentArchiveImpl, the getKnownClasses method returns only bean classes instead of all classes in BDA.
That is likely causing the ClassNameToBeanManager.findBeanManager to return an inappropriate bean manager.

@arjantijms
Copy link
Contributor Author

In failed tests CDIProviderRuntimeTest and EnterpriseInterceptorBindingResolutionTest, it seems that an inappropriate bean manager is returned by the ClassNameToBeanManager.findBeanManager method.

That might be a root cause for some of the other failures we saw, and which needed somewhat elaborate workarounds to pass. See e.g. this one: #23965

The tests gets a bean manager from the root archive, and we needed to do elaborate wrapping to point it back to the right archive.

@manovotn
Copy link

The bean manager returned by the method is decided by whether its caller class is recognized in BDAs, but that decision seems to be currently incorrect.

@tnagao7 those are good finds!
From Weld PoV, the name of the class is being searched in BeanDeploymentArchive.getKnownClasses() which is a collection of all known classes in given archive, not just beans. So the fact that the class isn't annotated and picked up as bean should not interfere with this method so long as GF correctly registers all known classes for each BeanDeploymentArchive.
In other words, Option 2 is what you should do to fix that and it seems it might indeed be cause of your other failures as well.

@dmatej dmatej linked a pull request Jun 14, 2022 that will close this issue
@arjantijms
Copy link
Contributor Author

All the CDI TCK tests are passing! :)

===============================================
CDI TCK
Total tests run: 1831, Passes: 1831, Failures: 0, Skips: 0
===============================================

@dmatej dmatej unpinned this issue Oct 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment