faces
faces copied to clipboard
jakartaee
TCK Challenge: Application API GetSetViewHanderTest doesn't accommodate ViewHandlerWrappers
Challenged Tests: com.sun.ts.tests.jsf.api.jakarta_faces.application.application.URLClient#applicationGetSetViewHandlerTest com.sun.ts.tests.jsf.api.jakarta_faces.application,applicationwrapper.URLClient#applicationWrapperGetSetViewHandlerTest
TCK Version: Jakarta Faces 4.0.x
Tested Implementation: Open Liberty -- containing MyFaces 4.0
Description: These are both preexisting tests from the Platform TCK that now run differently in Faces 4.0 due to the metadata changes made to involve CDI in the testing performed in the TCK. The tests set a ViewHandler through the appropriate API and then call the corresponding get method. They then check to see if the object received from the get is the same as the one that was just set.
This seems reasonable on its face, but it fails to account for the fact that an implementation might choose to wrap the ViewHandler in another object to make it CDI aware. Open Liberty does this by using a wrapper for the ViewHandler that extends org.jboss.weld.jsf.ConversationAwareViewHandler when CDI is enabled in the server.
The Javadoc for the API under test states that these methods are to set and get "the ViewHandler instance that will be utilized during the Restore View and Render Response phases of the request processing lifecycle." These methods do indeed do as required, as this instance is used in those phases, but they enhance the functionality of what the user provides in an appropriate manner.
The test could be updated to handle this case by using getWrapped to ensure that the wrapped class is indeed what the test set. @volosied will have a PR showing how this could be done shortly.
Commit with a potential fix : https://github.com/jakartaee/faces/commit/5e47009fc1cae30e52583981454c7567e2bffc42
Note: I haven't tested this change locally yet as I've encountered unrelated TCK build errors.
Note that this check in face of an implementation wrapping is something that also theoretically happens for similar tests for jakarta.faces.application.Application#getExpressionFactory().
@volosied
java } else if(resolver instanceof ViewHandlerWrapper && ((ViewHandlerWrapper)resolver).getWrapped().equals(tckHandler)){ matches = true; }
We can start with that, though ultimately maybe we should unwrap in a loop? As nothing forbids a single wrapping, technically nothing would prevent multiple wrappings either.
Commit with a potential fix : https://github.com/jakartaee/faces/commit/5e47009fc1cae30e52583981454c7567e2bffc42
Ps, where was the commit done? Would be best to make it a PR instead.