Add JSON-B TCK tests to repo

[aguibert]

Ports the existing JSON-B TCK tests from the https://github.com/eclipse-ee4j/jakartaee-tck repo.

This will provide several benefits:

• API, spec, and TCK tests are now in the same repo, so it will be much easier to keep the spec/api in sync with the TCK as JSON-B evolves
• The TCK tests can be produced as a consumable artifact, so implementations can simply add a dependency on the TCK tests and run them as normal JUnit tests

Once this PR is merged, I will open a PR on jakartaee-tck to remove the ported tests.

[aguibert]

To more easily review the changes, look at the second commit, which is the modifications I had to make after copy/pasting the tests over from the original jakartaee-tck repo

[m0mus]

You are a cool man @aguibert! Great work. It's quite difficult to review. I'll ask my TCK expert to do it. @jansupol can you review and coment please?

[jansupol]

What exactly do you expect from the TCK tests in the repo? These are just the test classes. Without the TCK framework, these are not able to be run.

[aguibert]

hi @jansupol, I've adapted the tests so they simply use the JUnit framework now, and this repo can now produce the TCK tests as a consumable jar artifact on Maven Central.

The benefit of this is that any implementation can then consume the TCK tests as a basic maven dependency and run them.

For example, here is a draft PR in Eclipse Yasson that will run the TCK tests simply with mvn test and also as part of the CI pipeline:
eclipse-ee4j/yasson#379
Specifically, look at the second commit

This setup is similar to what MicroProfile specs do. As I mentioned in the PR summary, the main benefit is that all of the spec artifacts (API, spec doc, and TCK) are housed in one repo

[aguibert]

Also, @m0mus it will be much easier to review if you only look at the second commit of this PR. The first commit was copy/pasting over the tests from the TCK repo, and the second commit is the changes I made to those tests (e.g. using JUnit instead of the custom TCK harness)

[aguibert]

One thing that I did not include in this PR is the pom.xml updates necessary to publish the TCK tests on maven central. Like Microprofile does:
https://mvnrepository.com/artifact/org.eclipse.microprofile.config/microprofile-config-tck

[scottmarlow]

Hi, just wanted to cross reference a discussion that started on the TCK dev ml that didn't reach a conclusion but might be helpful anyway. The part about retaining the testing of multiple vehicles in the standalone TCKs might be of interest.

[jansupol]

@aguibert I see what you did, now. I see the following issues (maybe I missed those):

• I do not see the signature tests (https://github.com/eclipse-ee4j/jakartaee-tck/tree/master/src/com/sun/ts/tests/signaturetest/jsonb)
• How do you run the CDI tests? CDI is part of the platform, so originally there was a mechanism to build WAR to deploy to the application server so that the actual CDI implementation of the server has been used. But I assume you just start external Weld.
• Have you thought about adding some pieces from the documentation? Especially the assertion list could be handy (https://github.com/eclipse-ee4j/jakartaee-tck/tree/master/internal/docs/jsonb).

[aguibert]

@jansupol

• SigTest: I was aware of the Signature tests, but I'm not sure what they do. If I'm reading SigTestEE correctly it looks like they just verify that the API classes exist? Is this just a hold-over from the early JavaEE days where each implementation had to actually re-write the spec API interfaces themselves? Nowadays it doesn't seem necessary because providers just pull in the API from Maven Central
• CDI tests: yep in Yasson we'd just start up the Weld SE container to verify. Or since CDI support is optional if an implementation didn't provide the CDI integration they could exclude those tests
• doc: Yes I can bring that over, good idea

[jansupol]

• Signature Test - Yes, you are right, signature tests do check the API is equal among vendors. But I assume it should be included. If you grab a newer version of the API and you think the changes were only cosmetical (such as license change, javadoc change), it tells you when some unexpected change is included. Also, it is not a problem to modify the pom to grab an alternative API. But if you think the signature test is obsolete, I would verify on the ongoing mailing list discussion.
• When you run Weld on your own, then the CDI should work, it is not optional, if I read the Spec right, so it makes sense to keep the tests.

[m0mus]

I understand a concern about not meeting Jakarta EE 9 deadlines if we start moving TCK tests out of CTS. I am proposing the following:

We will not move JSONB TCK tests from CTS yet. We will do it after Jakarta EE 9 is released. We will keep them in sync until that time. Users may use TCK tests in this repo as more convenient way of checking compatibility. But it cannot be used officially for compliance testing for now.

[m0mus]

I don't know will signature tests be required for the next platform releases or not. @aguibert do MicroProfile specs have signature tests?

[lukasj]

sigtests are useful to track and detect API changes which may be useful going forward. It's probably the last instance which may catch backward incompatible changes which may fall through reviews, so I would not abandon them.

To put my comment into some context - for EE 9, API changes are not allowed and sigtests are the best to guard this requirement.

[aguibert]

Could the API signatures be validated in the API project instead of being part of the TCK?

In MicroProfile we use a maven plugin to verify that we follow semantic versioning rules:
https://github.com/bndtools/bnd/tree/master/maven/bnd-baseline-maven-plugin

so we could simply add this plugin to the jsonb-api project and it would achieve the same result as the SigTests.

[m0mus]

@aguibert I don't see a configuration for bnd-baseline-maven-plugin in pom.xml. Am I looking at the wrong file? Also, it would be great to include some README with instructions how to perform TCK testing. You can use Yasson as a sample.

In general, I don't have other requests. I think we can merge it after you address my comments above.

[aguibert]

@m0mus currently we don't use the bnd plugin at all. I can add it in a separate PR.

I am going to rework this PR to satisfy some comments on the jakartaee-dev mailing list. Basically I'm going to rework it to use JUnit+Arquillian instead of just JUnit, so that it can easily be tested inside of an app server environment.

[aguibert]

@jansupol @m0mus this PR should be ready for approval now. Here is a summary of the additional changes I made:

• Added bnd-baseline-plugin to enforce semantic versioning rules in API module (replaces need for SigTest)
• Added Arquillian dependency so that the TCK tests can easily be run inside of an app server, in addition to running them standalone (weld-se-standalone container). Using this approach, the all tests (including CDI tests) pass in Yasson

[m0mus]

Everything is cool. Only one thing missing is README file explaining how to run tests with some implementation.

[aguibert]

@m0mus ok, I've pushed another commit with some README instructions (basically just a pointer to how Yasson does it). Should be ready for approval now

[m0mus]

LGTM