From c00920fe55dc61504bc0de2a8df004fc7264f5bf Mon Sep 17 00:00:00 2001 From: Allon Mureinik Date: Sat, 30 Sep 2017 15:25:34 +0300 Subject: [PATCH 1/3] Clean up if in CharUtilsTest#testIsAscii_char The if statement calls assertTrue on the if branch and assertFalse on the else branch on the same expression. This can easily be simplified to assertEquals with a boolean expression to make the code clean and easier to read. --- src/test/java/org/apache/commons/lang3/CharUtilsTest.java | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/test/java/org/apache/commons/lang3/CharUtilsTest.java b/src/test/java/org/apache/commons/lang3/CharUtilsTest.java index bd92c3355f5..35f7f02712a 100644 --- a/src/test/java/org/apache/commons/lang3/CharUtilsTest.java +++ b/src/test/java/org/apache/commons/lang3/CharUtilsTest.java @@ -220,11 +220,7 @@ public void testIsAscii_char() { assertFalse(CharUtils.isAscii(CHAR_COPY)); for (int i = 0; i < 128; i++) { - if (i < 128) { - assertTrue(CharUtils.isAscii((char) i)); - } else { - assertFalse(CharUtils.isAscii((char) i)); - } + assertEquals(i < 128, CharUtils.isAscii((char) i)); } } From 9512d67084b340c7493ff89f2f2cd2e4eee33241 Mon Sep 17 00:00:00 2001 From: Allon Mureinik Date: Sat, 30 Sep 2017 15:27:42 +0300 Subject: [PATCH 2/3] CharUtilsTest#testIsAscii_char loop The loop currently loops only up to 128, thus testing just positive return values of CharUtils#isAscii(char). This patch increase the loop to go over all the possible values of an unsigned byte, thus testing also negative return values. --- src/test/java/org/apache/commons/lang3/CharUtilsTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/apache/commons/lang3/CharUtilsTest.java b/src/test/java/org/apache/commons/lang3/CharUtilsTest.java index 35f7f02712a..71975d3a065 100644 --- a/src/test/java/org/apache/commons/lang3/CharUtilsTest.java +++ b/src/test/java/org/apache/commons/lang3/CharUtilsTest.java @@ -219,7 +219,7 @@ public void testIsAscii_char() { assertTrue(CharUtils.isAscii('\n')); assertFalse(CharUtils.isAscii(CHAR_COPY)); - for (int i = 0; i < 128; i++) { + for (int i = 0; i < 255; i++) { assertEquals(i < 128, CharUtils.isAscii((char) i)); } } From b763decb69826b0fd4387e53dfd9c4ca5ab91467 Mon Sep 17 00:00:00 2001 From: Allon Mureinik Date: Sat, 30 Sep 2017 15:32:54 +0300 Subject: [PATCH 3/3] Improve tests for CharUtils illegal usages CharUtilsTest has several instances of the following pattern: try { CharUtils.someMethod("illegal input"); } catch (final IllegalArgumentException ex) {} This pattern is not very useful for testing, as the test would pass whether an IllegalArgumentException is thrown or not. This patch enhances the test by explicitly failing it if the exception is not thrown: try { CharUtils.someMethod("illegal input"); fail("An IllegalArgumentException should have been thrown"); } catch (final IllegalArgumentException ex) {} --- src/test/java/org/apache/commons/lang3/CharUtilsTest.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/test/java/org/apache/commons/lang3/CharUtilsTest.java b/src/test/java/org/apache/commons/lang3/CharUtilsTest.java index 71975d3a065..50f263460be 100644 --- a/src/test/java/org/apache/commons/lang3/CharUtilsTest.java +++ b/src/test/java/org/apache/commons/lang3/CharUtilsTest.java @@ -21,6 +21,7 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.lang.reflect.Constructor; import java.lang.reflect.Modifier; @@ -84,6 +85,7 @@ public void testToChar_Character() { assertEquals('B', CharUtils.toChar(CHARACTER_B)); try { CharUtils.toChar((Character) null); + fail("An IllegalArgumentException should have been thrown"); } catch (final IllegalArgumentException ex) {} } @@ -100,9 +102,11 @@ public void testToChar_String() { assertEquals('B', CharUtils.toChar("BA")); try { CharUtils.toChar((String) null); + fail("An IllegalArgumentException should have been thrown"); } catch (final IllegalArgumentException ex) {} try { CharUtils.toChar(""); + fail("An IllegalArgumentException should have been thrown"); } catch (final IllegalArgumentException ex) {} } @@ -128,6 +132,7 @@ public void testToIntValue_char() { assertEquals(9, CharUtils.toIntValue('9')); try { CharUtils.toIntValue('a'); + fail("An IllegalArgumentException should have been thrown"); } catch (final IllegalArgumentException ex) {} } @@ -144,9 +149,11 @@ public void testToIntValue_Character() { assertEquals(3, CharUtils.toIntValue(new Character('3'))); try { CharUtils.toIntValue(null); + fail("An IllegalArgumentException should have been thrown"); } catch (final IllegalArgumentException ex) {} try { CharUtils.toIntValue(CHARACTER_A); + fail("An IllegalArgumentException should have been thrown"); } catch (final IllegalArgumentException ex) {} }