3 Replies Latest reply on Nov 16, 2007 4:30 AM by Tobias Hill

    A suggested SeamTest improvement

    Matt Drees Master

      I spent some time debugging one of my tests, trying to figure out why the conversation wasn't being propagated. I had something (very roughly) like this:

      
       String cid = new FacesRequest("/page1.xhtml") {
       @Override
       protected void invokeApplication() throws Exception {
       Manager.instance().beginConversation();
       FacesManager.instance().redirect("/page2.xhtml");
       }
      
       }.run();
      
       cid = new FacesRequest("/page2.xhtml", cid) {
       @Override
       protected void invokeApplication() {
       assert Manager.instance().isLongRunningConversation();
       }
       }.run();
      


      But the assertion fails, which I learned has to do with the fact that there was nothing rendered in the first request. So, I can fix it by doing this:

      
       String cid = new FacesRequest("/page1.xhtml") {
       @Override
       protected void invokeApplication() throws Exception {
       Manager.instance().beginConversation();
       FacesManager.instance().redirect("/page2.xhtml");
       }
      
       }.run();
      
       cid = new NonFacesRequest("/page2.xhtml", cid) {
       }.run();
      
       cid = new FacesRequest("/page2.xhtml", cid) {
       @Override
       protected void invokeApplication() {
       assert Manager.instance().isLongRunningConversation();
       }
       }.run();
      


      But it annoys me to have that empty request in there. So, it'd be nice to have something like this:

       String cid = new FacesRequestAndRedirect("/page1.xhtml", null, "/page2.xhtml") {
       @Override
       protected void invokeApplication() throws Exception {
       Manager.instance().beginConversation();
       FacesManager.instance().redirect(getRedirectedTo());
       }
      
       @Override
       public void renderAfterRedirect() throws Exception {
       assert Manager.instance().isLongRunningConversation();
       }
       }.run();
      
       cid = new FacesRequest("/page2.xhtml", cid) {
       @Override
       protected void invokeApplication() throws Exception {
       assert Manager.instance().isLongRunningConversation();
       }
       }.run();
      


      And it turns out that I could make it happen with this convolution:
      
       public class FacesRequestAndRedirect extends FacesRequest {
      
       private String redirectedTo;
      
       public FacesRequestAndRedirect() {
       super();
       }
      
       public FacesRequestAndRedirect(String viewId, String conversationId) {
       super(viewId, conversationId);
       }
      
       public FacesRequestAndRedirect(String viewId) {
       super(viewId);
       }
      
       public FacesRequestAndRedirect(String viewId, String conversationId, String redirectedTo) {
       super(viewId, conversationId);
       setRedirectedTo(redirectedTo);
       }
      
       @Override
       public final void renderResponse() throws Exception {
      
       }
      
       public void renderAfterRedirect() throws Exception {
      
       }
      
       public String getRedirectedTo() {
       return redirectedTo;
       }
      
       public void setRedirectedTo(String redirectedTo) {
       this.redirectedTo = redirectedTo;
       }
      
       @Override
       public String run() throws Exception {
       String cid = super.run();
      
       cid = new NonFacesRequest(getRedirectedTo(), cid) {
      
       @Override
       protected void renderResponse() throws Exception {
       renderAfterRedirect();
       }
       }.run();
       return cid;
       }
       }
      


      So, I haven't used it or thought about it much, but it might be worth putting something like this into Seam. Any thoughts?



        • 1. Re: A suggested SeamTest improvement
          Pete Muir Master

          Add a FR to JIRA and see what Gavin says :)

          • 3. Re: A suggested SeamTest improvement
            Tobias Hill Newbie

            I think most people will think their test is ok since it will pass (i.e. none of the assertions in renderRespose breaks). What most people (or was it just me?) will not realize is that the renderResponse isn't even called after a redirect - happily not knowing that they tested nothing.

            I would very much like to see this tightened up in one way or another. Maybe something like: If the user has overridden the renderResponse on a call that issues a redirect SeamTest could warn or even break with some clarifying message (i.e. "The call redirected, override renderResponseAfterRedirect instead). Or something better. I am just elaborating on how to make testing more fool-proof.

            I have myself been fooled by a redirect. When I recently implemented a major change to our system that I thought would break a lot of the integration test suite (it should), I was very surprised to see that most of it was still testing green. After even adding a "fail()" to the renderresponse() method and still testing green I got even more puzzled. But it led me to this and some other posts clarifying things.


            Tobias