Codegass opened a new issue, #6659:
URL: https://github.com/apache/cloudstack/issues/6659

   ##### ISSUE TYPE
   * Improvement Request
   ##### COMPONENT NAME
   unit-test
   ##### OS / ENVIRONMENT
   N/A
   ##### SUMMARY
   When I am running the test for 
[checkStrictModeWithCurrentAccountVmsPresent](https://github.com/Codegass/cloudstack/blob/71056191f2bdad4be1a7eaf9bb73a7dcee3516f2/plugins/deployment-planners/implicit-dedication/src/test/java/org/apache/cloudstack/implicitplanner/ImplicitPlannerTest.java#L199)
 I noticed that the logic of the assertion in test case is to check that all 
the items in the clusterList are equal to one. 
   
   This is currently expressed by the combination of a for loop and an if-else 
block, see [line 214 to line 
221](https://github.com/Codegass/cloudstack/blob/71056191f2bdad4be1a7eaf9bb73a7dcee3516f2/plugins/deployment-planners/implicit-dedication/src/test/java/org/apache/cloudstack/implicitplanner/ImplicitPlannerTest.java#L214-L221).
 List code below: The assertion logic is not explicit by reading the current 
code. 
   
   ```java
   ...
           boolean foundNeededCluster = false;
           for (Long cluster : clusterList) {
               if (cluster != 1) {
                   fail("Found a cluster that shouldn't have been present, 
cluster id : " + cluster);
               } else {
                   foundNeededCluster = true;
               }
           }
           assertTrue("Didn't find cluster 1 in the list. It should have been 
present", foundNeededCluster);
   ...
   ```
   
   This can be simplified into one line of code using hamcrest API, which make 
the assertion logic straightforward and explit:
   
   After refactoring
   ```java
   import org.hamcrest.Matchers.everyItem;
   import org.hamcrest.Matchers.equalTo;
   ...
        assertThat(clusterList, everyItem(equalTo(1));
   ...
   ```
   
   ###### Rationale
   Make the code clearer and easier to understand with hamcrest collection 
checker.
   
   ###### Operational impact
   
    No impacts to operations; only changes to test code.
   


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to