You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by GitBox <gi...@apache.org> on 2022/12/22 16:57:47 UTC

[GitHub] [netbeans] ehsavoie commented on a diff in pull request #5115: Added more tests BaseUtilities & Pair classes

ehsavoie commented on code in PR #5115:
URL: https://github.com/apache/netbeans/pull/5115#discussion_r1055658827


##########
platform/openide.util/test/unit/src/org/openide/util/BaseUtilitiesTest.java:
##########
@@ -59,7 +62,7 @@ public void testGetOperatingSystemWinNT () {
     public void testGetOperatingSystemFreebsd () {
         System.setProperty ("os.name", "FreeBSD");
         assertEquals ("System.getProperty (os.name) returns FreeBSD", "FreeBSD", System.getProperty ("os.name"));
-        assertEquals ("System.getProperty (os.name) returns freebsd", "freebsd", System.getProperty ("os.name").toLowerCase (Locale.US));
+        assertEquals ("System.getProperty (os.name) returns freebsd", "freebsd", System.getProperty ("os.name").toLowerCase (US));

Review Comment:
   No real value in this change, this creates noise



##########
platform/openide.util/test/unit/src/org/openide/util/BaseUtilitiesTest.java:
##########
@@ -557,4 +549,102 @@ public void test_toPrimitiveArray_returnsArrayOfPrimitives_whenGivenArrayOfObjec
         assertEquals(0.0, ((double[])result)[0], 0.00001);
         assertEquals(1.0, ((double[])result)[1], 0.00001);
     }
+    //--------------------------------------------------------------------------
+    public void test_getClassName_throwsNullPointer_whenGivenNullArgument() {
+        
+        expectNullPointer(() -> getClassName(null));
+    }
+    //--------------------------------------------------------------------------
+    public void test_getClassName_returnsClassName_forProperInvocation() {
+        
+        assertEquals("java.lang.String", getClassName(String.class));
+        assertEquals("java.lang.String[]", getClassName(String[].class));
+    }
+    //--------------------------------------------------------------------------
+    public void test_getShortClassName_throwsNullPointer_whenGivenNullArgument() {
+        
+        expectNullPointer(() -> getShortClassName(null));
+    }
+    //--------------------------------------------------------------------------
+    public void test_getShortClassName_returnsClassName_forProperInvocation() {
+        
+        assertEquals("String", getShortClassName(String.class));
+        assertEquals("String[]", getShortClassName(String[].class));
+        assertEquals("UnicodeBlock", getShortClassName(Character.UnicodeBlock.class));
+    }
+    //--------------------------------------------------------------------------
+    public void test_pureCalssName_thworsNullPointer_whenGivenNullArgument() {
+        
+        expectNullPointer(() -> pureClassName(null));
+    }
+    //--------------------------------------------------------------------------
+    public void test_pureCalssName__returnsEmptyString_whenGivenEmptyString() {
+        
+        assertEquals("", pureClassName(""));
+    }
+    //--------------------------------------------------------------------------
+    public void test_pureCalssName__returnsBlankString_whenGivenBlankString() {
+        
+        assertEquals(" \t", pureClassName(" \t"));
+    }
+    //--------------------------------------------------------------------------
+    public void test_pureCalssName__returnsClassName_whenGivenFullyQualifiedClassName() {
+        
+        assertEquals("java.lang.String", pureClassName("java.lang.String"));
+        assertEquals("UnicodeBlock", pureClassName("java.lang.Character$UnicodeBlock"));
+    }
+    //--------------------------------------------------------------------------
+    public void test_wrapString_throwsNullPointer_whenGivenNullArgument() {
+
+        expectNullPointer(() -> wrapString(null, 11, this.breakIterator, true));
+
+        //NullPointerException happens only when line breaking actually happenes.
+        //Change second argument below from 1 to 10 and and rerun the test to observe this.
+        //Maybe we should tighten this behavior to throw NPE unconditionally.
+        expectNullPointer(() -> wrapString("ab\ncd", 1, null, true));
+    }
+    //--------------------------------------------------------------------------
+    public void test_wrapString_assumesWidthEqalsOne_whenGivenZeroWidth() {
+
+        assertEquals("a\n\n \n\nb\n", wrapString("a\nb", 0, this.breakIterator, true));
+    }
+    //--------------------------------------------------------------------------
+    public void test_wrapString_assumesWidthEqalsOne_whenGivenNegativeWidth() {
+
+        assertEquals("a\n\n \n\nb\n", wrapString("a\nb", -1, this.breakIterator, true));
+    }
+    //--------------------------------------------------------------------------
+    public void test_wrapString_returnsblankString_whenGivenEmptyString() {
+
+        assertEquals("\n", wrapString("", 10, this.breakIterator, true));
+        assertEquals(" \n", wrapString(" ", 10, this.breakIterator, true));
+        assertEquals(" \n", wrapString("\n", 10, this.breakIterator, true));
+    }
+    //--------------------------------------------------------------------------
+    public void test_wrapString_returnsOneLineWithOriginalString_whenGivenShortString() {
+
+        // String shorter than given width (10).
+        assertEquals("abc def\n", wrapString("abc\ndef", 10, this.breakIterator, true));
+    }
+    //--------------------------------------------------------------------------
+    public void test_wrapString_wrapsLinesEvenInShostStrings_whenRemoveNewLinesEqualsFalse() {
+
+        // String shorter than given width (10).
+        assertEquals("abc\ndef\n", wrapString("abc\ndef", 10, this.breakIterator, false));
+    }
+    //--------------------------------------------------------------------------
+    public void test_wrapString_wrapsString_whenGivenLongString() {
+
+        assertEquals("abc \ndef\n", wrapString("abc\ndef", 5, this.breakIterator, true));
+    }
+    //--------------------------------------------------------------------------
+    private void expectNullPointer(final Runnable code) {

Review Comment:
   Wouldn't a Supplier<T> be more appropriate ? 



##########
platform/openide.util/test/unit/src/org/openide/util/PairTest.java:
##########
@@ -19,39 +19,90 @@
 package org.openide.util;
 
 import org.netbeans.junit.NbTestCase;
-import org.openide.util.Pair;
 
 /**
  *
- * @author Tomas Zezula
+ * @author Tomas Zezula, Lukasz Bownik
  */
 public class PairTest extends NbTestCase {
 
-    public PairTest(String name) {
+    //--------------------------------------------------------------------------
+    public PairTest(final String name) {
+
         super(name);
     }
 
+    //-------------------------------------------------------------------------- 
+    public void test_pairEquality() {
+
+        Pair<Integer, Integer> pair11a = Pair.of(1, 1);
+        Pair<Integer, Integer> pair11b = Pair.of(1, 1);
+        Pair<Integer, Integer> pair12 = Pair.of(1, 2);
+        Pair<Integer, Integer> pairNul1 = Pair.of(null, 1);
+        Pair<Integer, Integer> pair1Null = Pair.of(1, null);
+        Pair<Integer, Integer> pairNullNull = Pair.of(null, null);
+
+        assertTrue(pair11a.equals(pair11a));
+        assertEquals(pair11a.hashCode(), pair11a.hashCode());
+        assertEquals(new Integer(1), pair11a.first());
+        assertEquals(new Integer(1), pair11a.second());
+
+        assertTrue(pair12.equals(pair12));
+        assertEquals(pair12.hashCode(), pair12.hashCode());
+        assertEquals(new Integer(1), pair12.first());
+        assertEquals(new Integer(2), pair12.second());
+
+        assertTrue(pair11a.equals(pair11b));
+        assertEquals(pair11a.hashCode(), pair11b.hashCode());
+        assertEquals(new Integer(1), pair11b.first());
+        assertEquals(new Integer(1), pair11b.second());
+
+        assertTrue(pair1Null.equals(pair1Null));
+        assertEquals(pair1Null.hashCode(), pair1Null.hashCode());
+        assertEquals(new Integer(1), pair1Null.first());
+        assertEquals(null, pair1Null.second());
+
+        assertTrue(pairNul1.equals(pairNul1));
+        assertEquals(pairNul1.hashCode(), pairNul1.hashCode());
+        assertEquals(null, pairNul1.first());
+        assertEquals(new Integer(1), pairNul1.second());
+
+        assertTrue(pairNullNull.equals(pairNullNull));
+        assertEquals(pairNullNull.hashCode(), pairNullNull.hashCode());
+        assertEquals(null, pairNullNull.first());
+        assertEquals(null, pairNullNull.second());
+    }
+
+    //-------------------------------------------------------------------------- 
+    public void test_pairInequality() {

Review Comment:
   Please use camelCase and not snakeCase



##########
platform/openide.util/test/unit/src/org/openide/util/PairTest.java:
##########
@@ -19,39 +19,90 @@
 package org.openide.util;
 
 import org.netbeans.junit.NbTestCase;
-import org.openide.util.Pair;
 
 /**
  *
- * @author Tomas Zezula
+ * @author Tomas Zezula, Lukasz Bownik
  */
 public class PairTest extends NbTestCase {
 
-    public PairTest(String name) {
+    //--------------------------------------------------------------------------
+    public PairTest(final String name) {
+
         super(name);
     }
 
+    //-------------------------------------------------------------------------- 
+    public void test_pairEquality() {
+
+        Pair<Integer, Integer> pair11a = Pair.of(1, 1);
+        Pair<Integer, Integer> pair11b = Pair.of(1, 1);
+        Pair<Integer, Integer> pair12 = Pair.of(1, 2);
+        Pair<Integer, Integer> pairNul1 = Pair.of(null, 1);
+        Pair<Integer, Integer> pair1Null = Pair.of(1, null);
+        Pair<Integer, Integer> pairNullNull = Pair.of(null, null);
+
+        assertTrue(pair11a.equals(pair11a));
+        assertEquals(pair11a.hashCode(), pair11a.hashCode());
+        assertEquals(new Integer(1), pair11a.first());
+        assertEquals(new Integer(1), pair11a.second());
+
+        assertTrue(pair12.equals(pair12));
+        assertEquals(pair12.hashCode(), pair12.hashCode());
+        assertEquals(new Integer(1), pair12.first());
+        assertEquals(new Integer(2), pair12.second());
+
+        assertTrue(pair11a.equals(pair11b));
+        assertEquals(pair11a.hashCode(), pair11b.hashCode());
+        assertEquals(new Integer(1), pair11b.first());
+        assertEquals(new Integer(1), pair11b.second());
+
+        assertTrue(pair1Null.equals(pair1Null));
+        assertEquals(pair1Null.hashCode(), pair1Null.hashCode());
+        assertEquals(new Integer(1), pair1Null.first());
+        assertEquals(null, pair1Null.second());
+
+        assertTrue(pairNul1.equals(pairNul1));
+        assertEquals(pairNul1.hashCode(), pairNul1.hashCode());
+        assertEquals(null, pairNul1.first());
+        assertEquals(new Integer(1), pairNul1.second());
+
+        assertTrue(pairNullNull.equals(pairNullNull));
+        assertEquals(pairNullNull.hashCode(), pairNullNull.hashCode());
+        assertEquals(null, pairNullNull.first());
+        assertEquals(null, pairNullNull.second());
+    }
+
+    //-------------------------------------------------------------------------- 
+    public void test_pairInequality() {
+
+        Pair<Integer, Integer> pair11a = Pair.of(1, 1);
+        Pair<Integer, Integer> pair12 = Pair.of(1, 2);
+        Pair<Integer, Integer> pair21 = Pair.of(2, 1);
+        Pair<Integer, Integer> pairNul1 = Pair.of(null, 1);
+        Pair<Integer, Integer> pair1Null = Pair.of(1, null);
+        Pair<Integer, Integer> pairNullNull = Pair.of(null, null);
+        
+        assertFalse(pair11a.equals(pair12));
+        assertFalse(pair11a.equals(pair21));
+        assertFalse(pair11a.equals(pairNul1));
+        assertFalse(pair11a.equals(pair1Null));
+        assertFalse(pair11a.equals(pairNullNull));
+        assertFalse(pairNul1.equals(pair11a));
+        assertFalse(pair1Null.equals(pair11a));
+        assertFalse(pairNullNull.equals(pair11a));
 
-    public void testPairs() {
-        final Pair<Integer,Integer> p1a = Pair.of(1, 1);
-        final Pair<Integer,Integer> p1b = Pair.of(1, 1);
-        final Pair<Integer,Integer> p2 = Pair.of(1, 2);
-        final Pair<Integer,Integer> p3 = Pair.of(2, 1);
-        final Pair<Integer,Integer> p4 = Pair.of(null, 1);
-        final Pair<Integer,Integer> p5 = Pair.of(1, null);
-        final Pair<Integer,Integer> p6 = Pair.of(null, null);
-        assertTrue(p1a.equals(p1a));
-        assertTrue(p1a.equals(p1b));
-        assertFalse(p1a.equals(p2));
-        assertFalse(p1a.equals(p3));
-        assertFalse(p1a.equals(p4));
-        assertFalse(p1a.equals(p5));
-        assertFalse(p1a.equals(p6));
-        assertEquals(p1a.hashCode(), p1b.hashCode());
-        assertEquals(p4.hashCode(), p4.hashCode());
-        assertEquals(p5.hashCode(), p5.hashCode());
-        assertEquals(p6.hashCode(), p6.hashCode());
-        assertFalse(p4.hashCode() == p5.hashCode());
+        assertFalse(pair11a.equals(null));
+        assertFalse(pair11a.equals(""));
     }
 
+    //--------------------------------------------------------------------------
+    public void test_pair_toString() {

Review Comment:
   Please use camelCase and not snakeCase



##########
platform/openide.util/test/unit/src/org/openide/util/BaseUtilitiesTest.java:
##########
@@ -309,75 +303,73 @@ public void test_toURI_producesURI_wchichResolvesAndWrapsProperly()
         URI wrappedResolvedURI = new URI("jar:" + resolvedURI + "!/");
         assertEquals(wrappedURI, wrappedResolvedURI);
     }
+    
+    //--------------------------------------------------------------------------
+    public void test_parseParameters_throwsNullPointer_whenGivenNullArgument() {
+
+        expectNullPointer(() -> parseParameters(null));
+    }
+
+    //--------------------------------------------------------------------------
+    public void test_parseParameters_returnsEmptyArray_whenGivenEmptyString() {
+
+        assertEquals(0, parseParameters("").length);
+        assertEquals(0, parseParameters("   \t\r\n").length);
+    }
+
+    //--------------------------------------------------------------------------
+    public void test_parseParameters_returnsProperArray_forProperInput() {
+
+        String[] args;

Review Comment:
   Why define the variable in a different line ?



##########
platform/openide.util/test/unit/src/org/openide/util/BaseUtilitiesTest.java:
##########
@@ -100,6 +103,7 @@ public void testWhatIsFreeBSD () {
         assertFalse ("freebsd is not isWindows", BaseUtilities.isWindows ());
         assertTrue ("freebsd isUnix", BaseUtilities.isUnix ());
     }
+    

Review Comment:
   Please avoid changing formatting as this creates noise



##########
platform/openide.util/test/unit/src/org/openide/util/PairTest.java:
##########
@@ -19,39 +19,90 @@
 package org.openide.util;
 
 import org.netbeans.junit.NbTestCase;
-import org.openide.util.Pair;
 
 /**
  *
- * @author Tomas Zezula
+ * @author Tomas Zezula, Lukasz Bownik
  */
 public class PairTest extends NbTestCase {
 
-    public PairTest(String name) {
+    //--------------------------------------------------------------------------
+    public PairTest(final String name) {
+
         super(name);
     }
 
+    //-------------------------------------------------------------------------- 
+    public void test_pairEquality() {

Review Comment:
   Please use camelCase and not snakeCase



-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists