jsonb-api icon indicating copy to clipboard operation
jsonb-api copied to clipboard

TCK must check serializers better

Open mkarg opened this issue 5 years ago • 4 comments

TCK as of Jakarta EE 8 does not detect the following violation of the JSON-B specification 1.1:

  • Johnzon does not implement JsonbConfig#with(De)Serializers, but instead throws an exception, while the JSON-B specification does not explicitly say that support of that methods is optional. Users of the spec will interpret the chapter as "all JSON-B implementations will support that", and run into problems when using Johnzon. Either the TCK has to be more strict, or the spec has to be more clear.
  • Yasson runs deserializers alyways even for classes not being matched by the generic type of the deserializer.

See also the discussion between @romain-grecourt and me in #184

Closing #184 hereby, as it ended up in the decision that most likely the TCK is not strict enough.

mkarg avatar Aug 22 '19 10:08 mkarg

Hmm some tck executions were failing due to that when we made it passing at johnzon so it sounds like it is covered. So guess tck are ok and johnzon pre 1.2.0 was not respecting jsonb. Do you have a pointer it would be a jsonb issue?

rmannibucau avatar Aug 22 '19 11:08 rmannibucau

Currently the JSON-B TCK tests are housed in a different repo with the rest of the JakartaEE tests: https://github.com/eclipse-ee4j/jakartaee-tck/tree/master/src/com/sun/ts/tests/jsonb

After JakartaEE 8 I would like to migrate these tests into this repo under a tck folder (just like MicroProfile does) so that we have the src, spec, javadoc, website, and tck tests all under the same repo so it's easy to keep them in sync.

Also, we could produce the JSON-B TCK as a maven artifact so that implementations could consume it and run themselves against the TCK easily, and preferably as part of their CI/CD pipeline too.

aguibert avatar Aug 27 '19 16:08 aguibert

@aguibert I saw that you already created jsonb-api/tck but I don't see the usage in yasson about the artifact jakarta.json.bind-tck. Yasson is still using the script yasson/etc/tck.sh that downloads the tests from: http://download.eclipse.org/ee4j/jakartaee-tck/jakartaee8-eftl/promoted/eclipse-${TCK_NAME}-$TCK_VERSION.zip

Is this still in progress, or I am missing something?.

I'm asking because we are planing to do the same for jsonp, so we can take similar approach.

In case you have some local changes in yasson related to this topic do you mind to share it with me in advance?.

jbescos avatar Feb 18 '20 10:02 jbescos

I found it. There is pull request with it: https://github.com/eclipse-ee4j/yasson/pull/379

jbescos avatar Feb 18 '20 14:02 jbescos