1 2 Previous Next 18 Replies Latest reply on Aug 14, 2013 11:45 AM by lfryc

    Page Fragments refactoring

    jhuska

      Hello guys, me and Jirka are working on Page Fragments refactoring. We have started with Accordion and Autocomplete page fragments. We would like to know your opinions on some things, to be sure that we can continue in the similar way with other fragments.

       

      Lets start with RichFacesAutocomplete page fragment.

      The refactoring is made in this commit

       

      Basically what we are trying to achieve with all page fragments:

      • create high level API, which contains only important methods which all users will need. e.g.: Autocomplete
      • its implementation which will hide other not so important methods (usually used for framework testing), so the API stays simple and clean
      • Sample of usage of the new Autocomplete ca be found here.

       

      Second refactored page fragment is RichFacesAccordion.

      The refactoring is made in this commit.

       

      • (2) We would like to use interfaces SwitchableComponent and ComponentsContainer as a base for common functionality for: r:tabPanel, r:accordion, r:collapsiblePanel, r:togglePanel
      • currently it is implemented just for r:accordion, but we believe that it will work for the other as well
      • Sample usage of such accordion.

       

      (3) Third question: Some page fragments are able to retrieve their settings from the <script> element of the corresponding element. E.g. Autocomplete can retrieve whether it is Ajax or client based, or tokens used for separating the values. An user of such page fragment would not have to call .setMode() or setTokens(String tokens). There are other examples for other page fragments. The concern here is that such Page Fragments will be behaving in a little bit in magic way. On the other hand it can save some of the settings which user needs to do. What is your opinion on this ? IMHO we can setup the page fragment automatically from script, but still we should provide setters for these features, hidden in the AdvancedInteractions.

       

      Do you have some ideas on this guys ? What can be done better or so ?

      Thanks

        • 1. Re: Page Fragments refactoring
          lfryc

          Hey Juraj, Jiri,

           

          (1) I'm not convinced about AdvancedInteractions pattern, seems too much methods could hide under this, tests will be then probably cluttered with advanced().

          I would certainly give it a try though.

           

          I believe advanced().setToken(";") could be exposed rather as setup().token(";"), etc.

           

          (2) Remember that oeople should be able to simply expose direct methods for retrieving particular content - e.g.:

           

          UserDetails details = tabPanel.switchToUserDetails(); // cals switchTo(1).getContent(UserDetails.class) underneath
          ContactInformation contact = tabPanel.switchToContactInformation(); // calls switchTo(2).getContent(ContactInformation);
          

           

          But that's not completely relevant - you should just have such an API in mind.

           

          Otherwise both fragments looks great.

           

          ----

           

          (3) I think it's a magic, it could be exposed as high-level API though, see:

           

          // I'm not fan of this non-type-safe definition, but it's certainly an option
          @FindBy(".rf-cal") @Initialize("token=;")
          RichFacesCalendar calendar; 
          
          // no magic involved
          @FindBy(".rf-cal")
          RichFacesCalendar calendar = RichFacesCalendar.init().token(";");
          
          // ha, magic will be involved, but it is requested by user himself!
          @FindBy(".rf-cal")
          RichFacesCalendar calendar = RichFacesCalendar.init().fromWidget();
          
          
          public void test() {
               // request a magic
               calendar.setup().fromWidget();
          }
          
          

           

          Note that the RichFacesCalendar.init()... won't "inject" calendar implementation itself, it would just inject proxy which will store the setup. It can delegate after fragment's construction to setup() / advanced().

          • 2. Re: Page Fragments refactoring
            ppitonak

            (1) I like the AdvancedInteractions pattern. Lukas, you need to realize that there are two types of users who will use RichFaces page fragments: RichFaces team and RichFaces users. RichFaces team needs to have access to all parts of a component so that we can test it thoroughly. On the other hand, there are RichFaces users who can believe us that a component works correctly and they don't need to test component itself. What they are interested in is their business logic.

             

            RichFaces framework tests will be full of calls to method advanced(). RichFaces users probably won't have to use advanced functionality often. Another solution (which we have now) is to use "internal" package and have two versions of each page fragment - one for RichFaces team and one for users. However, QE team finds it much simpler to implement AdvancedInteractions pattern.

             

            (2) I agree that we should keep such an  API in mind.

             

            (3) I think that hiding complexity is a good thing here. I agree with Juraj that we should provide setters for all attributes that are set automatically.

            • 3. Re: Page Fragments refactoring
              lfryc

              (1) That's completely valid argument, but I think providing different interfaces do the best job with limiting API for particular users.

               

              (3) We need to keep in mind that even though the Page Fragments conceptually helps people leverage higher level API than WebDriver, people still want to do assertion

              of behavior:

               

              Use case:

              say my component supports "server" and "ajax" modes of updating.

              I use "ajax", but later I switch to "server".

              My test continues to work, because the fragment automatically loads configuration from widget setup.

              However in my case, the page functionality breaks, because it was written in a way to keep the state between multiple AJAX requests and the component now reloads in "server" mode.

               

              That's the situation where too clever page fragments might cause tests to not recognize failure.

               

              I'm not saying it's necessarily evil, just trying to open discussion. :-)

              • 4. Re: Page Fragments refactoring
                jhuska

                (1) IMHO both solutions can cause problems and we should consider which one has more advantages over the other one. And to choose one which is more easier to use, not implement.

                 

                Multiple Interfaces (pros) or AdvancedInteractions (cons):

                1. Tests will not be polluted with .advanced() method calls
                2. Just guess that it is more standard solution for this purpose

                 

                Multiple Interfaces (cons) of AdvancedInteractions (pros):

                1. The more interfaces are there, the more missleading it can be
                2. A need to use method from such interface would mean to inject such Page Fragment even when you need to use such method only once (there are workarounds, but IMHO most of the people would do it in this way)
                3. An user would need to remember what method is in which interface. Or he would need to look into documentation. When using advanced() you just can use autocompletition to advise you what methods you can use.

                 

                I personally like AdvancedInteractions more, but only because I find it a nicer pattern, which gives me better flexibility in what I want to do with the Page Fragment. But I am junior Please correct me where I am wrong or add your pros and cons.

                 

                (2) Lukas, I do not know If I understand your use case correctly but IMHO: If the test continue to work (is passing) while the page functionality is broken, then it is a poorly developed test, and the setup of the page fragment does not matter.

                • 5. Re: Page Fragments refactoring
                  lfryc

                  (1) I'm fine with advanced() as long as no one has any other objections against it.

                   

                  (3) Simpler: Usually the test drives the component under test:

                   

                  test ---> component

                   

                  but what you want to introduce is configuration of a test driven by the state of component under test:

                   

                  test <---> component

                   

                  ----

                   

                  As we have discussed it, it forms the unheatly bi-directional dependency between the test and the component,

                   

                  and I guess it depends on particular use case if it can make tests undeterministic.

                   

                  On the level of page fragments in might make sense, but that's the question for wider discussion.

                  • 6. Re: Page Fragments refactoring
                    jhuska

                    (3) I have probably found out a case which supports your assumption that automatic setting can be bad.

                     

                    1. suppose that you have autocomplete with tokens=";"

                    2. you want to change it in the application for tokens=", ", however, you accidentally change it with a mistake, lets say you forget there "dot" (tokens=",. ").

                    3. your page fragment for autocomplete will use for separating values the token with that "dot" automatically, and will not catch the bug

                     

                    Do you think that similar cases can be enough for forgetting this way of automatic page fragments setting ?

                    • 7. Re: Page Fragments refactoring
                      lfryc

                      What about PageFragment#advanced().setupFromWidget(); ?

                      • 8. Re: Page Fragments refactoring
                        jhuska

                        For me, that is a very good trade off.

                        • 9. Re: Page Fragments refactoring
                          jhuska

                          So, I was trying to implement the solution with PageFragment#advanced().setupFromWidget(), and I came across an interesting problem.

                          Consider following html code:

                           

                          {code:xml}

                          <form id="myForm">

                            <div id="pageFragmentRoot">

                              <span id="content"></span>

                              <script></script>

                            </div>

                            <script>some custom script</script>

                          </form>

                          {code}

                           

                          The implementation of the Page fragment would use something like: @FindBy(jquery="script:last") to locate the right script, or somethig similar.

                           

                          Nevertheless, it will matter more than previously what root will an user of that page fragment assign to it.

                          E.g. @FindBy(id="myForm") PageFragment would not work as previously. Because we do not put any restriction on what the root locator should look like.

                           

                          This is also a little problem of Graphene I think. We should state somewhere in the docs about how the root should look like. Whether it has to be the first element which identify the fragment or what.

                           

                          For me the solution is:

                          1. put into the contract of the method PageFragment#advanced().setupFromWidget() that the correct root has to be set, according to some rules.
                          2. create a guideline in the Graphen docs, what should be set as a root element. A note that an page fragment author should use id,className or other locating techniques would be good as well.

                           

                          What do you think guys ?

                          • 10. Re: Page Fragments refactoring
                            jhuska

                            Another question uprose.

                             

                            Some of the page fragments depends on the interfaces which are defined in Graphene:

                            https://github.com/arquillian/arquillian-graphene/tree/master/graphene-component-api

                             

                            The question is whether we want this ?

                             

                            That repository was created with the following vision: to create a uniform API for components which you can find in all frameworks.

                             

                            Now when we developed some page fragments I found this endeavor as not feasible. It is very difficult to define such API, because each framework is quite different and I can not see a chance that someone from other framework will depend on that API.

                             

                            What do you think ?

                             

                            Lukas, do we continue with that sub project of Graphene ? What advantage do you see of doing so ? IMHO richfaces page fragments should have their interfaces defined in that same repository as the implementation.

                            • 11. Re: Page Fragments refactoring
                              lfryc

                              You can locate a last script which is a direct child of the root:

                               

                              @FindBy(jquery = "> script:last-child")
                              

                               

                              http://quirksmode.org/css/selectors/firstchild.html

                              http://api.jquery.com/last-child-selector/

                               

                              ----

                               

                              Btw I guess you are trying to parse the contents of <script> element?

                               

                              In RichFaces 4, you can retrieve some properties directly from component markup.

                              It means getting the handle of component:

                               

                              RichFaces.$(document.getElementById('id')).component;
                              

                               

                              and call the component's JavaScript API method.

                               

                              ----

                               

                              Howevr in RichFaces 5, it will be even simpler, since all the component options are exposed as options following jQuery UI Widget Factory, e.g.:

                               

                              $('#id').richAutocomplete('option', 'tokens');
                              
                              • 12. Re: Page Fragments refactoring
                                lfryc

                                I have already expressed such concerns back few months.

                                 

                                It's not feasable to diverge such an API when even first implementation haven't be released for public use.

                                 

                                +1 for removing it at this point.

                                Juraj Húska wrote:

                                 

                                The question is whether we want this ?

                                 

                                That repository was created with the following vision: to create a uniform API for components which you can find in all frameworks.

                                 

                                Now when we developed some page fragments I found this endeavor as not feasible. It is very difficult to define such API, because each framework is quite different and I can not see a chance that someone from other framework will depend on that API.

                                 

                                What do you think ?

                                 

                                Lukas, do we continue with that sub project of Graphene ? What advantage do you see of doing so ? IMHO richfaces page fragments should have their interfaces defined in that same repository as the implementation.

                                • 13. Re: Page Fragments refactoring
                                  lfryc

                                  Lukáš Fryč wrote:

                                   

                                  Hey Juraj, Jiri,

                                   

                                  (2) Remember that oeople should be able to simply expose direct methods for retrieving particular content - e.g.:

                                   

                                  UserDetails details = tabPanel.switchToUserDetails(); // cals switchTo(1).getContent(UserDetails.class) underneath
                                  ContactInformation contact = tabPanel.switchToContactInformation(); // calls switchTo(2).getContent(ContactInformation);

                                   

                                  FYI To illustrate this ^ idea, we have created this gist:

                                   

                                  https://gist.github.com/lfryc/6120060

                                  • 14. Re: Page Fragments refactoring
                                    jhuska

                                    Sorry for lot of questions, but I want Page Fragments to be tip top

                                     

                                    I am a little bit hesitating with using of GrapheneElement vs. WebElement, because:

                                     

                                    • it is OK to cast GrapheneElement to WebElement
                                    • but the problem start when working with List<WebElement> and List<GrapheneElement>, there is no way to cast it in type safe manner.
                                    • therefore it would force a little bit users to use GrapheneElement IMHO, which I suppose as an optional way, IMHO we should promote the standard way from WebDriver as much as possible, to provide people well know concepts

                                     

                                    What do you think guys, can we use GrapheneElement accross the Page Fragments ?

                                     

                                    The solution can be:

                                    • provide in Graphene something like Graphene.convertWebElementList() or similar
                                    • or a way how to obtain List<GrapheneElement>, currently it can be only injected
                                    • or not to use GrapheneElement in collections etc.
                                    1 2 Previous Next