-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14451/#review26660
-----------------------------------------------------------



utils/src/com/cloud/utils/net/NetUtils.java
<https://reviews.apache.org/r/14451/#comment51942>

    I have created https://issues.apache.org/jira/browse/CLOUDSTACK-4807. You 
could assign the bug to yourself and resolve it when your patch is merged.
    Just add the bug id in the bug section in the review board, also add the 
branch(master) in the review board, it helps people applying the patch.



utils/src/com/cloud/utils/net/NetUtils.java
<https://reviews.apache.org/r/14451/#comment51944>

    Thanks for the explanation.
    Can you keep this variable with all other static variables at the beginning 
of the class and not at this place.



utils/test/com/cloud/utils/net/NetUtilsTest.java
<https://reviews.apache.org/r/14451/#comment51947>

    What I am suggesting is that add a method something like
    boolean isEqual(var1, var2) {
            if var1 == var2 
                return true
            return false
    }
    
    Now you can add cases like
    assertFalse(isEqual(15, NetUtils.mac2Long("00:00:00:00:00:ff")))
    
    This is just one implementation as there is no assertNotEquals() method 
provided by Junit. You could certainly add better implementations. 
    We generally use assertFalse for boolean methods.
    You can have a look at testIsSameIpRange():
     // Check for 2 different CIDRs and different IP Range        
assertFalse(NetUtils.isSameIpRange(cidrFirst, cidrThird));
    
    // Check for Incorrect format of CIDR        
assertFalse(NetUtils.isSameIpRange(cidrFirst, "10.3.6.5/50"));
    
    Just a few such cases will suffice.
    


- Saksham Srivastava


On Oct. 2, 2013, 9:19 p.m., Laszlo Hornyak wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14451/
> -----------------------------------------------------------
> 
> (Updated Oct. 2, 2013, 9:19 p.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> - tests added for mac2Long, byte2Mac, long2Mac, ip2Long and long2Ip
> - isWindows static method removed - it was not used
> - minor changes in the implementations of the tested methods
> 
> 
> Diffs
> -----
> 
>   utils/src/com/cloud/utils/net/NetUtils.java 1e72e22 
>   utils/test/com/cloud/utils/net/NetUtilsTest.java 3cfc98f 
> 
> Diff: https://reviews.apache.org/r/14451/diff/
> 
> 
> Testing
> -------
> 
> yes
> 
> 
> Thanks,
> 
> Laszlo Hornyak
> 
>

Reply via email to