jakartaee / rest

Jakarta RESTful Web Services
Other
354 stars 114 forks source link

TCK Migration: Move pending tests from jakartaee-tck/src/com/sun/ts/tests/jaxrs/api (#1015) #1030

Closed jelemux closed 2 years ago

jelemux commented 2 years ago

This PR migrates all tests from jakartaee-tck/src/com/sun/ts/tests/jaxrs/api in https://github.com/eclipse-ee4j/jakartaee-tck.

Linked issues: #1015

To test the changes :

export JAVA_HOME=<JDK11_HOME>
export M2_HOME=<MAVEN_HOME>
export PATH=$M2_HOME/bin:$JAVA_HOME/bin:$PATH

In jaxrs-tck/: mvn clean install

In jersey-tck/: run all tests : mvn clean verify -Parq-glassfish-managed

Test failures: (those should be tracked in #1020)

alwin-joseph commented 2 years ago

@jelemux Thankyou for this PR. Please note I have created a PR https://github.com/eclipse-ee4j/jaxrs-api/pull/1029 with tck migration for tests in jaxrs/ee/rs/core,client,ext folders. There were dependencies on some files in jaxrs/api folders for the same. Hence I have simply added those files too without enabling the test runs. So it is possible that we have merge conflicts with these two PRs. We should pick the changes for those files from this PR only while resolving the conflicts.

alwin-joseph commented 2 years ago

@jansupol @mkarg @andymc12 @spericas Please review

alwin-joseph commented 2 years ago

Test failures: (those should be tracked in #1020)

* `api/rs/core/newcookie`: `equalsTest` fails because `NewCookie.hashcode()` throws a `NullPointerException` because sameSite is `null` (test disabled)

* `api/rs/core/linkjaxbadapter`: Implementation for `jakarta.xml.bind-api` cannot be found and I don't know how to add it (tests disabled)

* `api/rs/ext/interceptor`: Tests fail with `IllegalStateException: [FATAL] HttpRequest is null` (tests disabled)

We can revisit the failures later (tracked in https://github.com/eclipse-ee4j/jaxrs-api/issues/1020) as some of the fixes for eg for api/rs/core/linkjaxbadapter failure might be pushed in different changes. I have added jakarta.xml.bind-api in the jaxrs-tck/pom.xml which should resolve the same.

mkarg commented 2 years ago

@jelemux Thanks a lot for this PR, you're awesome! :-)

alwin-joseph commented 2 years ago

@jelemux Thankyou for this PR. Please note I have created a PR #1029 with tck migration for tests in jaxrs/ee/rs/core,client,ext folders. There were dependencies on some files in jaxrs/api folders for the same. Hence I have simply added those files too without enabling the test runs. So it is possible that we have merge conflicts with these two PRs. We should pick the changes for those files from this PR only while resolving the conflicts.

@jelemux Hi Jeremias - We have the PR https://github.com/eclipse-ee4j/jaxrs-api/pull/1029 merged now causing conflicts in this PR with master. Can you please help resolve it by picking the changes in this PR over the current changes in master.

The conflict was created since I had to use some of the test files from api/ folder to run the tests in #1029. But I had not made enough changes in those files from api/ to run them as Junit tests.

Thanks in advance.

jelemux commented 2 years ago

@alwin-joseph Will do! Maybe I'll find some time this evening.