You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2021/09/21 04:01:24 UTC

[GitHub] [cloudstack] wx930910 opened a new issue #5479: Refactor GroupByExtension to improve test logic

wx930910 opened a new issue #5479:
URL: https://github.com/apache/cloudstack/issues/5479


   <!--
   Verify first that your issue/request is not already reported on GitHub.
   Also test if the latest release and main branch are affected too.
   Always add information AFTER of these HTML comments, but no need to delete the comments.
   -->
   
   ##### ISSUE TYPE
   <!-- Pick one below and delete the rest -->
    * Improvement Request
   
   ##### SUMMARY
   <!-- Explain the problem/feature briefly -->
   I noticed that there is a test class [GroupByExtension](https://github.com/apache/cloudstack/blob/bf6266188c89a5487383f216333ae10e878d2c10/framework/db/src/test/java/com/cloud/utils/db/GroupByTest.java#L103) extends production class [GroupBy](https://github.com/apache/cloudstack/blob/bf6266188c89a5487383f216333ae10e878d2c10/framework/db/src/main/java/com/cloud/utils/db/GroupBy.java#L26) to assist testing  [GroupBy.toSql(StringBuilder)](https://github.com/apache/cloudstack/blob/bf6266188c89a5487383f216333ae10e878d2c10/framework/db/src/main/java/com/cloud/utils/db/GroupBy.java#L26). This might not be the best priactice in unit testing and can be improved by leveraging mocking frameworks.
   
   ###### Current Implementation
    * `GroupByExtension` implements `GroupBy` and overrides methods to make `GroupBy.init(Builder)` do nothing.
    * In test case, the child test class is used to test `GroupBy.toSql(StringBuilder)`.
    
   ###### Proposed Implementation
    * Replace `GroupByExtension` with a mocking object created by Mockito.
    * Use method stub to control the behavior of `GroupBy.init(Builder)`.
    * Create a method to return the mocking object for reusing.
   
   ###### Motivation
    * Decouple test class `GroupByExtension` from production class `GroupBy`.
    * Remove the redundant test child class `GroupByExtension`.
   
   ##### More Thought
    * If we only use the spying object once, we can create it inside of the test case to make test logic more explict.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] DaanHoogland removed a comment on issue #5479: Refactor GroupByExtension to improve test logic

Posted by GitBox <gi...@apache.org>.
DaanHoogland removed a comment on issue #5479:
URL: https://github.com/apache/cloudstack/issues/5479#issuecomment-923788201


   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] DaanHoogland commented on issue #5479: Refactor GroupByExtension to improve test logic

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #5479:
URL: https://github.com/apache/cloudstack/issues/5479#issuecomment-923788201


   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] nvazquez closed issue #5479: Refactor GroupByExtension to improve test logic

Posted by GitBox <gi...@apache.org>.
nvazquez closed issue #5479:
URL: https://github.com/apache/cloudstack/issues/5479


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org