rafaelweingartner commented on a change in pull request #2293: 
CLOUDSTACK-10047: DVSwitch fixes and improvements
URL: https://github.com/apache/cloudstack/pull/2293#discussion_r144262446
 
 

 ##########
 File path: utils/src/test/java/com/cloud/utils/UriUtilsTest.java
 ##########
 @@ -57,4 +60,45 @@ public void getUpdateUri() {
         //XXX: Interesting cases not covered:
         // * port is ignored and left out from the return value
     }
+
+    @Test
+    public void expandVlanEmpty() {
+        List<Integer> vlans = UriUtils.expandVlanUri("");
+        Assert.assertTrue(vlans.size() == 0);
+    }
+
+    @Test
+    public void expandVlanSingleValue() {
+        List<Integer> vlans = UriUtils.expandVlanUri("10");
+        Assert.assertTrue(vlans.size() == 1);
+        Assert.assertEquals(vlans, Collections.singletonList(10));
+    }
+
+    @Test
+    public void expandVlanValidRange() {
+        List<Integer> vlans = UriUtils.expandVlanUri("10-12,14,17,40-43");
+        Assert.assertEquals(vlans, Arrays.asList(10,11,12,14,17,40,41,42,43));
+    }
+
+    @Test
+    public void expandVlanInvalidRange() {
+        List<Integer> vlans = UriUtils.expandVlanUri("10-,12-14,-4,5-2");
+        Assert.assertEquals(vlans, Arrays.asList(10,12,13,14));
+    }
+
+    @Test
+    public void testVlanUriOverlap() {
+        Assert.assertTrue(UriUtils.checkVlanUriOverlap("10-30,45,50,12,31", 
"10"));
+        Assert.assertTrue(UriUtils.checkVlanUriOverlap("10-30,45,50,12,31", 
"32,33-44,30-31"));
+        Assert.assertTrue(UriUtils.checkVlanUriOverlap("10-30", "25-35"));
+    }
+
+    @Test
+    public void testVlanUriNoOverlap() {
+        Assert.assertFalse(UriUtils.checkVlanUriOverlap("10-30,45,50,12,31", 
null));
+        Assert.assertFalse(UriUtils.checkVlanUriOverlap("10-30,45,50,12,31", 
""));
+        Assert.assertFalse(UriUtils.checkVlanUriOverlap("10-30,45,50,12,31", 
"32"));
+        Assert.assertFalse(UriUtils.checkVlanUriOverlap("10,22,111", "12"));
+        Assert.assertFalse(UriUtils.checkVlanUriOverlap("100-200", 
"30-40,50,201-250"));
 
 Review comment:
   I had already seen these tests, I did not say anything because in my option 
they are properly written ;)
   
   So, about that `null` case, in my option it is already covered, if for some 
reason someone alters the method to return null, your test cases will catch it 
(this is great!). That is why I was saying you do not need those null checks.
 
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to