search

jakartaee / faces

Jakarta Faces
Other
102 stars 54 forks source link

TCK Challenge - SignatureTest changes required to match section 5.9.1 #1474

Closed shighbar closed 3 years ago

shighbar commented 5 years ago

Specification version JSF 2.3 (Section 5.9.1)

Coordinates of the challenged test(s) https://github.com/eclipse-ee4j/jakartaee-tck/tree/master/src/com/sun/ts/tests/signaturetest

TCK version & exclude list versions build.name=jakartaeetck-8.0.0_07-Aug-2019 build.date.time=Wed, 7 Aug 2019 08\:52\:52 build.comment=\ 

Exclude list default with above build. Last comit 5c0de06 on March 27, 2019.

Implementation under test Open Liberty, IBM Corporation https://openliberty.io/ OpenJDK 1.8.0_222

Full test names: com/sun/ts/tests/signaturetest/javaee/JavaEESigTest.java#signatureTest_from_jsp com/sun/ts/tests/signaturetest/javaee/JavaEESigTest.java#signatureTest_from_servlet

Description The above signature tests fail because the spec has contradicting guidence which has resulted in different implementations having different signatures. See supporting issues below. 1) The JSF 2.3 specification in Section 5.9.1 states the following: Support for Injection into JSF Managed Objects It must be possible to use @Inject when specifying the following kinds of JSF managed objects.

2) FacesConverter is defined in the following: https://javaee.github.io/javaee-spec/javadocs/javax/faces/convert/FacesConverter.html this lists the following target: @Target(value={TYPE,FIELD,METHOD,PARAMETER})

3) FacesValidator is defined in the following: https://javaee.github.io/javaee-spec/javadocs/javax/faces/validator/FacesValidator.html this lists the following target: @Target(value=TYPE)

4) FacesBehavior is defined in the following: https://javaee.github.io/javaee-spec/javadocs/javax/faces/component/behavior/FacesBehavior.html this lists the following target: @Target(value=TYPE)

Note the difference in the Target specified, in order for the FacesValidator and FacesBehavior to do what is specified in section 5.9.1 they must match the @Target of FacesConverter. See change in the open source here: https://issues.apache.org/jira/browse/MYFACES-4159

The solution here would be to update the sig-test-pkg-list_se8 golden file and add the below classes to the end to match the updated spec once complete (Spec issue 1471) and updated in the RI (see Mojarra issue 4389).

I believe these tests should be ignored or an exception made to edit the sig-test-pkg-list_se8.txt to add in the missing classes ( javax.faces.validator.FacesValidator & javax.faces.component.behavior.FacesBehavior ) for implementations which have implemented the new behavior.

Supporting materials:

https://github.com/eclipse-ee4j/faces-api/issues/1471 https://github.com/eclipse-ee4j/mojarra/issues/4389 https://issues.apache.org/jira/browse/MYFACES-4159

kwsutter commented 5 years ago

Just to clarify, this is the same issue that WebSphere Liberty raised during the Java EE 8 testing. We're just trying to make it Jakarta EE 8 official. Since we can not change testcases for Jakarta EE 8, can we just get these excluded for now? We will have to do the real fix post Jakarta EE 8. Thanks.

arjantijms commented 5 years ago

@kwsutter I'm fully supportive of this. What do we have to do to make this official? Should be a good point to raise in the next spec committee call at least.

kwsutter commented 5 years ago

@arjantijms We should first accept this as a valid problem by labeling this issue with "accepted". Since the full solution requires updates to the tests and javadoc, I don't know if we have the time in Jakarta EE 8 to do this. So, we probably need to document how to exclude these tests as a workaround. It's my understanding that these signature tests are all-or-none. We don't want to exclude all of the signature tests. Just the ones identified. This may require each implementation to modify their TCK run to only exclude the ones that are in the wrong (or hand-modify the errant TCK tests). Either case, it sounds like a documented workaround for now.

shighbar commented 5 years ago

@kwsutter, @arjantijms It is true that these tests are kind of all or nothing - they test a great many packages and if any fail the entire test fail. The process would normally be to exclude these tests until corrected. The solution listed though while we can't implement until the spec issues resolved and the next release of TCK - is worth executing to ensure that there are not other signature test failures. With the above solution the test should pass proving that many other packages under test are correct. The other option in the interim until corrected is to confirm in the test logs that the only Failed packages are those in questions. For example from our logs; Failed packages listed below: javax.faces.component.behavior(static mode) javax.faces.component.behavior(reflection mode) javax.faces.validator(static mode) javax.faces.validator(reflection mode) Hopefully that helps anyone else running the signature tests.

bshannon commented 5 years ago

@kwsutter @arjantijms I think we need to go through the approval process for the TCK before we can go through the test challenge process for tests in the TCK.

In the normal case we would just fix a bug like this in the TCK before it's released. But we decided that the Jakarta EE 8 TCK should be bug-for-bug compatible with the Java EE 8 so that any product that passes the Java EE 8 TCK can also pass the Jakarta EE 8 TCK.

It sounds like the fix for this would cause existing Java EE 8 products to fail the TCK. For Java EE we would've added an alternate test to allow either signature, but for Jakarta EE we decided that we would not allow alternate tests. When evaluating this test challenge we'll need to decide whether it's appropriate to update the TCK in a way that would cause existing Java EE 8 products that are expecting to pass the Jakarta EE 8 TCK to fail.

kwsutter commented 5 years ago

@bshannon I agree that the official challenge process is for post approval, but this is kind of unique. We found this problem during our initial Java EE 8 testing. When we tried to get it resolved for Jakarta EE 8, we determined that we didn't want to upset the apple cart with progressing Jakarta EE 8 (and keep it consistent with Java EE 8).

Since we (WebSphere Liberty) had to do a workaround for the Java EE 8 testing, I just wanted to document this issue and get it accepted as something to correct post Jakarta EE 8. I would also like validation that the proposed workarounds are suitable for now. Not looking for any TCK or javadoc updates at this time. Thanks.

We will be doing some form of the proposed workarounds for Open Liberty's certification of Jakarta EE 8. In case there are any questions, we wanted to have something to reference (this issue).

kwsutter commented 5 years ago

Per the discussion in our Spec Committee, the specific failures referenced by this Challenge are as follows:

Summary

[javatest.batch] Some signatures failed.
[javatest.batch]    Failed packages listed below: 
[javatest.batch]        javax.faces.component.behavior(static mode)
[javatest.batch]        javax.faces.component.behavior(reflection mode)
[javatest.batch]        javax.faces.validator(static mode)
[javatest.batch]        javax.faces.validator(reflection mode)

Specifics

[javatest.batch] Missed Annotations
[javatest.batch] ------------------
[javatest.batch] 
[javatest.batch] javax.faces.component.behavior.FacesBehavior:               anno 0 java.lang.annotation.Target(java.lang.annotation.ElementType[] value=[TYPE])
[javatest.batch] 
[javatest.batch] Added Annotations
[javatest.batch] -----------------
[javatest.batch] 
[javatest.batch] javax.faces.component.behavior.FacesBehavior:               anno 0 java.lang.annotation.Target(java.lang.annotation.ElementType[] value=[FIELD,METHOD,PARAMETER,TYPE])
[javatest.batch] 
[javatest.batch] 09-05-2019 11:24:59:  SVR: ********** Package 'javax.faces.component.behavior' - FAILED (STATIC MODE) **********
[javatest.batch] Missed Annotations
[javatest.batch] ------------------
[javatest.batch] 
[javatest.batch] javax.faces.component.behavior.FacesBehavior:               anno 0 java.lang.annotation.Target(java.lang.annotation.ElementType[] value=[TYPE])
[javatest.batch] 
[javatest.batch] Added Annotations
[javatest.batch] -----------------
[javatest.batch] 
[javatest.batch] javax.faces.component.behavior.FacesBehavior:               anno 0 java.lang.annotation.Target(java.lang.annotation.ElementType[] value=[FIELD,METHOD,PARAMETER,TYPE])
[javatest.batch] 
[javatest.batch] 09-05-2019 11:24:59:  SVR: ********** Package 'javax.faces.component.behavior' - FAILED (REFLECTION MODE) **********
[javatest.batch] javax.faces.validator.FacesValidator:   anno 0 java.lang.annotation.Target(java.lang.annotation.ElementType[] value=[TYPE])
[javatest.batch] 
[javatest.batch] Added Annotations
[javatest.batch] -----------------
[javatest.batch] 
[javatest.batch] javax.faces.validator.FacesValidator:   anno 0 java.lang.annotation.Target(java.lang.annotation.ElementType[] value=[FIELD,METHOD,PARAMETER,TYPE])
[javatest.batch] 
[javatest.batch] 09-05-2019 11:25:02:  SVR: ********** Package 'javax.faces.validator' - FAILED (STATIC MODE) **********
[javatest.batch] Missed Annotations
[javatest.batch] ------------------
[javatest.batch] 
[javatest.batch] javax.faces.validator.FacesValidator:   anno 0 java.lang.annotation.Target(java.lang.annotation.ElementType[] value=[TYPE])
[javatest.batch] 
[javatest.batch] Added Annotations
[javatest.batch] -----------------
[javatest.batch] 
[javatest.batch] javax.faces.validator.FacesValidator:   anno 0 java.lang.annotation.Target(java.lang.annotation.ElementType[] value=[FIELD,METHOD,PARAMETER,TYPE])
[javatest.batch] 
[javatest.batch] 09-05-2019 11:25:02:  SVR: ********** Package 'javax.faces.validator' - FAILED (REFLECTION MODE) **********
scottmarlow commented 3 years ago

@arjantijms could you please close this issue with label changed to accepted or invalid as per https://jakarta.ee/committees/specification/tckprocess

Whether this is accepted or invalid, I do not see any immediate action other than considering https://github.com/eclipse-ee4j/faces-api/issues/1471 which I think is still pending for the next EE release that makes Jakarta Server Faces changes.

brideck commented 3 years ago

@scottmarlow Since this has been accepted, should my plan re: certification be to show results that include these failures and then reference this issue and provide a link to our specific results showing that these are our only failures in the signature tests?

scottmarlow commented 3 years ago

@arjantijms can you please close this accepted TCK challenge. Thank you!

arjantijms commented 3 years ago

Hi, sorry, it should have been closed on 11 Dec. Thanks for the reminder!

edbratt commented 3 years ago

Are the TCK tests updated, or have instructions for correcting the TCK been worked out with the TCK project / team?

scottmarlow commented 3 years ago

Are the TCK tests updated, or have instructions for correcting the TCK been worked out with the TCK project / team?

No, are you thinking we should make the TCK signature test tooling not verify the annotation signatures (so it would ignore the difference)? Or something?