Standalone TCK shouldn't require CDI SE

[brideck]

Open Liberty would like to run this TCK against what we ship to ensure that we've pulled our JSON implementations into the product correctly. However, because of the TCK's dependency on an SE implementation of CDI, we can't currently do this.

Attempts to run in this fashion result in the following exception:

java.lang.NoClassDefFoundError: jakarta/enterprise/inject/se/SeContainer
        at java.base/java.lang.Class.getDeclaredFields0(Native Method)
        at java.base/java.lang.Class.privateGetDeclaredFields(Class.java:3297)
        at java.base/java.lang.Class.getDeclaredFields(Class.java:2371)
        at org.junit.platform.commons.util.ReflectionUtils.getDeclaredFields(ReflectionUtils.java:1392)
        at org.junit.platform.commons.util.ReflectionUtils.findAllFieldsInHierarchy(ReflectionUtils.java:1150)
        at org.junit.platform.commons.util.ReflectionUtils.findFields(ReflectionUtils.java:1138)
        at org.junit.platform.commons.util.AnnotationUtils.findAnnotatedFields(AnnotationUtils.java:371)
        at org.junit.platform.commons.util.AnnotationUtils.findAnnotatedFields(AnnotationUtils.java:348)
        at org.junit.jupiter.engine.descriptor.ExtensionUtils.registerExtensionsFromFields(ExtensionUtils.java:99)
        at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.prepare(ClassBasedTestDescriptor.java:148)
        at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.prepare(ClassBasedTestDescriptor.java:78)
        ...

There are two tests in the TCK (both found in https://github.com/eclipse-ee4j/jsonb-api/tree/master/tck/src/main/java/ee/jakarta/tck/json/bind/cdi/customizedmapping) that define fields of this type. Should there be tests that are verifying integration with other Jakarta EE specs in a standalone TCK that can't run in an EE environment?

[scottmarlow]

FYI https://www.eclipse.org/lists/jakartaee-platform-dev/msg03362.html

[gurunrao]

@brideck - We have observed two failures with JSONB test with Glassfish 7 Web Profile, are the two failure related to above failure?
Failure details:

java.lang.IllegalStateException: No valid CDI implementation found
	at jakarta.enterprise.inject.se.SeContainerInitializer.findSeContainerInitializer(SeContainerInitializer.java:108)
	at jakarta.enterprise.inject.se.SeContainerInitializer.newInstance(SeContainerInitializer.java:99)
	at ee.jakarta.tck.json.bind.cdi.customizedmapping.adapters.AdaptersCustomizationCDITest.startContainer(AdaptersCustomizationCDITest.java:59)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)

CI run https://ci.eclipse.org/jakartaee-tck/job/10/job/jakarta-jsonb-tck-glassfish-run/18/org.glassfish$glassfish.jsonb-tck/testReport/

[gurunrao]

Adding to above comment:
JSONB TCK tests pass FULL profile GF bundle, due to jar weld-se-shaded.jar in classpath - https://github.com/eclipse-ee4j/jakartaee-tck/blob/master/glassfish-runner/jsonb-tck/pom.xml#L90, same jar is missing from Web Profile GF 7 bundle.

[brideck]

Yes, this is exactly the sort of failure that I'm talking about. Whereas GF ships a weld-se implementation that it can add to the classpath, Open Liberty does not, and should not be required to -- EE is not a superset of SE.

[scottmarlow]

@lukasj this started as a question but implementations like GlassFish 7 Web Profile implementation do not include the weld-se-shaded.jar artifact which causes the TCK test to fail.

The same problem will happen for a Core Profile implementation that doesn't include CDI SE implementation (e.g. weld-se-shaded.jar). Please update this issue to make into a TCK challenge so we can start addressing it. Thank you!

[lukasj]

@scottmarlow I'm not a committer on this project, so there is really not much I can do.

@Verdent can you take a look?

[lukasj]

@scottmarlow btw I really do not think that maven is the best tool to run TCK tests against some ZIP binary. For that, I used gradle in parsson, where I can easily mix maven dependencies with local ones and alter all paths as necessary (see this if you're interested)

[scottmarlow]

@scottmarlow I'm not a committer on this project, so there is really not much I can do.

My bad, I should of looked at https://projects.eclipse.org/projects/ee4j.jsonb/who

@Verdent can you take a look?

David, could you please add the challenge label to make this an official TCK challenge. Thank you!

[Verdent]

@scottmarlow I do understand the problem, but JSONB spec also declares following
Implementations must provide a CDI support in serializers/deserializers to allow injection of CDI managed beans into it.
Because of this declaration, the TCK tests for CDI support should be in place from my point of view. CDI itself is optional for the impl run, but this should verify its support if it is present.

[brideck]

@scottmarlow I do understand the problem, but JSONB spec also declares following Implementations must provide a CDI support in serializers/deserializers to allow injection of CDI managed beans into it. Because of this declaration, the TCK tests for CDI support should be in place from my point of view. CDI itself is optional for the impl run, but this should verify its support if it is present.

This is what the jsonb/cdi tests that we left in the Platform TCK is designed to cover, too, right? If there's also a need to prove this function from an SE environment/perspective, then these tests could just be excluded from executions looking to supplement an EE certification effort. If CDI itself is optional, then there should also be a way to make the related tests in the TCK optional, no? Note: I fully understand that I could just pull in something like weld-se from Maven to make this all work, but should that be a requirement?

[lukasj]

If CDI itself is optional

let's get an answer to this question first. Does the spec say anywhere that CDI is optional or if CDI is available or similar thing to make it clear that CDI may not be available at runtime and when it is missing it is still fully compliant API/spec/impl combination?

[Verdent]

No, spec does not explicitly say, CDI is optional. Only that it supports CDI.

[scottmarlow]

From the Core Profile Specification https://github.com/eclipse-ee4j/jakartaee-platform/blob/master/specification/src/main/asciidoc/coreprofile/CoreProfileDefinition.adoc:

The Java SE section of the CDI 4.0 specification is not required for Core Profile implementations. Only Full CDI implementations are required to support the (CDI) Java SE API classes.

Also from the same Core Profile link, Jakarta JSON Binding 3.0 is a required component.

I think that Core Profile 10 implementations should have a way to pass the JSON-B 3.0 TCK without being required to implement jakarta.enterprise.inject.se.SeContainer.

As far as Jakarta EE 10 Full Platform implementations go though, I think they do need to provide the jakarta.enterprise.inject.se.SeContainer implementation as per guidance in https://jakarta.ee/specifications/cdi/4.0/jakarta-cdi-spec-4.0.html:

CDI implementations in Jakarta EE containers are required to support CDI Full.

[brideck]

As far as Jakarta EE 10 Full Platform implementations go though, I think they do need to provide the jakarta.enterprise.inject.se.SeContainer implementation as per guidance in https://jakarta.ee/specifications/cdi/4.0/jakarta-cdi-spec-4.0.html:

CDI implementations in Jakarta EE containers are required to support CDI Full.

The CDI specification also states:

CDI implementations that support the Java SE API are required to support CDI Full.

The wording here of "that support" implies that not every valid CDI implementation needs to support Java SE at all. I read all of the above to mean:

EE Core Profile = CDI Lite
EE = CDI Full
SE (if supported by implementation) = CDI Full

The statement in the draft Core Profile spec doc about Full CDI implementations being required to support SE seems like an overreach compared to what's in the CDI spec itself.

It would be useful if a CDI spec expert chimed in with what the intention was.

[starksm64]

@brideck is correct in that support for SE requires support for Full, but support for Full does not require SE. The CDI TCK userguide examples for running against WildFly have the following group exclusions:

Full Java EE Platform, excluded groups = se
Web Profile, excluded groups = javaee-full,se

So in both the Web Profile and Full Platform, Java SE related tests are excluded.

[brideck]

So in both the Web Profile and Full Platform, Java SE related tests are excluded.

I noticed that as well, but I also noticed that the 4.0 TCK user guide does not exclude the 'se' tests in section 4.5, showing how to run for Web Profile. Likewise, the user guide is completely silent on how to go about running for Full Platform (implying that you shouldn't need an exclude statement at all?).

It would seem that this is not a well-understood situation.

[starksm64]

Ok, I see. I have created this CDI TCK issue to address that:
jakartaee/cdi-tck#378

As I said in that issue, it really does not make sense for Full Platform/Web Profile implementations to imply anything about the Java SE bootstrap behavior because that is a completely separate startup behavior that cannot be configured using the same Arquillian container.

[starksm64]

The proposed userguide update PR is: jakartaee/cdi-tck#379

[Ladicek]

Hi,

copying my reply to cdi-dev here:

the structure of the CDI specification is as follows:

• CDI core (consists of CDI Lite and CDI Full, where Lite is a subset of Full)
• CDI SE
• CDI EE

Standalone implementations of CDI Lite indeed do not have to support CDI SE (and cannot, because of some methods in the SE API; we could define a subset of the CDI SE API that can be implemented under the Lite constraints, but didn't yet).

Implementations of CDI SE and CDI EE must support Full, as stated at the very beginning of the "CDI in Java SE" and "CDI in Jakarta EE" sections of the CDI specification.

Now, is it possible for standalone implementations of CDI Full to exist, without support of SE (or EE)? I don't think we ever considered that question, but I don't see anything preventing that. In any case, I don't think that matters in the JSON-B TCK issue. My understanding is that Jakarte Core Profile includes CDI Lite and not CDI Full, which implies that Jakarta Core Profile does not include CDI SE (and cannot, at the moment, as described above).

My recommendation for the JSON-B TCK would hence be: if you want tests to exercise some CDI SE or EE integration, those should be in separate groups that can be excluded. If you use the CDI SE API to bootstrap a CDI container in order to verify CDI Lite integration, that should most likely become a TCK SPI that integrators have to implement in any way they wish (because CDI Lite doesn't have a bootstrapping API).

[scottmarlow]

FYI the two tests being challenged are:

• ee.jakarta.tck.json.bind.cdi.customizedmapping.adapters.AdaptersCustomizationCDITest
• ee.jakarta.tck.json.bind.cdi.customizedmapping.serializers.SerializersCustomizationCDITest

Are we ready to accept that these tests should be excluded in a https://download.eclipse.org/jakartaee/jsonb/3.0/jakarta-jsonb-tck-3.0.1.zip release? If yes, please mark the challenge as accepted.

[Verdent]

I think removal is not correct in this case. These test are needed there for certification reasons etc. What could be done, is to flag them in JUnit, so they can be easily excluded out of the run if needed. Or do something similar, but I do not think removing it is the way to go.

[scottmarlow]

So the tests would default to be run but a JSON-B implementation certification request would still be accepted if their test results are missing these two (challenged) CDI SE tests.

[starksm64]

There needs to be a Junit CDI_SE group that can be excluded for certification of profiles/platform implementations that don't require the CDI Java SE support.

[rmannibucau]

While it is fine to run in standalone, it is a vendor subset of jsonb which requires cdi so test must pass with cdi but not use CDI SeContainer/Initializer since requirement is for its runtime not to bootstrap CDI.

Side note: CDI Lite is NOT required by JSON-B and must not be enforced for now.

[scottmarlow]

jakartaee/platform-tck#1062 shows how the (TCK) user can exclude the JSON-B CDI_SE tests via maven-surefire-plugin exclude. If The JSON-B team decides to accept this as a valid workaround for this challenge, then the challenge could be marked as accepted with the workaround being:

      <plugin>
        <artifactId>maven-surefire-plugin</artifactId>
        <version>3.0.0-M5</version>
        <configuration>
          <excludes>
             <exclude>ee.jakarta.tck.json.bind.cdi.customizedmapping.adapters.AdaptersCustomizationCDITest,ee.jakarta.tck.json.bind.cdi.customizedmapping.serializers.SerializersCustomizationCDITest</exclude>
          </excludes>
          </configuration>
      </plugin>

[m0mus]

Folks, I don't see it as a challenge. You are just trying to use JSONB TCKs a way they are not designed to be used. See my comment on Scott's PR: jakartaee/platform-tck#1062 (comment)

Short summary:

Don't run standalone tests on your platform implementation, run only the platform bundle. Standalone tests are designed to test JSONB implementation (not a PLATFORM implementation), they use JUnit and work on Java SE environment. You should certify your JSONB implementation separately. If you use Yasson, you don't need to do it because it's certified already.

[m0mus]

After doscussing this challenge with @scottmarlow and @brideck we agreed that everything works as expected. I am not accepting the challenge.
Although everything works as expected it may not be a convenient for all use cases. I created an issue for CDI tests redesign which we will address later:
#331

[scottmarlow]

Just to clarify, the JSON-B Standalone TCK tests currently do need a SE_CDI implementation to pass, like Weld. 👍 for addressing this later!