> On Oct. 4, 2013, 6:43 a.m., Saksham Srivastava wrote: > > utils/src/com/cloud/utils/net/NetUtils.java, line 149 > > <https://reviews.apache.org/r/14451/diff/1/?file=360597#file360597line149> > > > > 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.
Can't assign, I need some permission in jira, but the bug id is set in reviewboard. Thx! > On Oct. 4, 2013, 6:43 a.m., Saksham Srivastava wrote: > > utils/src/com/cloud/utils/net/NetUtils.java, line 366 > > <https://reviews.apache.org/r/14451/diff/1/?file=360597#file360597line366> > > > > 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. sure, I will send an update. > On Oct. 4, 2013, 6:43 a.m., Saksham Srivastava wrote: > > utils/test/com/cloud/utils/net/NetUtilsTest.java, line 225 > > <https://reviews.apache.org/r/14451/diff/1/?file=360598#file360598line225> > > > > 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. > > Ok... what I do not quite understand is what the purpose of this negative test is. There is already test for mac2Long("...") must be equal long <value>, so a negative test seems to be a test that proves "15 != 16 or any other long value, 15 only equals to 15" :) - Laszlo ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14451/#review26660 ----------------------------------------------------------- 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 > >
