glassfish 7 platform tck jsonp testsuite pluggability test failures. #23892
Comments
|
Platform TCK JSONP testsuite run logs - jsonp-run.txt |
Having to depend on a combined jar instead of the API + Impl seems, wrong. But it is what it is I guess. Unless someone steps up to look at the class loader, I'll replace them by jakarta.json.jar for now. cc @dmatej |
|
the classloading issue may be solvable via having sth like following in the test war: ...untested but I believe that should work |
|
Is there a TCK issue for this by chance? In WildFly we have an issue with the MyJsonProvider assuming it will be the one loaded. I don't see anything in the specification that says providers from a deployment are required to be loaded. |
|
thread context class loader of the deployment is the one to be able to find test provider |
|
In case it helps, I looked inside latest EE 10 Platform TCK build. I extracted contents of jakartaeetck/dist/com/sun/ts/tests/jsonp/pluggability/jsonprovidertests/jsonprovidertests_appclient_vehicle.ear and the contained jsonp_alternate_provider.jar file. Contents of jsonp_alternate_provider.jar: Also worth noting that the pluggability test was in EE 9.1: And the test hasn't been changed as https://github.com/eclipse-ee4j/jakartaee-tck/tree/master/src/com/sun/ts/tests/jsonp/pluggability shows last change was for EE 9.1 (removed unneeded imports). |
Not for all class loading models. For example this is not the case for JBoss Modules. |
|
Implementation discovery consists of following steps:
ServiceLoader is defined to rely on the thread context classloader, it knows nothing about other class loading models. |
|
Sorry, I don't think I explained myself well :) Yes, using the TCCL is correct and it does work. However, there is an assumption that the |
|
Wouldn’t that violate or go against recommendation in section 10.7.2 of the servlet spec? |
|
That section states though:
I will admit the next sentence somewhat contradicts that, but does use the wording "It is recommended". All this said, I'm not really arguing we should make any changes for Jakarta EE 10. I'm just saying there are assumptions there that don't seem valid. However, these tests have been around for a while :) I did get WildFly itself passing the tests now with an up and coming change waiting to be merged. |
|
I do not think that the implementation of a spec is ment by container’s implementation classes. How would it be possible for the user app to use ie hibernate specific features in a server having it as a default runtime which is not made accessible (to satisfy should not allow access part)? |
|
In my personal opinion that shouldn't be allowed :) The whole point of Jakarta EE is to provide a container where dependencies like that are not required in a deployment. Anyway, I really don't want to distract from this issue since it's a GlassFish filed issue and not really about this specifically :) |
|
It is - the test is to cover the use-case where user-app contains different spec runtime than the one provided by the server (ie jackson vs parsson, hibernate vs eclipselink vs openjpa, etc; or even just different version from the one provided by the server). One container is designed to host different deployments with possibly different needs at the same time. If the deployment should not be allowed to override the server provided runtime, this test and use-case probably becomes invalid. |
User-app containing different spec implementation was never considered part of the Jakarta Platform specification as far as I can remember. I agree that this discussion could be moved to a SPEC issue for discussion. |
|
more discussion on issue at GF dev - https://www.eclipse.org/lists/glassfish-dev/msg01279.html |
|
@arjantijms from the linked GlassFish discussion, it sounds like this failure is thought to be a (GlassFish) implementation bug. Do you agree? |
|
At least |
|
Looks like we are getting further, actually since this is a generic issue, I will just paste new errors here. Results are here https://ci.eclipse.org/jakartaee-tck/job/jakartaee-tck/job/master/1813/artifact/jsonp-results.tar.gz The first failure that I see in the below results is: Note the above Then we see the NullPointerException: |
|
Note that I am only reporting each failure once even though the same problem occurring more than one test. Continuing to add additional failures from previous comment results: Failure in com/sun/ts/tests/jsonp/pluggability/jsonprovidertests/Client.java#jsonProviderTest11_from_appclient com/sun/ts/tests/jsonp/pluggability/jsonprovidertests/Client.java#jsonProviderTest12_from_appclient failure: com/sun/ts/tests/jsonp/pluggability/jsonprovidertests/Client.java#jsonProviderTest13_from_appclient failure: com/sun/ts/tests/jsonp/pluggability/jsonprovidertests/Client.java#jsonProviderTest14_from_appclient: com/sun/ts/tests/jsonp/pluggability/jsonprovidertests/Client.java#jsonProviderTest15_from_appclient failure: |
|
log.zip has the remaining failures (72 total including the above 25 failures). |
|
@lukasj is this failure limited to EE implementations that have the org.glassfish.hk2.osgiresourcelocator.ServiceLoader class? I don't see it with WildFly currently but am curious if other Jakarta EE 10 implementations may also see the same failure and need to follow the recommendation to combine the JSON-P SPEC API classloader with the JSON-P implementation classloader? |
|
This was fixed in WildFly by exporting the implementation to the deployments class loader https://github.com/wildfly/wildfly/blob/98fd7caada41a1e30b1cea5cd10904a1056d78cc/ee-9/ee-9-api/src/main/resources/modules/system/layers/base/jakarta/json/api/main/module.xml#L35-L42. |
|
So it seems that this issue impacts non-OSGi EE implementations also which does sound like JSON-P |
|
It actually could be the fact that the old GlassFish RI used to ship with both the API and implementation. Eclipse Parsson correctly leaves the API out of the artifact. I'm making some guesses though because I don't know much about how other app servers crate class loaders. |
|
@arjantijms do you think GlassFish should work around this failure or challenge the (Platform TCK JSON-P) tests that are failing? I'm not sure of what the outcome of the challenge would be of course. Even if the tests aren't challenged, they could be in the future if other implementations raise a TCK challenge. |
|
Reproduced locally now: jsonp, 4 passed, 72 failed, 284 seconds. |
GlassFish includes both Parsson 1.1.0 and JSON-API 2.1.0. |
|
Do I understand well, that:
See also ServiceLoader.iterator's javadoc:
So ... now what? |
read the end of Guru's initial report - the part about replacing This is how I did it:
=> I see expected empty json array there and no exception in the log OR read my comment from April 6... This is what I did:
=> I got
<glassfish-web-app>
<class-loader delegate="false"/>
</glassfish-web-app>
=> I see expected empty json array there and no exception in the log This should clearly show that if I am able to get my custom json provider implementation sitting in the WAR file loaded and used properly by the server, there should be a way to get these tests passing. An alternative could also be to ask the parsson project to provide extra implementation only binary without META-INF/services file for GF (or GF could do this modification during the build time itself). As to why both ways work:
the doesn't exist any more part is wrong - the default
Well, one can wheedle the ServiceLoader to load what is required, especially in the EE server environment, where particular hierarchy and classloader ordering is defined. The trick here is to make the app loader to be the first instead of the last by changing the order through the ... the choice & decision is yours... |
|
Just for the record - I prefer addition of gf-web.xml file(s) to the TCK over going back to API+Impl bundle on the GF side |
|
Nice recapitulation, but ...
But in my opinion TCK tests are right, this should not happen on appserver :-) |
|
re 1&2 there is a difference in how SE and EE do things wrt APIs and JPMS which is relevant for some EE specs, including JSON Processing - the former believes that each API should come with some default implementation allowing it to be overriden while the latter believes APIs should be completely standalone leaving the responsibility of finding out and setting up some implementation up to the user. jakarta.json.jar follows what SE team does, jakarta.json-api.jar+parsson.jar follows what EE wants. The argument about accidentally using implementation specific APIs EE has with the SE approach became less relevant on JPMS. OTOH EE servers do not support JPMS to enforce it from the implementations and one can not expect users to rely on JPMS when they're building their apps. re 3 this is documented approach in the GF user guide to allow apps to override arbitrary server provided library. But I have to agree one cannot expect users to read docs, especially when there is SO :-D re 4 that can work too |
I would say that doesn't mean the implementation should be referenced directly from the code, just that there should be some real-world-usable implementation proving that the concept is correct. re re 3) Maybe someone should read JVM javadoc first ... it is not about ignoring documented "workarounds", but about having to override the default behavior of the server, more work, more explanations. Btw I tried that and it wasn't enough. Nevermind, I found yet solution 5, which already existed in GlassFish, but just needs some changes. Now I am on 18 failures instead original 72, so this will be resolved soon without much pain, finally, uffff. Changes are just in GlassFish, so no more pain for other projects :-) |
…oyed applications
Issue #23892 Fixed rules for overriding resources by deployed applications
glassfish 7 latest nightly build has 72 test failures for platform tck jsonp pluggability tests.
The failing test are as follows:
Detailed logs for failure can be found at: https://ci.eclipse.org/jakartaee-tck/blue/rest/organizations/jenkins/pipelines/jakartaee-tck/branches/master/runs/1722/nodes/50/steps/199/log/?start=0
Based on discussion with @lukasj, failure reason for test failures with GF 7 is due to presence of jars
jakarta.json-api.jar + parsson.jarinstead of justjakarta.json.jar.jakarta.json-api.jar + parsson.jarwill not pass pluggability tests without changes to GF7 class-loader, "jakarta.json.jar" will pass pluggability tests without modification of GF class-loader.CC @lukasj
The text was updated successfully, but these errors were encountered: