TCKs in github actions

[jungm]

• Disables running TCKs by default, introduces tck-jsonb and tck-jsonp profiles to do so
• Seperately runs JSON-P and JSON-P TCKs in GitHub Actions as part of CI (without failing the CI run if TCKs are failing. Might change this in the future when johnzon is passing all TCKs)

[rmannibucau]

Why is it skipped by default? I would enable them and if you need to drop them just exclude the module with maven (-pl !$module), wdyt?

[jungm]

skipped them by default to not break the build just because of a tck failure, imho that behavior can be changed when tcks are passing
Or do you think we should just break the build by default? Can understand that this is possibly unwanted complexity in poms

[rmannibucau]

@jungm the broken build can be "fixed" by excluding the broken tests for now, then we remove the exclusions once they pass but dropping some coverage by default sounds worse than the opposite without more thoughts to me

[jungm]

imho excluding failing tests isn't really a fix, though I can agree on using sth like -pl !tck instead of just skipping TCKs altogether by default

CI wise wdyt in general of splitting up in "johnzon itself builds" and "johnzon is TCK compliant"? I can imagine that when TCKs are done that's not really needed anymore and failing TCKs are a reason to fail the CI run

[rmannibucau]

To be honest tck are just part of the test suite and when i did personally setup them first in projects it was in the impl module, not outside so a split is something weirdish to me.
CI wise we want to enforce the coverage we have so if we pass 80% of tck we want it ran for any pr probably.

[jeanouii]

I'm against skipping the TCK. They are part of the test suite and I want to know if a commit introduces a backward compatibility issue or a TCK issue.

skipped them by default to not break the build just because of a tck failure

This is exactly why tests are useful.
If the build is red, we have 2 options:

• fix our code
• fix or exclude the related test

Either way, it can't be a silent decision/action.

[jungm]

tbh the whole reason I made this PR was so we can get the TCK test suite merged without breaking builds on the master branch. Also CI fails right now at jsonb TCKs and doesn't even run jsonp TCKs
Splitting out TCKs as a seperate job in CI and disabling them by default for now seemed like a good idea to achieve that for me

Feel like this PR can be closed if either:

• the jakartaee-10-tck branch is kept as is to run TCKs (after rebasing)
• TCK fixes should be PRed against jakartaee-10-tck
• jakartaee-10-tck gets merged into master, either as is or disabling currently failing TCKs and PRs that fix those reenable them

[jeanouii]

Ohhh I see.
Yes this is work in progress but I understand your point now.
I rebased the other PR and ran it. I'll copy the results so we can collaborate and get it merged asap on master.

[jeanouii]

Do we want to rebase and merge this?

[rmannibucau]

From the previous comment I'm tempted to say rebase => probably, merge => no with the main blocker having a toggle for tck instead of running what passes by default.
Other open point: why running them in a dedicated module and not in core and jsonb modules as it should be by construction (splitting the module is okish for complex builds like tomee but for trivial ones like batchee or johnzon, not sure)

wdyt?

[jeanouii]

I don't mind. I split them apart because I thought it was more clear and simpler to activate/deactivate the entire module
But I'm fine if we want to merge them.

[jungm]

Is this PR even still needed then if we'll integrate running TCKs into core/jsonb modules? (and probably skip the failing ones for now?)

[jungm]

Closing this, both JSON-P and JSON-B TCKs are passing now so no need to add extra complexity in CI anymore