markt-asf commented on code in PR #962:
URL: https://github.com/apache/tomcat/pull/962#discussion_r2965332489
##########
test/org/apache/tomcat/util/buf/TestStringUtils.java:
##########
@@ -22,7 +6,7 @@
import org.junit.Test;
/*
- * None of these tests should throw a NPE.
+ *
*/
Review Comment:
Why is this explanatory comment removed?
##########
test/org/apache/tomcat/util/buf/TestStringUtils.java:
##########
@@ -31,47 +15,65 @@ public void testNullArray() {
Assert.assertEquals("", StringUtils.join((String[]) null));
}
-
@Test
public void testNullArrayCharStringBuilder() {
StringBuilder sb = new StringBuilder();
StringUtils.join((String[]) null, ',', sb);
Assert.assertEquals("", sb.toString());
}
-
@Test
public void testNullCollection() {
Assert.assertEquals("", StringUtils.join((Collection<String>) null));
}
-
@Test
public void testNullCollectionChar() {
Assert.assertEquals("", StringUtils.join(null, ','));
}
-
@Test
public void testNullIterableCharStringBuilder() {
StringBuilder sb = new StringBuilder();
StringUtils.join((Iterable<String>) null, ',', sb);
Assert.assertEquals("", sb.toString());
}
-
@Test
public void testNullArrayCharFunctionStringBuilder() {
StringBuilder sb = new StringBuilder();
StringUtils.join((String[]) null, ',', null, sb);
Assert.assertEquals("", sb.toString());
}
-
@Test
public void testNullIterableCharFunctionStringBuilder() {
StringBuilder sb = new StringBuilder();
StringUtils.join((Iterable<String>) null, ',', null, sb);
Assert.assertEquals("", sb.toString());
}
-}
+
+ // ✅ Added tests by Y Charan
Review Comment:
Remove this line. We do not advertise authorship in the Tomcat code base.
##########
test/org/apache/catalina/util/TestParameterMap.java:
##########
@@ -300,11 +300,37 @@ public void testEntrySetImmutabilityAfterLocked() {
Assert.fail("ParameterMap is not locked.");
} catch (UnsupportedOperationException expectedException) {
}
-
try {
entrySet.clear();
Assert.fail("ParameterMap is not locked.");
} catch (UnsupportedOperationException expectedException) {
}
}
+ @Test
Review Comment:
Needs a space.
##########
test/org/apache/tomcat/util/buf/TestStringUtils.java:
##########
@@ -31,47 +15,65 @@ public void testNullArray() {
Assert.assertEquals("", StringUtils.join((String[]) null));
}
-
@Test
public void testNullArrayCharStringBuilder() {
StringBuilder sb = new StringBuilder();
StringUtils.join((String[]) null, ',', sb);
Assert.assertEquals("", sb.toString());
}
-
@Test
public void testNullCollection() {
Assert.assertEquals("", StringUtils.join((Collection<String>) null));
}
-
@Test
public void testNullCollectionChar() {
Assert.assertEquals("", StringUtils.join(null, ','));
}
-
Review Comment:
No.
##########
test/org/apache/tomcat/util/buf/TestStringUtils.java:
##########
@@ -1,19 +1,3 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
package org.apache.tomcat.util.buf;
Review Comment:
Absolutely not. It is **TOTALLY** unacceptable to remove a license header.
##########
test/org/apache/tomcat/util/buf/TestStringUtils.java:
##########
@@ -31,47 +15,65 @@ public void testNullArray() {
Assert.assertEquals("", StringUtils.join((String[]) null));
}
-
@Test
public void testNullArrayCharStringBuilder() {
StringBuilder sb = new StringBuilder();
StringUtils.join((String[]) null, ',', sb);
Assert.assertEquals("", sb.toString());
}
-
@Test
public void testNullCollection() {
Assert.assertEquals("", StringUtils.join((Collection<String>) null));
}
-
@Test
public void testNullCollectionChar() {
Assert.assertEquals("", StringUtils.join(null, ','));
}
-
@Test
public void testNullIterableCharStringBuilder() {
StringBuilder sb = new StringBuilder();
StringUtils.join((Iterable<String>) null, ',', sb);
Assert.assertEquals("", sb.toString());
}
-
@Test
public void testNullArrayCharFunctionStringBuilder() {
StringBuilder sb = new StringBuilder();
StringUtils.join((String[]) null, ',', null, sb);
Assert.assertEquals("", sb.toString());
}
-
Review Comment:
No.
##########
test/org/apache/tomcat/util/buf/TestStringUtils.java:
##########
@@ -31,47 +15,65 @@ public void testNullArray() {
Assert.assertEquals("", StringUtils.join((String[]) null));
}
-
@Test
public void testNullArrayCharStringBuilder() {
StringBuilder sb = new StringBuilder();
StringUtils.join((String[]) null, ',', sb);
Assert.assertEquals("", sb.toString());
}
-
Review Comment:
No.
##########
test/org/apache/tomcat/util/buf/TestStringUtils.java:
##########
@@ -31,47 +15,65 @@ public void testNullArray() {
Assert.assertEquals("", StringUtils.join((String[]) null));
}
-
Review Comment:
No.
##########
test/org/apache/catalina/util/TestParameterMap.java:
##########
@@ -300,11 +300,37 @@ public void testEntrySetImmutabilityAfterLocked() {
Assert.fail("ParameterMap is not locked.");
} catch (UnsupportedOperationException expectedException) {
}
-
Review Comment:
No. This is here for readability
##########
test/org/apache/tomcat/util/buf/TestStringUtils.java:
##########
@@ -31,47 +15,65 @@ public void testNullArray() {
Assert.assertEquals("", StringUtils.join((String[]) null));
}
-
@Test
public void testNullArrayCharStringBuilder() {
StringBuilder sb = new StringBuilder();
StringUtils.join((String[]) null, ',', sb);
Assert.assertEquals("", sb.toString());
}
-
@Test
public void testNullCollection() {
Assert.assertEquals("", StringUtils.join((Collection<String>) null));
}
-
Review Comment:
No.
##########
test/org/apache/tomcat/util/buf/TestStringUtils.java:
##########
@@ -31,47 +15,65 @@ public void testNullArray() {
Assert.assertEquals("", StringUtils.join((String[]) null));
}
-
@Test
public void testNullArrayCharStringBuilder() {
StringBuilder sb = new StringBuilder();
StringUtils.join((String[]) null, ',', sb);
Assert.assertEquals("", sb.toString());
}
-
@Test
public void testNullCollection() {
Assert.assertEquals("", StringUtils.join((Collection<String>) null));
}
-
@Test
public void testNullCollectionChar() {
Assert.assertEquals("", StringUtils.join(null, ','));
}
-
@Test
public void testNullIterableCharStringBuilder() {
StringBuilder sb = new StringBuilder();
StringUtils.join((Iterable<String>) null, ',', sb);
Assert.assertEquals("", sb.toString());
}
-
@Test
public void testNullArrayCharFunctionStringBuilder() {
StringBuilder sb = new StringBuilder();
StringUtils.join((String[]) null, ',', null, sb);
Assert.assertEquals("", sb.toString());
}
-
@Test
public void testNullIterableCharFunctionStringBuilder() {
StringBuilder sb = new StringBuilder();
StringUtils.join((Iterable<String>) null, ',', null, sb);
Assert.assertEquals("", sb.toString());
}
-}
+
+ // ✅ Added tests by Y Charan
+
+ @Test
+ public void testSplitCommaSeparatedOnlyCommas() {
+ String[] result = StringUtils.splitCommaSeparated(",,,");
+ Assert.assertArrayEquals(new String[]{}, result);
+ }
+
+ @Test
+ public void testSplitCommaSeparatedMultipleCommas() {
+ String[] result = StringUtils.splitCommaSeparated("a,,b");
+ Assert.assertArrayEquals(new String[]{"a", "", "b"}, result);
+ }
+
+ // 🔥
+
Review Comment:
Remove this nonsense.
##########
test/org/apache/tomcat/util/buf/TestStringUtils.java:
##########
@@ -31,47 +15,65 @@ public void testNullArray() {
Assert.assertEquals("", StringUtils.join((String[]) null));
}
-
@Test
public void testNullArrayCharStringBuilder() {
StringBuilder sb = new StringBuilder();
StringUtils.join((String[]) null, ',', sb);
Assert.assertEquals("", sb.toString());
}
-
@Test
public void testNullCollection() {
Assert.assertEquals("", StringUtils.join((Collection<String>) null));
}
-
@Test
public void testNullCollectionChar() {
Assert.assertEquals("", StringUtils.join(null, ','));
}
-
@Test
public void testNullIterableCharStringBuilder() {
StringBuilder sb = new StringBuilder();
StringUtils.join((Iterable<String>) null, ',', sb);
Assert.assertEquals("", sb.toString());
}
-
Review Comment:
No.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
