garydgregory commented on code in PR #904:
URL: https://github.com/apache/commons-lang/pull/904#discussion_r940163028
##########
src/test/java/org/apache/commons/lang3/ValidateTest.java:
##########
@@ -774,6 +774,45 @@ void
shouldThrowIllegalArgumentExceptionWithGivenMessageForCollectionWithNullEle
}
}
+ @Nested
+ class ValidState {
+
+ @Nested
+ class WitMessage {
+ @Test
+ void shouldNotThrowExceptionForValidIndex() {
+ Validate.validState(true, "The Message");
+ }
+
+ @Test
+ void shouldThrowExceptionForTrueExpression() {
+ assertThrows(
+ IllegalStateException.class,
+ () -> Validate.validState(false, "The Message"));
+
Review Comment:
Too much whitespace IMO.
##########
src/test/java/org/apache/commons/lang3/RangeTest.java:
##########
@@ -103,6 +103,10 @@ public void testBetweenWithCompare() {
assertTrue(rbstr.contains("i"), "should contain i");
assertFalse(rbstr.contains("houses"), "should not contain houses");
assertFalse(rbstr.contains(""), "should not contain ''");
+
+ assertThrows(
Review Comment:
You can put this all on one line IMO.
##########
src/test/java/org/apache/commons/lang3/ValidateTest.java:
##########
@@ -774,6 +774,45 @@ void
shouldThrowIllegalArgumentExceptionWithGivenMessageForCollectionWithNullEle
}
}
+ @Nested
+ class ValidState {
+
+ @Nested
+ class WitMessage {
+ @Test
+ void shouldNotThrowExceptionForValidIndex() {
+ Validate.validState(true, "The Message");
+ }
+
+ @Test
+ void shouldThrowExceptionForTrueExpression() {
+ assertThrows(
+ IllegalStateException.class,
+ () -> Validate.validState(false, "The Message"));
+
+ }
+
+ }
+
+ @Nested
+ class WithoutMessage {
+
+ @Test
+ void shouldNotThrowExceptionForTrueExpression() {
+ Validate.validState(true);
+ }
+
+ @Test
+ void shouldThrowExceptionForTrueExpression() {
+ assertThrows(
Review Comment:
You can put stuff like this all on one line IMO.
##########
src/test/java/org/apache/commons/lang3/builder/EqualsBuilderTest.java:
##########
@@ -443,6 +446,16 @@ public void testObjectRecursiveGenericInteger() {
assertFalse(new EqualsBuilder().setTestRecursive(true).append(o1_b,
o2).isEquals());
}
+ @Test
+ public void testObjectsBypassReflectionClasses() {
+
Review Comment:
Too much whitespace.
##########
src/test/java/org/apache/commons/lang3/ClassUtilsTest.java:
##########
@@ -410,6 +420,9 @@ class Named {
}.getClass()));
assertEquals("org.apache.commons.lang3",
ClassUtils.getPackageCanonicalName(Named.class));
assertEquals("org.apache.commons.lang3",
ClassUtils.getPackageCanonicalName(Inner.class));
+ assertEquals(StringUtils.EMPTY,
ClassUtils.getPackageCanonicalName((Class<?>) null));
+
Review Comment:
Remove extra two blank lines.
##########
src/test/java/org/apache/commons/lang3/ClassUtilsTest.java:
##########
@@ -52,6 +52,8 @@
@SuppressWarnings("boxing") // JUnit4 does not support primitive equality
testing apart from long
public class ClassUtilsTest {
+ private static final String DEFAULT_CANONICAL_NAME = "java.lang.Object";
Review Comment:
Maybe `DEFAULT_CANONICAL_NAME` -> `OBJECT_CANONICAL_NAME`?
##########
src/test/java/org/apache/commons/lang3/NotImplementedExceptionTest.java:
##########
@@ -44,6 +45,9 @@ public void testConstructors() {
assertCorrect("Issue in (Throwable, String)", nie, nested.toString(),
nested, code);
nie = new NotImplementedException(message, nested, code);
assertCorrect("Issue in (String, Throwable, String)", nie, message,
nested, code);
+
+ final NotImplementedException nieNullCode = new
NotImplementedException();
Review Comment:
Do we really need the local variable?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
