NetUtils testing - Refactor tests: - Upgrade tests to use jUnit4 - Add hamcrest dependency (contribution of by Laszio Hornyak) - Break big tests in small unit tests - Replace assertTrue/False with complex conditions by assertThat with specific matchers - Remove dead code: - Private static method never called locally - Add test for method that validates cidrs
Signed-off-by: Laszlo Hornyak <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/ef547108 Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/ef547108 Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/ef547108 Branch: refs/heads/acl-item-cidrs Commit: ef547108dde9257422ac66a3de5868c93890968c Parents: 169cd6f Author: miguelaferreira <[email protected]> Authored: Fri Feb 21 17:18:23 2014 +0100 Committer: Laszlo Hornyak <[email protected]> Committed: Tue Feb 25 19:23:13 2014 +0100 ---------------------------------------------------------------------- pom.xml | 10 +- utils/src/com/cloud/utils/net/NetUtils.java | 15 +- .../test/com/cloud/utils/net/NetUtilsTest.java | 227 +++++++++++++------ 3 files changed, 168 insertions(+), 84 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/ef547108/pom.xml ---------------------------------------------------------------------- diff --git a/pom.xml b/pom.xml index c0b8219..02b1808 100644 --- a/pom.xml +++ b/pom.xml @@ -49,7 +49,9 @@ <cs.logging.version>1.1.1</cs.logging.version> <cs.discovery.version>0.5</cs.discovery.version> <cs.ejb.version>3.0</cs.ejb.version> - <cs.junit.version>4.10</cs.junit.version> + <!-- do not forget to also upgrade hamcrest library with junit --> + <cs.junit.version>4.11</cs.junit.version> + <cs.hamcrest.version>1.3</cs.hamcrest.version> <cs.bcprov.version>1.46</cs.bcprov.version> <cs.jsch.version>0.1.42</cs.jsch.version> <cs.jpa.version>2.0.0</cs.jpa.version> @@ -414,6 +416,12 @@ <scope>test</scope> </dependency> <dependency> + <groupId>org.hamcrest</groupId> + <artifactId>hamcrest-library</artifactId> + <version>${cs.hamcrest.version}</version> + <scope>test</scope> + </dependency> + <dependency> <groupId>org.mockito</groupId> <artifactId>mockito-all</artifactId> <version>1.9.5</version> http://git-wip-us.apache.org/repos/asf/cloudstack/blob/ef547108/utils/src/com/cloud/utils/net/NetUtils.java ---------------------------------------------------------------------- diff --git a/utils/src/com/cloud/utils/net/NetUtils.java b/utils/src/com/cloud/utils/net/NetUtils.java index c22e39a..6350986 100755 --- a/utils/src/com/cloud/utils/net/NetUtils.java +++ b/utils/src/com/cloud/utils/net/NetUtils.java @@ -44,13 +44,12 @@ import org.apache.commons.lang.SystemUtils; import org.apache.commons.net.util.SubnetUtils; import org.apache.log4j.Logger; -import com.googlecode.ipv6.IPv6Address; -import com.googlecode.ipv6.IPv6AddressRange; -import com.googlecode.ipv6.IPv6Network; - import com.cloud.utils.IteratorUtil; import com.cloud.utils.Pair; import com.cloud.utils.script.Script; +import com.googlecode.ipv6.IPv6Address; +import com.googlecode.ipv6.IPv6AddressRange; +import com.googlecode.ipv6.IPv6Network; public class NetUtils { protected final static Logger s_logger = Logger.getLogger(NetUtils.class); @@ -158,14 +157,6 @@ public class NetUtils { return cidrList.toArray(new String[0]); } - private static boolean isWindows() { - String os = System.getProperty("os.name"); - if (os != null && os.startsWith("Windows")) - return true; - - return false; - } - public static String getDefaultHostIp() { if (SystemUtils.IS_OS_WINDOWS) { Pattern pattern = Pattern.compile("\\s*0.0.0.0\\s*0.0.0.0\\s*(\\S*)\\s*(\\S*)\\s*"); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/ef547108/utils/test/com/cloud/utils/net/NetUtilsTest.java ---------------------------------------------------------------------- diff --git a/utils/test/com/cloud/utils/net/NetUtilsTest.java b/utils/test/com/cloud/utils/net/NetUtilsTest.java index d3e283c..3183826 100644 --- a/utils/test/com/cloud/utils/net/NetUtilsTest.java +++ b/utils/test/com/cloud/utils/net/NetUtilsTest.java @@ -19,54 +19,90 @@ package com.cloud.utils.net; +import static org.hamcrest.Matchers.anyOf; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.lessThanOrEqualTo; +import static org.hamcrest.Matchers.not; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; + import java.math.BigInteger; -import java.net.URI; import java.util.SortedSet; import java.util.TreeSet; -import junit.framework.TestCase; - import org.apache.log4j.Logger; import org.junit.Test; import com.googlecode.ipv6.IPv6Address; -public class NetUtilsTest extends TestCase { +public class NetUtilsTest { private static final Logger s_logger = Logger.getLogger(NetUtilsTest.class); @Test - public void testGetRandomIpFromCidr() { + public void testGetRandomIpFromCidrWithSize24() throws Exception { + String cidr = "192.168.124.1"; + int size = 24; + int netCharacters = 12; + + long ip = NetUtils.getRandomIpFromCidr(cidr, size, new TreeSet<Long>()); + + assertThat("The ip " + NetUtils.long2Ip(ip) + " retrieved must be within the cidr " + cidr + "/" + size, cidr.substring(0, netCharacters), equalTo(NetUtils.long2Ip(ip) + .substring(0, netCharacters))); + } + + @Test + public void testGetRandomIpFromCidrWithSize16() throws Exception { + String cidr = "192.168.124.1"; + int size = 16; + int netCharacters = 8; + + long ip = NetUtils.getRandomIpFromCidr(cidr, 16, new TreeSet<Long>()); + + assertThat("The ip " + NetUtils.long2Ip(ip) + " retrieved must be within the cidr " + cidr + "/" + size, cidr.substring(0, netCharacters), equalTo(NetUtils.long2Ip(ip) + .substring(0, netCharacters))); + } + + @Test + public void testGetRandomIpFromCidrWithSize8() throws Exception { String cidr = "192.168.124.1"; - long ip = NetUtils.getRandomIpFromCidr(cidr, 24, new TreeSet<Long>()); - assertEquals("The ip " + NetUtils.long2Ip(ip) + " retrieved must be within the cidr " + cidr + "/24", cidr.substring(0, 12), NetUtils.long2Ip(ip) - .substring(0, 12)); + int size = 8; + int netCharacters = 4; - ip = NetUtils.getRandomIpFromCidr(cidr, 16, new TreeSet<Long>()); - assertEquals("The ip " + NetUtils.long2Ip(ip) + " retrieved must be within the cidr " + cidr + "/16", cidr.substring(0, 8), NetUtils.long2Ip(ip).substring(0, 8)); + long ip = NetUtils.getRandomIpFromCidr(cidr, 16, new TreeSet<Long>()); - ip = NetUtils.getRandomIpFromCidr(cidr, 8, new TreeSet<Long>()); - assertEquals("The ip " + NetUtils.long2Ip(ip) + " retrieved must be within the cidr " + cidr + "/8", cidr.substring(0, 4), NetUtils.long2Ip(ip).substring(0, 4)); + assertThat("The ip " + NetUtils.long2Ip(ip) + " retrieved must be within the cidr " + cidr + "/" + size, cidr.substring(0, netCharacters), equalTo(NetUtils.long2Ip(ip) + .substring(0, netCharacters))); + } + + @Test + public void testGetRandomIpFromCidrUsignAvoid() throws Exception { + String cidr = "192.168.124.1"; + int size = 30; SortedSet<Long> avoid = new TreeSet<Long>(); - ip = NetUtils.getRandomIpFromCidr(cidr, 30, avoid); - assertTrue("We should be able to retrieve an ip on the first call.", ip != -1); + long ip = NetUtils.getRandomIpFromCidr(cidr, size, avoid); + assertThat("We should be able to retrieve an ip on the first call.", ip, not(equalTo(-1L))); avoid.add(ip); - ip = NetUtils.getRandomIpFromCidr(cidr, 30, avoid); - assertTrue("We should be able to retrieve an ip on the second call.", ip != -1); - assertTrue("ip returned is not in the avoid list", !avoid.contains(ip)); + ip = NetUtils.getRandomIpFromCidr(cidr, size, avoid); + assertThat("We should be able to retrieve an ip on the second call.", ip, not(equalTo(-1L))); + assertThat("ip returned is not in the avoid list", avoid, not(contains(ip))); avoid.add(ip); - ip = NetUtils.getRandomIpFromCidr(cidr, 30, avoid); - assertTrue("We should be able to retrieve an ip on the third call.", ip != -1); - assertTrue("ip returned is not in the avoid list", !avoid.contains(ip)); + ip = NetUtils.getRandomIpFromCidr(cidr, size, avoid); + assertThat("We should be able to retrieve an ip on the third call.", ip, not(equalTo(-1L))); + assertThat("ip returned is not in the avoid list", avoid, not(contains(ip))); avoid.add(ip); - - ip = NetUtils.getRandomIpFromCidr(cidr, 30, avoid); + ip = NetUtils.getRandomIpFromCidr(cidr, size, avoid); assertEquals("This should be -1 because we ran out of ip addresses: " + ip, ip, -1); } @Test - public void testVpnPolicy() { + public void testIsValidS2SVpnPolicy() { assertTrue(NetUtils.isValidS2SVpnPolicy("aes128-sha1")); assertTrue(NetUtils.isValidS2SVpnPolicy("3des-sha1")); assertTrue(NetUtils.isValidS2SVpnPolicy("3des-sha1,aes256-sha1")); @@ -81,22 +117,12 @@ public class NetUtilsTest extends TestCase { assertFalse(NetUtils.isValidS2SVpnPolicy(",aes;modp1536,,,")); } - public void testIpv6() { - assertTrue(NetUtils.isValidIpv6("fc00::1")); - assertFalse(NetUtils.isValidIpv6("")); - assertFalse(NetUtils.isValidIpv6(null)); - assertFalse(NetUtils.isValidIpv6("1234:5678::1/64")); - assertTrue(NetUtils.isValidIp6Cidr("1234:5678::1/64")); - assertFalse(NetUtils.isValidIp6Cidr("1234:5678::1")); - assertEquals(NetUtils.getIp6CidrSize("1234:5678::1/32"), 32); - assertEquals(NetUtils.getIp6CidrSize("1234:5678::1"), 0); - BigInteger two = new BigInteger("2"); - assertEquals(NetUtils.countIp6InRange("1234:5678::1-1234:5678::2"), two); - assertEquals(NetUtils.countIp6InRange("1234:5678::2-1234:5678::0"), null); + @Test + public void testGetIp6FromRange() { assertEquals(NetUtils.getIp6FromRange("1234:5678::1-1234:5678::1"), "1234:5678::1"); for (int i = 0; i < 5; i++) { String ip = NetUtils.getIp6FromRange("1234:5678::1-1234:5678::2"); - assertTrue(ip.equals("1234:5678::1") || ip.equals("1234:5678::2")); + assertThat(ip, anyOf(equalTo("1234:5678::1"), equalTo("1234:5678::2"))); s_logger.info("IP is " + ip); } String ipString = null; @@ -106,49 +132,104 @@ public class NetUtilsTest extends TestCase { ipString = NetUtils.getIp6FromRange(ipStart.toString() + "-" + ipEnd.toString()); s_logger.info("IP is " + ipString); IPv6Address ip = IPv6Address.fromString(ipString); - assertTrue(ip.compareTo(ipStart) >= 0); - assertTrue(ip.compareTo(ipEnd) <= 0); + assertThat(ip, greaterThanOrEqualTo(ipStart)); + assertThat(ip, lessThanOrEqualTo(ipEnd)); } - //Test isIp6RangeOverlap - assertFalse(NetUtils.isIp6RangeOverlap("1234:5678::1-1234:5678::ffff", "1234:5678:1::1-1234:5678:1::ffff")); - assertTrue(NetUtils.isIp6RangeOverlap("1234:5678::1-1234:5678::ffff", "1234:5678::2-1234:5678::f")); - assertTrue(NetUtils.isIp6RangeOverlap("1234:5678::f-1234:5678::ffff", "1234:5678::2-1234:5678::f")); - assertFalse(NetUtils.isIp6RangeOverlap("1234:5678::f-1234:5678::ffff", "1234:5678::2-1234:5678::e")); - assertFalse(NetUtils.isIp6RangeOverlap("1234:5678::f-1234:5678::f", "1234:5678::2-1234:5678::e")); - //Test getNextIp6InRange + } + + @Test + public void testCountIp6InRange() { + assertEquals(NetUtils.countIp6InRange("1234:5678::1-1234:5678::2"), new BigInteger("2")); + assertEquals(NetUtils.countIp6InRange("1234:5678::2-1234:5678::0"), null); + } + + @Test + public void testGetIp6CidrSize() { + assertEquals(NetUtils.getIp6CidrSize("1234:5678::1/32"), 32); + assertEquals(NetUtils.getIp6CidrSize("1234:5678::1"), 0); + } + + @Test + public void testIsValidIp6Cidr() { + assertTrue(NetUtils.isValidIp6Cidr("1234:5678::1/64")); + assertFalse(NetUtils.isValidIp6Cidr("1234:5678::1")); + } + + @Test + public void testIsValidIpv6() { + assertTrue(NetUtils.isValidIpv6("fc00::1")); + assertFalse(NetUtils.isValidIpv6("")); + assertFalse(NetUtils.isValidIpv6(null)); + assertFalse(NetUtils.isValidIpv6("1234:5678::1/64")); + } + + @Test + public void testIsIp6InRange() { + assertTrue(NetUtils.isIp6InRange("1234:5678:abcd::1", "1234:5678:abcd::1-1234:5678:abcd::1")); + assertFalse(NetUtils.isIp6InRange("1234:5678:abcd::1", "1234:5678:abcd::2-1234:5678:abcd::1")); + assertFalse(NetUtils.isIp6InRange("1234:5678:abcd::1", null)); + assertTrue(NetUtils.isIp6InRange("1234:5678:abcd::1", "1234:5678::1-1234:5679::1")); + } + + @Test + public void testIsIp6InNetwork() { + assertFalse(NetUtils.isIp6InNetwork("1234:5678:abcd::1", "1234:5678::/64")); + assertTrue(NetUtils.isIp6InNetwork("1234:5678::1", "1234:5678::/64")); + assertTrue(NetUtils.isIp6InNetwork("1234:5678::ffff:ffff:ffff:ffff", "1234:5678::/64")); + assertTrue(NetUtils.isIp6InNetwork("1234:5678::", "1234:5678::/64")); + } + + @Test + public void testGetNextIp6InRange() { String range = "1234:5678::1-1234:5678::8000:0000"; assertEquals(NetUtils.getNextIp6InRange("1234:5678::8000:0", range), "1234:5678::1"); assertEquals(NetUtils.getNextIp6InRange("1234:5678::7fff:ffff", range), "1234:5678::8000:0"); assertEquals(NetUtils.getNextIp6InRange("1234:5678::1", range), "1234:5678::2"); range = "1234:5678::1-1234:5678::ffff:ffff:ffff:ffff"; assertEquals(NetUtils.getNextIp6InRange("1234:5678::ffff:ffff:ffff:ffff", range), "1234:5678::1"); - //Test isIp6InNetwork - assertFalse(NetUtils.isIp6InNetwork("1234:5678:abcd::1", "1234:5678::/64")); - assertTrue(NetUtils.isIp6InNetwork("1234:5678::1", "1234:5678::/64")); - assertTrue(NetUtils.isIp6InNetwork("1234:5678::ffff:ffff:ffff:ffff", "1234:5678::/64")); - assertTrue(NetUtils.isIp6InNetwork("1234:5678::", "1234:5678::/64")); - //Test isIp6InRange - assertTrue(NetUtils.isIp6InRange("1234:5678:abcd::1", "1234:5678:abcd::1-1234:5678:abcd::1")); - assertFalse(NetUtils.isIp6InRange("1234:5678:abcd::1", "1234:5678:abcd::2-1234:5678:abcd::1")); - assertFalse(NetUtils.isIp6InRange("1234:5678:abcd::1", null)); - assertTrue(NetUtils.isIp6InRange("1234:5678:abcd::1", "1234:5678::1-1234:5679::1")); } - public void testPvlan() { - URI uri = NetUtils.generateUriForPvlan("123", "456"); - assertTrue(uri.toString().equals("pvlan://123-i456")); - assertTrue(NetUtils.getPrimaryPvlanFromUri(uri).equals("123")); - assertTrue(NetUtils.getIsolatedPvlanFromUri(uri).equals("456")); + @Test + public void testIsIp6RangeOverlap() { + assertFalse(NetUtils.isIp6RangeOverlap("1234:5678::1-1234:5678::ffff", "1234:5678:1::1-1234:5678:1::ffff")); + assertTrue(NetUtils.isIp6RangeOverlap("1234:5678::1-1234:5678::ffff", "1234:5678::2-1234:5678::f")); + assertTrue(NetUtils.isIp6RangeOverlap("1234:5678::f-1234:5678::ffff", "1234:5678::2-1234:5678::f")); + assertFalse(NetUtils.isIp6RangeOverlap("1234:5678::f-1234:5678::ffff", "1234:5678::2-1234:5678::e")); + assertFalse(NetUtils.isIp6RangeOverlap("1234:5678::f-1234:5678::f", "1234:5678::2-1234:5678::e")); } - public void testIsSameIpRange() { - //Test to check IP Range of 2 CIDRs + @Test + public void testGenerateUriForPvlan() { + assertEquals("pvlan://123-i456", NetUtils.generateUriForPvlan("123", "456").toString()); + } + + @Test + public void testGetPrimaryPvlanFromUri() { + assertEquals("123", NetUtils.getPrimaryPvlanFromUri(NetUtils.generateUriForPvlan("123", "456"))); + } + + @Test + public void testGetIsolatedPvlanFromUri() { + assertEquals("456", NetUtils.getIsolatedPvlanFromUri(NetUtils.generateUriForPvlan("123", "456"))); + } + + @Test + public void testIsValidCIDR() throws Exception { + //Test to check IP Range of 2 CIDR String cidrFirst = "10.0.144.0/20"; String cidrSecond = "10.0.151.0/20"; String cidrThird = "10.0.144.0/21"; + assertTrue(NetUtils.isValidCIDR(cidrFirst)); assertTrue(NetUtils.isValidCIDR(cidrSecond)); assertTrue(NetUtils.isValidCIDR(cidrThird)); + } + + @Test + public void testIsSameIpRange() { + String cidrFirst = "10.0.144.0/20"; + String cidrSecond = "10.0.151.0/20"; + String cidrThird = "10.0.144.0/21"; //Check for exactly same CIDRs assertTrue(NetUtils.isSameIpRange(cidrFirst, cidrFirst)); @@ -160,17 +241,14 @@ public class NetUtilsTest extends TestCase { assertFalse(NetUtils.isSameIpRange(cidrFirst, "10.3.6.5/50")); } - public void testMacGenerateion() { + @Test + public void testGenerateMacOnIncrease() { String mac = "06:01:23:00:45:67"; - String newMac = NetUtils.generateMacOnIncrease(mac, 2); - assertTrue(newMac.equals("06:01:25:00:45:67")); - newMac = NetUtils.generateMacOnIncrease(mac, 16); - assertTrue(newMac.equals("06:01:33:00:45:67")); + assertEquals("06:01:25:00:45:67", NetUtils.generateMacOnIncrease(mac, 2)); + assertEquals("06:01:33:00:45:67", NetUtils.generateMacOnIncrease(mac, 16)); mac = "06:ff:ff:00:45:67"; - newMac = NetUtils.generateMacOnIncrease(mac, 1); - assertTrue(newMac.equals("06:00:00:00:45:67")); - newMac = NetUtils.generateMacOnIncrease(mac, 16); - assertTrue(newMac.equals("06:00:0f:00:45:67")); + assertEquals("06:00:00:00:45:67", NetUtils.generateMacOnIncrease(mac, 1)); + assertEquals("06:00:0f:00:45:67", NetUtils.generateMacOnIncrease(mac, 16)); } @Test @@ -187,4 +265,11 @@ public class NetUtilsTest extends TestCase { assertFalse(NetUtils.isSameIsolationId("2", "vlan://3")); assertFalse(NetUtils.isSameIsolationId("bla", null)); } + + @Test + public void testValidateGuestCidr() throws Exception { + String guestCidr = "192.168.1.0/24"; + + assertTrue(NetUtils.validateGuestCidr(guestCidr)); + } }
