jakartaee / faces

Jakarta Faces
Other
100 stars 55 forks source link

Faces 4.0 TCK Challenge to use Date formats that are compatible with Java 21 #1935

Open scottmarlow opened 3 weeks ago

scottmarlow commented 3 weeks ago

https://issues.redhat.com/browse/WFLY-19491 contains the three Faces 4.0 TCK test failures that occur when running on Java 21. I suspect the problem is with invalid month names (e.g. Sep) being used in the following:

[ERROR] Failures: 
[ERROR]   Issue4070IT.testJavaTimeTypes:96->doTestJavaTimeTypes:119 expected:<[2015-09-30T16:14:43]> but was:<[]>
[ERROR]   Issue4087IT.testJavaTimeTypes:101
[ERROR]   Issue4110IT.testJavaTimeTypes:79->doTestJavaTimeTypes:95 expected:<[2015-09-30]> but was:<[]>
[INFO] 

I think that https://github.com/jakartaee/faces/commit/c971c205b5d038ed85c1d6b6ebca8ff13ef83a1b is likely related as that change seems to change partial Sep month names to full month names (e.g. May).

arjantijms commented 3 weeks ago

I can accept the challenge, however we would have to exclude the tests formally, or we'd have to ask for another exception with the specification committee again.

Our assertion for changing the test in 4.1 was that testing for dates was not the real purpose of the test, so we just picked other dates that just happened not to be affected by the JDK 17/21 issue.

Maybe on that base the spec committee can grant us an exception to change the 4.0 tests?

scottmarlow commented 3 weeks ago

I can accept the challenge, however we would have to exclude the tests formally, or we'd have to ask for another exception with the specification committee again.

Please do add the challenge label, thank you!

As per Accepted Challenges section of the TCK Process of which I will paste part of below in quotes, we can make limited test changes if the (Faces) specification lead/team approves of making the test change to be included in the service release:

"

As another alternative to excluding a challenged test, it may be possible to adjust the test validation logic to “expand” the validation check. E.g. if a test expects a value “A1” for a specific variable “x”, but a challenge is raised arguing that the specification language actually allows for either values “A1” and “A2” (but no other values) to be valid values for “x”, then it could be a valid course of action to modify the challenged test to allow either “A1” OR “A2” for “x”.

Since this line of thinking might be applied to cases that aren’t quite as straightforward as this example, care should be taken when using this approach. A particular danger is that an implementation that has already demonstrated compliance before the challenge was raised might actually not pass the new, modified test.

To limit the confusion and additional work such a scenario would cause, if there is already at least one certified compatible implementation before the challenge, the new, modified TCK should be run against at least one such implementation (and ideally all of them) before the changes are published, released, and finalized.

If such a change were released, and it was later found to cause a previously-certified implementation to fail the new, modified test, then excluding the test would likely be the only option, and this would require yet another, additional service release. "

We have already used the above TCK Process change to make other test changes that have been included in service releases. Still, if you prefer to just exclude the tests, that is also fine.

Our assertion for changing the test in 4.1 was that testing for dates was not the real purpose of the test, so we just picked other dates that just happened not to be affected by the JDK 17/21 issue.

My view on the 4.1 change is that the test was broken by a JDK change and changing the test to be able to pass on JDK 21 is the best outcome. If there is risk that the test change may cause Faces 4.0/4.1 implementations to no longer be Faces compatible, then that is a test change that should be avoided. If we do back port the test change to 4.0 and later find that the change breaks compatibility for some Faces implementations (as highly unlikely that could be), then a subsequent Faces 4.0.x TCK service release would then be released that excludes the tests.

Maybe on that base the spec committee can grant us an exception to change the 4.0 tests?

As per above, the TCK Process changed so that you don't have to ask for an exception.

edburns commented 1 week ago

@arjantijms we discussed this at the Platform Dev call today, but we still need an answer.