jakartaee / rest

Jakarta RESTful Web Services
Other
353 stars 114 forks source link

exclude additional Jakarta XML Binding tests that were previously excluded in the Jakarta EE Platform TCK #1117

Closed WhiteCat22 closed 2 years ago

WhiteCat22 commented 2 years ago

While running the TCK without Jakarta XML Binding present, we've been running into failures with these tests that were previously excluded in the Jakarta EE Platform TCK.

Emily-Jiang commented 2 years ago

@spericas @jansupol @jim-krueger please review asap as it blocks Jakarta EE 10 Core Profile ballot. Thank you!

jansupol commented 2 years ago

The test jaxrs-tck/src/main/java/ee/jakarta/tck/ws/rs/spec/provider/standardhaspriority/JAXRSClientIT.java seems to suspiciously exclude tests that shall not be handled by a JAX-B framework, the media type is TEXT/PLAIN.

The Spec, Section 4.2.4 defines default providers for

java.lang.Boolean, java.lang.Character, java.lang.Number Only for text/plain. Corresponding primitive types supported via boxing/unboxing conversion

They should not be handled by JAX-B.

jansupol commented 2 years ago

The other three files seem fine by me.

jansupol commented 2 years ago

Same with

MultivaluedMap<String,String> Form content (application/x-www-form-urlencoded).

The implementation should contain a provider, no matter the JAX-B.

brideck commented 2 years ago

The test jaxrs-tck/src/main/java/ee/jakarta/tck/ws/rs/spec/provider/standardhaspriority/JAXRSClientIT.java seems to suspiciously exclude tests that shall not be handled by a JAX-B framework, the media type is TEXT/PLAIN.

The Spec, Section 4.2.4 defines default providers for

java.lang.Boolean, java.lang.Character, java.lang.Number Only for text/plain. Corresponding primitive types supported via boxing/unboxing conversion

They should not be handled by JAX-B.

This was addressed in the linked issue:

"In this case it's not enough to just mark the individual test method. The application itself has further references to JAXBElement. In the case of OL, it's the reference in ee.jakarta.tck.ws.rs.spec.provider.standardhaspriority.Resource that causes the servlet to fail to initialize."

The change in this PR is also 100% a lateral move from how the tests used to be in the Platform TCK. Because of the framework used (JavaTest) only entire packages could be tagged for exclusion previously.

jansupol commented 2 years ago

it's the reference in ee.jakarta.tck.ws.rs.spec.provider.standardhaspriority.Resource that causes the servlet to fail to initialize.

I see. That sounds correct. The tests should be split into two apps for the future version, then. For the time being, let's mark all the tests.

jansupol commented 2 years ago

I have created issue #1118 for this.

arjantijms commented 2 years ago

we should fasttrack it to resolve this ASAP.

+1

9 days and 3 reviews seem enough, right? I'm never sure why Jakarta REST always needs so many reviews and so much time. For basically all other specs there's either no reviews at all, or just one review. It doesn't look like those specs are so much worse, are they?

spericas commented 2 years ago

Merged

mkarg commented 2 years ago

9 days and 3 reviews seem enough, right? I'm never sure why Jakarta REST always needs so many reviews and so much time. For basically all other specs there's either no reviews at all, or just one review. It doesn't look like those specs are so much worse, are they?

All specs are individually governed and your frequent rants about it do not change anything.