Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set renderKitId within initFaces() #1653

Merged
merged 1 commit into from Apr 22, 2022

Conversation

volosied
Copy link
Contributor

@volosied volosied commented Apr 22, 2022

Previously when application.getViewHandler().createView(facesContext, viewId); was called, the renderKitId was set within createView. (Please correct me if I'm wrong)

https://github.com/eclipse-ee4j/mojarra/blob/3.0.2/impl/src/main/java/com/sun/faces/application/ViewHandlerImpl.java#L314-L356

My proposed solution is to set the renderKitId during the setup phase in the test instead. Everything I tried failed because ViewHandlingStrategyNotFoundException was thrown. The viewId is null here, so there's not strategy that can returned.

In my testing, HTML_BASIC was set.

I think this would be best solution. Also, can anyone run a build to verify?

@arjantijms @alwin-joseph

@volosied volosied mentioned this pull request Apr 22, 2022
12 tasks
@alwin-joseph
Copy link
Contributor

alwin-joseph commented Apr 22, 2022

I can see good improvement in the test results with this change when run in my local. Thanks @volosied !
Test results: passed: 5,068; failed: 362

Others can validate if the solution is feasible.

There were issues like below which is an issue with the httpclient-4.5.5.jar dependency and not test issue. I suspect this is my local environment issue, using the jar from https://github.com/eclipse-ee4j/jakartaee-tck/blob/master/lib/httpclient-4.5.5.jar resolved the same issue with Security tck for me.
[exec] [javatest.batch] 04-22-2022 13:00:58: ERROR: java.lang.IllegalArgumentException: Cannot locate declared field class org.apache.http.impl.client.HttpClientBuilder.sslcontext

Hence the result might be same or better than this with this change.

Other exceptions from server.logs. These can be separately looked into.

  1. [echo] java.lang.NullPointerException
    

    [echo] at jakarta.faces.component.UIComponentBase.getRenderer(UIComponentBase.java:1047)
    [echo] at jakarta.faces.component.UIComponentBase.getClientId(UIComponentBase.java:242)
    [echo] at com.sun.ts.tests.jsf.api.jakarta_faces.context.facescontextwrapper.TestServlet.addTestMessagesToContext(TestServlet.java:1022)
    [echo] at com.sun.ts.tests.jsf.api.jakarta_faces.context.facescontextwrapper.TestServlet.facesCtxWrapperGetMessageListTest(TestServlet.java:349)
    [echo] at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    [echo] at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    [echo] at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    [echo] at java.base/java.lang.reflect.Method.invoke(Method.java:566)
    [echo] at com.sun.ts.tests.jsf.common.servlets.HttpTCKServlet.invokeTest(HttpTCKServlet.java:163)
    [echo] at com.sun.ts.tests.jsf.common.servlets.HttpTCKServlet.doGet(HttpTCKServlet.java:104)

  2.  [echo] java.lang.IllegalArgumentException: wrong number of arguments
    

    [echo] at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    [echo] at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    [echo] at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    [echo] at java.base/java.lang.reflect.Method.invoke(Method.java:566)
    [echo] at com.sun.ts.tests.jsf.common.servlets.HttpTCKServlet.invokeTest(HttpTCKServlet.java:163)
    [echo] at com.sun.ts.tests.jsf.common.servlets.HttpTCKServlet.doGet(HttpTCKServlet.java:104)

  3.  [echo] java.lang.NullPointerException
    

    [echo] at com.sun.faces.context.flash.ELFlash.loggingGetPhaseMapForWriting(ELFlash.java:806)
    [echo] at com.sun.faces.context.flash.ELFlash.getPhaseMapForWriting(ELFlash.java:835)
    [echo] at com.sun.faces.context.flash.ELFlash.entrySet(ELFlash.java:492)
    [echo] at com.sun.faces.context.flash.FlashELResolver.getFeatureDescriptors(FlashELResolver.java:335)
    [echo] at com.sun.faces.el.DemuxCompositeELResolver$DescriptorIterator._getCurrIterator(DemuxCompositeELResolver.java:299)
    [echo] at com.sun.faces.el.DemuxCompositeELResolver$DescriptorIterator.hasNext(DemuxCompositeELResolver.java:274)
    [echo] at com.sun.ts.tests.jsf.spec.el.elresolvers.TestServlet.facesImplicitObjectResolverFeatureDescriptorTest(TestServlet.java:216)

@alwin-joseph
Copy link
Contributor

Update :
With this change (and #1654) I was able to get below result from https://ci.eclipse.org/jakartaee-tck/job/10/job/jakarta-faces-tck-build-run/7/
Test results: passed: 5,220; failed: 210

Server log exceptions towards the end of https://ci.eclipse.org/jakartaee-tck/job/10/job/jakarta-faces-tck-build-run/7/consoleFull.

@volosied
Copy link
Contributor Author

Thanks @alwin-joseph! I see a number of jakarta.servlet.ServletException: com.sun.faces.application.view.ViewHandlingStrategyNotFoundException errors in the logs. I'll try to tackle that next.

@volosied volosied changed the title Call setRenderKitId within initFaces Set renderKitId within initFaces() Apr 22, 2022
@arjantijms
Copy link
Contributor

@volosied good find, wow!

@arjantijms arjantijms merged commit b9c548b into jakartaee:master Apr 22, 2022
@volosied volosied mentioned this pull request Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants