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.

Sign up for GitHub

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

Jump to bottom

Problem running invoker tests that expect a failure inside WildFly #558

Closed
starksm64 opened this issue Feb 29, 2024 · 12 comments · Fixed by #556
Closed

Problem running invoker tests that expect a failure inside WildFly #558

starksm64 opened this issue Feb 29, 2024 · 12 comments · Fixed by #556
Assignees
@starksm64
Milestone
CDI 4.1

Comments

@starksm64
Copy link
Contributor

I added the ability to run the core tests inside wildfly with the updated weld jars, and I'm seeing the new invoker tests that expect a failure not generating a failure when run inside of wildfly. The log was showing:

2024-02-28 22:23:34,931 WARN  [org.jboss.weld.deployer] (MSC service thread 1-1) WFLYWELD0007: Could not load portable extension class org.jboss.cdi.tck.tests.invokers.invalid.InterceptorInvokerTest$TestExtension: java.lang.ClassNotFoundException: org.jboss.cdi.tck.tests.invokers.invalid.InterceptorInvokerTest$TestExtension from [Module "deployment.InterceptorInvokerTest9f51e3b08633d3c9a9e67467c893989d5bedfe9.war" from Service Module Loader]
        at org.jboss.modules.ModuleClassLoader.findClass(ModuleClassLoader.java:200)
        at org.jboss.modules.ConcurrentClassLoader.performLoadClassUnchecked(ConcurrentClassLoader.java:410)
        at org.jboss.modules.ConcurrentClassLoader.performLoadClass(ConcurrentClassLoader.java:398)
        at org.jboss.modules.ConcurrentClassLoader.loadClass(ConcurrentClassLoader.java:116)
        at org.jboss.as.weld@31.0.0.Final//org.jboss.as.weld.deployment.processors.WeldPortableExtensionProcessor.loadExtension(WeldPortableExtensionProcessor.java:140)
        at org.jboss.as.weld@31.0.0.Final//org.jboss.as.weld.deployment.processors.WeldPortableExtensionProcessor.loadExtensions(WeldPortableExtensionProcessor.java:94)
        at org.jboss.as.weld@31.0.0.Final//org.jboss.as.weld.deployment.processors.WeldPortableExtensionProcessor.loadAttachments(WeldPortableExtensionProcessor.java:80)
        at org.jboss.as.weld@31.0.0.Final//org.jboss.as.weld.deployment.processors.WeldPortableExtensionProcessor.deploy(WeldPortableExtensionProcessor.java:59)
        at org.jboss.as.server@23.0.1.Final//org.jboss.as.server.deployment.DeploymentUnitPhaseService.start(DeploymentUnitPhaseService.java:172)
...

So looking at the tests, they were not adding the BuildCompatibleExtension to the test archive, so I fixed that, but then the deployments failed to generate an error due to:

2024-02-28 22:27:27,194 INFO  [org.jboss.weld.Bootstrap] (Weld Thread Pool -- 1) WELD-000119: Not generating any bean definitions from org.jboss.cdi.tck.tests.invokers.invalid.InterceptorInvokerTest$MyInterceptor because of underlying class loading error: Type org.jboss.cdi.tck.tests.invokers.invalid.InterceptorInvokerTest from [Module "deployment.InterceptorInvokerTest9f51e3b08633d3c9a9e67467c893989d5bedfe9.war" from Service Module Loader] not found.  If this is unexpected, enable DEBUG logging to see the full error.
2024-02-28 22:27:27,361 INFO  [org.wildfly.extension.undertow] (ServerService Thread Pool -- 32) WFLYUT0021: Registered web context: '/InterceptorInvokerTest9f51e3b08633d3c9a9e67467c893989d5bedfe9' for server 'default-server'

All of the inner classes used by the InterceptorInvokerTest are static, so I'm not clear on why there is an attempt to load the InterceptorInvokerTest class. Maybe something I'm not familiar with regarding inner classes. It does seem as though weld should be generating an exception here rather than log logging an error. I'll create a separate issue for that.

I have just refactored the tests to not use inner classes and I have all core tck tests passing with embedded weld as well as when run from within wildfly.
@starksm64 starksm64 self-assigned this Feb 29, 2024
@starksm64 starksm64 added this to the CDI 4.1 milestone Feb 29, 2024
@starksm64 starksm64 mentioned this issue Feb 29, 2024
Merged
starksm64 added a commit that referenced this issue Feb 29, 2024
@starksm64
Remove the web tck tests (#556)
692016e 
* Remove the web tck tests
* Restore ability to run the core tck tests inside of wildfly
* Fix a problem with invoker tests that expected failures when running inside of wildfly, fixes #558
* Update to 4.1.0.Beta2 CDI API

---------

Signed-off-by: Scott M Stark <starksm64@gmail.com>
@Ladicek
Copy link
Contributor

Ladicek commented Feb 29, 2024

I see that most of the invokers test don't add the BCExtension into the test archive, which is probably a mistake.

The test class should be in the archive, so even though I'm not sure why Weld would try to load it, it should work 🤷

@manovotn
Copy link
Contributor

I see that most of the invokers test don't add the BCExtension into the test archive, which is probably a mistake.

The test class should be in the archive, so even though I'm not sure why Weld would try to load it, it should work 🤷

Seems like Weld will try to make sure each declaring class is loadable in this code.
Though I cannot understand why the test class class isn't 🤔

@manovotn
Copy link
Contributor

Ok, so when I look at the resulting (WAR) archive Arq. generates, the test class is not part of it, despite being explicitly added in the test.

And looking into how we build test archive, I found that this bit will exclude the class if it is running in client mode and the class name equals to the test name. And while I don't recall what the client mode really was (but apparently we are running in it), this seems very deliberate :)
Maybe this rings a bell @Ladicek or @starksm64?

@Ladicek
Copy link
Contributor

Ladicek commented Feb 29, 2024

We apparently treat tests that are expected to fail as "run as client" due to https://github.com/jakartaee/cdi-tck/blob/master/impl/src/main/java/org/jboss/cdi/tck/shrinkwrap/ArchiveBuilder.java#L1344 Frankly, that's rather weird...

@starksm64
Copy link
Contributor Author

starksm64 commented Feb 29, 2024 via email

@manovotn
Copy link
Contributor

Caused by: java.lang.NoClassDefFoundError:
jakarta/enterprise/inject/build/compatible/spi/InvokerFactory

@starksm64 does your WFLY have latest CDI API in it?
I had a patching profile in core repo for this (that CI uses as well), see https://github.com/weld/core/blob/master/jboss-as/pom.xml#L214-L217

@starksm64
Copy link
Contributor Author

starksm64 commented Feb 29, 2024 via email

@manovotn
Copy link
Contributor

Basically all of the invoker tests appear to be failing with a CNFE on jakarta.enterprise.inject.build.compatible.spi.InvokerFactory, so the wildfly modules must not be updated correctly or something is missing.

Yes, see my previous comment :)
#558 (comment)

@starksm64
Copy link
Contributor Author

starksm64 commented Feb 29, 2024 via email

@manovotn
Copy link
Contributor

@starksm64 Yes, I was able to reproduce immediately even without the TCK dist-build.
After some more digging, the @ShouldThrowException marks the deployment as testable=false by default.
This is what triggers the TCK behavior as well and why Weld fails.

I do not think this is something we can fix on Weld side - it seems that tests expecting a failure should be running as testable=false and as such will need to avoid using nested classes.

@manovotn
Copy link
Contributor

As captured in the related Weld issue, I don't think this really is a Weld issue.
Instead it seems that tests expecting an exception are by design testable=false leading to different deployment creation missing certain classes - test class included.

And that's a problem when analyzing some of the nested static classes as Weld (or any other impl really) will at some point need to verify that they are valid beans - i.e. that they are top level classes or nested static classes. This eventually leads to Class#getEnclosingClass() (with reflection) and to CNFE.

@starksm64
Copy link
Contributor Author

Ok, I'll move that test back to not using nested classes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@starksm64 starksm64

Labels
None yet
Projects
None yet
Milestone
CDI 4.1
Development

Successfully merging a pull request may close this issue.

Remove the web tck tests
3 participants
@starksm64 @Ladicek @manovotn