[ 
https://issues.apache.org/jira/browse/TRINIDAD-2510?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14160868#comment-14160868
 ] 

Andy Schwartz commented on TRINIDAD-2510:
-----------------------------------------

Thanks for the patch.  I see that this introduces a new protected (final) 
method:

  /**
   * Performs a visit of all style sheet nodes in the specified test
   * skin. This overloaded method is useful in extending SkinTestCase to
   * create more test cases since TestSkin does not provide all the skins
   * that test needs.
   */
  protected final void visitStyleSheets(String skinResourcePath, 
StyleSheetVisitor visitor)
  {
    StyleSheetDocument document = SkinTestUtils.parseSkin(skinResourcePath);
    
StyleSheetVisitUtils.visitStyleSheets(document.getStyleSheetsAsCollection(), 
visitor);
  }

That is an overload of this protected method that is already present:

  /**
   * Performs a visit of all style sheet nodes in the specified test
   * skin
   */
  protected void visitStyleSheets(TestSkin testSkin, StyleSheetVisitor visitor)
  {
    StyleSheetDocument document = getSkinDocument(testSkin);
    
StyleSheetVisitUtils.visitStyleSheets(document.getStyleSheetsAsCollection(), 
visitor);
  }

The new protected method that takes the String resource path looks like the 
more primitive method, and thus would make more sense as the non-final hook.

What do you think about doing this:

1.  Change the old TestSkin-based method such that it looks up its resource 
path and then calls the new String-based method with this value.
2.  Make the old TestSkin-based method final.
3.  Make the new String-based method non-final.

This also has the nice side effect of avoiding the duplicate call to 
StyleSheetVisitUtils.visitStyleSheets().

> make SkinTestCase more extendable
> ---------------------------------
>
>                 Key: TRINIDAD-2510
>                 URL: https://issues.apache.org/jira/browse/TRINIDAD-2510
>             Project: MyFaces Trinidad
>          Issue Type: Improvement
>          Components: Skinning
>    Affects Versions: 2.1.0-core
>            Reporter: Anand V Nath
>            Priority: Minor
>         Attachments: jira-2510-modified.patch, jira-2510.patch
>
>
> SkinTestCase which is part of trinidad-impl/test, works with TestSkin enum 
> which does not offer flexibility. The change proposed is to add an overloaded 
> method for visiting skins where the skin resource path can be passed as a 
> String. This will enable test cases extending SkinTestCase to work with any 
> skin of their choice.
> The proposed change is to add this method in SkinTestCase:
>   /**
>    * Performs a visit of all style sheet nodes in the specified test
>    * skin. This overloaded method is useful in extending SkinTestCase to
>    * create more test cases since TestSkin does not provide all the skins
>    * that test needs.
>    */
>   protected final void visitStyleSheets(String skinResourcePath, 
> StyleSheetVisitor visitor)
>   {
>     StyleSheetDocument document = SkinTestUtils.parseSkin(skinResourcePath);
>     
> StyleSheetVisitUtils.visitStyleSheets(document.getStyleSheetsAsCollection(), 
> visitor);
>   }



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to