You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by jc...@apache.org on 2010/08/07 14:04:47 UTC
svn commit: r983219 -
/commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/StringUtilsTest.java
Author: jcarman
Date: Sat Aug 7 12:04:46 2010
New Revision: 983219
URL: http://svn.apache.org/viewvc?rev=983219&view=rev
Log:
Some code clean-up. Removing unnecessary boxing, array creation for varargs, etc.
Modified:
commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/StringUtilsTest.java
Modified: commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/StringUtilsTest.java
URL: http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/StringUtilsTest.java?rev=983219&r1=983218&r2=983219&view=diff
==============================================================================
--- commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/StringUtilsTest.java (original)
+++ commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/StringUtilsTest.java Sat Aug 7 12:04:46 2010
@@ -77,7 +77,7 @@ public class StringUtilsTest extends Tes
private static final String[] EMPTY_ARRAY_LIST = {};
private static final String[] NULL_ARRAY_LIST = {null};
private static final String[] MIXED_ARRAY_LIST = {null, "", "foo"};
- private static final Object[] MIXED_TYPE_LIST = {new String("foo"), new Long(2)};
+ private static final Object[] MIXED_TYPE_LIST = {"foo", 2L};
private static final String SEPARATOR = ",";
private static final char SEPARATOR_CHAR = ';';
@@ -191,7 +191,7 @@ public class StringUtilsTest extends Tes
public void testConcat_Objects() {
assertEquals("abc", StringUtils.concat("a", "b", "c"));
assertEquals("a", StringUtils.concat(null, "", "a"));
- assertEquals(null, StringUtils.concat(null));
+ assertEquals(null, StringUtils.concat((String[])null));
}
public void testConcatWith_StringObjects() {
@@ -199,7 +199,7 @@ public class StringUtilsTest extends Tes
assertEquals("a...b...c", StringUtils.concatWith("...", "a", "b", "c"));
assertEquals("a", StringUtils.concatWith("", null, "", "a"));
assertEquals("a", StringUtils.concatWith(null, null, "", "a"));
- assertEquals(null, StringUtils.concatWith(null, null));
+ assertEquals(null, StringUtils.concatWith(null, (String[])null));
}
public void testJoin_Objectarray() {
@@ -361,7 +361,7 @@ public class StringUtilsTest extends Tes
}
}
- String[] results = null;
+ String[] results;
String[] expectedResults = {"ab", "de fg"};
results = StringUtils.split("ab de fg", null, 2);
assertEquals(expectedResults.length, results.length);
@@ -696,7 +696,7 @@ public class StringUtilsTest extends Tes
// Match example in javadoc
{
- String[] results = null;
+ String[] results;
String[] expectedResults = {"a", "", "b", "c"};
results = StringUtils.splitPreserveAllTokens("a..b.c",'.');
assertEquals(expectedResults.length, results.length);
@@ -724,7 +724,7 @@ public class StringUtilsTest extends Tes
}
{
- String[] results = null;
+ String[] results;
String[] expectedResults = {"ab", "de fg"};
results = StringUtils.splitPreserveAllTokens("ab de fg", null, 2);
assertEquals(expectedResults.length, results.length);
@@ -734,7 +734,7 @@ public class StringUtilsTest extends Tes
}
{
- String[] results = null;
+ String[] results;
String[] expectedResults = {"ab", " de fg"};
results = StringUtils.splitPreserveAllTokens("ab de fg", null, 2);
assertEquals(expectedResults.length, results.length);
@@ -744,7 +744,7 @@ public class StringUtilsTest extends Tes
}
{
- String[] results = null;
+ String[] results;
String[] expectedResults = {"ab", "::de:fg"};
results = StringUtils.splitPreserveAllTokens("ab:::de:fg", ":", 2);
assertEquals(expectedResults.length, results.length);
@@ -754,7 +754,7 @@ public class StringUtilsTest extends Tes
}
{
- String[] results = null;
+ String[] results;
String[] expectedResults = {"ab", "", " de fg"};
results = StringUtils.splitPreserveAllTokens("ab de fg", null, 3);
assertEquals(expectedResults.length, results.length);
@@ -764,7 +764,7 @@ public class StringUtilsTest extends Tes
}
{
- String[] results = null;
+ String[] results;
String[] expectedResults = {"ab", "", "", "de fg"};
results = StringUtils.splitPreserveAllTokens("ab de fg", null, 4);
assertEquals(expectedResults.length, results.length);
@@ -775,7 +775,7 @@ public class StringUtilsTest extends Tes
{
String[] expectedResults = {"ab", "cd:ef"};
- String[] results = null;
+ String[] results;
results = StringUtils.splitPreserveAllTokens("ab:cd:ef",":", 2);
assertEquals(expectedResults.length, results.length);
for (int i = 0; i < expectedResults.length; i++) {
@@ -784,7 +784,7 @@ public class StringUtilsTest extends Tes
}
{
- String[] results = null;
+ String[] results;
String[] expectedResults = {"ab", ":cd:ef"};
results = StringUtils.splitPreserveAllTokens("ab::cd:ef",":", 2);
assertEquals(expectedResults.length, results.length);
@@ -794,7 +794,7 @@ public class StringUtilsTest extends Tes
}
{
- String[] results = null;
+ String[] results;
String[] expectedResults = {"ab", "", ":cd:ef"};
results = StringUtils.splitPreserveAllTokens("ab:::cd:ef",":", 3);
assertEquals(expectedResults.length, results.length);
@@ -804,7 +804,7 @@ public class StringUtilsTest extends Tes
}
{
- String[] results = null;
+ String[] results;
String[] expectedResults = {"ab", "", "", "cd:ef"};
results = StringUtils.splitPreserveAllTokens("ab:::cd:ef",":", 4);
assertEquals(expectedResults.length, results.length);
@@ -814,7 +814,7 @@ public class StringUtilsTest extends Tes
}
{
- String[] results = null;
+ String[] results;
String[] expectedResults = {"", "ab", "", "", "cd:ef"};
results = StringUtils.splitPreserveAllTokens(":ab:::cd:ef",":", 5);
assertEquals(expectedResults.length, results.length);
@@ -824,7 +824,7 @@ public class StringUtilsTest extends Tes
}
{
- String[] results = null;
+ String[] results;
String[] expectedResults = {"", "", "ab", "", "", "cd:ef"};
results = StringUtils.splitPreserveAllTokens("::ab:::cd:ef",":", 6);
assertEquals(expectedResults.length, results.length);
@@ -1330,28 +1330,28 @@ public class StringUtilsTest extends Tes
}
public void testLengthString() {
- assertEquals(0, StringUtils.length(null));
- assertEquals(0, StringUtils.length(""));
- assertEquals(0, StringUtils.length(StringUtils.EMPTY));
- assertEquals(1, StringUtils.length("A"));
- assertEquals(1, StringUtils.length(" "));
- assertEquals(8, StringUtils.length("ABCDEFGH"));
+ assertEquals(0, CharSequenceUtils.length(null));
+ assertEquals(0, CharSequenceUtils.length(""));
+ assertEquals(0, CharSequenceUtils.length(StringUtils.EMPTY));
+ assertEquals(1, CharSequenceUtils.length("A"));
+ assertEquals(1, CharSequenceUtils.length(" "));
+ assertEquals(8, CharSequenceUtils.length("ABCDEFGH"));
}
public void testLengthStringBuffer() {
- assertEquals(0, StringUtils.length(new StringBuffer("")));
- assertEquals(0, StringUtils.length(new StringBuffer(StringUtils.EMPTY)));
- assertEquals(1, StringUtils.length(new StringBuffer("A")));
- assertEquals(1, StringUtils.length(new StringBuffer(" ")));
- assertEquals(8, StringUtils.length(new StringBuffer("ABCDEFGH")));
+ assertEquals(0, CharSequenceUtils.length(new StringBuffer("")));
+ assertEquals(0, CharSequenceUtils.length(new StringBuffer(StringUtils.EMPTY)));
+ assertEquals(1, CharSequenceUtils.length(new StringBuffer("A")));
+ assertEquals(1, CharSequenceUtils.length(new StringBuffer(" ")));
+ assertEquals(8, CharSequenceUtils.length(new StringBuffer("ABCDEFGH")));
}
public void testLengthStringBuilder() {
- assertEquals(0, StringUtils.length(new StringBuilder("")));
- assertEquals(0, StringUtils.length(new StringBuilder(StringUtils.EMPTY)));
- assertEquals(1, StringUtils.length(new StringBuilder("A")));
- assertEquals(1, StringUtils.length(new StringBuilder(" ")));
- assertEquals(8, StringUtils.length(new StringBuilder("ABCDEFGH")));
+ assertEquals(0, CharSequenceUtils.length(new StringBuilder("")));
+ assertEquals(0, CharSequenceUtils.length(new StringBuilder(StringUtils.EMPTY)));
+ assertEquals(1, CharSequenceUtils.length(new StringBuilder("A")));
+ assertEquals(1, CharSequenceUtils.length(new StringBuilder(" ")));
+ assertEquals(8, CharSequenceUtils.length(new StringBuilder("ABCDEFGH")));
}
//-----------------------------------------------------------------------
@@ -1806,7 +1806,7 @@ public class StringUtilsTest extends Tes
}
public void testDifferenceAt_StringArray(){
- assertEquals(-1, StringUtils.indexOfDifference(null));
+ assertEquals(-1, StringUtils.indexOfDifference((String[])null));
assertEquals(-1, StringUtils.indexOfDifference(new String[] {}));
assertEquals(-1, StringUtils.indexOfDifference(new String[] {"abc"}));
assertEquals(-1, StringUtils.indexOfDifference(new String[] {null, null}));
@@ -1826,33 +1826,33 @@ public class StringUtilsTest extends Tes
}
public void testGetCommonPrefix_StringArray(){
- assertEquals("", StringUtils.getCommonPrefix(null));
- assertEquals("", StringUtils.getCommonPrefix(new String[] {}));
- assertEquals("abc", StringUtils.getCommonPrefix(new String[] {"abc"}));
- assertEquals("", StringUtils.getCommonPrefix(new String[] {null, null}));
- assertEquals("", StringUtils.getCommonPrefix(new String[] {"", ""}));
- assertEquals("", StringUtils.getCommonPrefix(new String[] {"", null}));
- assertEquals("", StringUtils.getCommonPrefix(new String[] {"abc", null, null}));
- assertEquals("", StringUtils.getCommonPrefix(new String[] {null, null, "abc"}));
- assertEquals("", StringUtils.getCommonPrefix(new String[] {"", "abc"}));
- assertEquals("", StringUtils.getCommonPrefix(new String[] {"abc", ""}));
- assertEquals("abc", StringUtils.getCommonPrefix(new String[] {"abc", "abc"}));
- assertEquals("a", StringUtils.getCommonPrefix(new String[] {"abc", "a"}));
- assertEquals("ab", StringUtils.getCommonPrefix(new String[] {"ab", "abxyz"}));
- assertEquals("ab", StringUtils.getCommonPrefix(new String[] {"abcde", "abxyz"}));
- assertEquals("", StringUtils.getCommonPrefix(new String[] {"abcde", "xyz"}));
- assertEquals("", StringUtils.getCommonPrefix(new String[] {"xyz", "abcde"}));
- assertEquals("i am a ", StringUtils.getCommonPrefix(new String[] {"i am a machine", "i am a robot"}));
+ assertEquals("", StringUtils.getCommonPrefix((String[])null));
+ assertEquals("", StringUtils.getCommonPrefix());
+ assertEquals("abc", StringUtils.getCommonPrefix("abc"));
+ assertEquals("", StringUtils.getCommonPrefix(null, null));
+ assertEquals("", StringUtils.getCommonPrefix("", ""));
+ assertEquals("", StringUtils.getCommonPrefix("", null));
+ assertEquals("", StringUtils.getCommonPrefix("abc", null, null));
+ assertEquals("", StringUtils.getCommonPrefix(null, null, "abc"));
+ assertEquals("", StringUtils.getCommonPrefix("", "abc"));
+ assertEquals("", StringUtils.getCommonPrefix("abc", ""));
+ assertEquals("abc", StringUtils.getCommonPrefix("abc", "abc"));
+ assertEquals("a", StringUtils.getCommonPrefix("abc", "a"));
+ assertEquals("ab", StringUtils.getCommonPrefix("ab", "abxyz"));
+ assertEquals("ab", StringUtils.getCommonPrefix("abcde", "abxyz"));
+ assertEquals("", StringUtils.getCommonPrefix("abcde", "xyz"));
+ assertEquals("", StringUtils.getCommonPrefix("xyz", "abcde"));
+ assertEquals("i am a ", StringUtils.getCommonPrefix("i am a machine", "i am a robot"));
}
public void testStartsWithAny() {
- assertFalse(StringUtils.startsWithAny(null, null));
- assertFalse(StringUtils.startsWithAny(null, new String[] {"abc"}));
- assertFalse(StringUtils.startsWithAny("abcxyz", null));
- assertFalse(StringUtils.startsWithAny("abcxyz", new String[] {}));
- assertTrue(StringUtils.startsWithAny("abcxyz", new String[] {"abc"}));
- assertTrue(StringUtils.startsWithAny("abcxyz", new String[] {null, "xyz", "abc"}));
- assertFalse(StringUtils.startsWithAny("abcxyz", new String[] {null, "xyz", "abcd"}));
+ assertFalse(StringUtils.startsWithAny(null, (String[])null));
+ assertFalse(StringUtils.startsWithAny(null, "abc"));
+ assertFalse(StringUtils.startsWithAny("abcxyz", (String[])null));
+ assertFalse(StringUtils.startsWithAny("abcxyz"));
+ assertTrue(StringUtils.startsWithAny("abcxyz", "abc"));
+ assertTrue(StringUtils.startsWithAny("abcxyz", null, "xyz", "abc"));
+ assertFalse(StringUtils.startsWithAny("abcxyz", null, "xyz", "abcd"));
}
public void testNormalizeSpace() {
Re: svn commit: r983219 - /commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/StringUtilsTest.java
Posted by Jörg Schaible <jo...@gmx.de>.
sebb wrote:
> On 8 August 2010 22:34, James Carman <ja...@carmanconsulting.com> wrote:
>> To make sure I'm actually passing in the exact same values (by letting
>> the compiler create the arrays using the varargs feature) I added the
>> following output to the startsWithAny() method:
>>
>> System.out.println("startsWithAny(): Received " + string + " input
>> with " + (searchStrings == null ? null :
>> Arrays.asList(searchStrings).toString()) + " input array");
>>
>> When running the old version of the test code to the new version,
>> there was no difference in the output.
>>
>> I also ran a similar test by changing getCommonPrefix() method:
>>
>> System.out.println("getCommonPrefix(): Received " + (strs == null ?
>> null : Arrays.asList(strs).toString()) + " input array");
>>
>> Again, no changes in the output. So, we're at least exercising the
>> "guts" of the methods the same. Now I can understand that you might
>> be saying that we're not exercising how the code is called the same
>> way in both cases, but the varargs support vs. the manual array
>> creation is identical. If you think we should add back in the manual
>> String[] stuff, I guess we could, but I really don't see much value in
>> it other than testing that varargs == manual String[].
>>
>
> Although the compiler changes varargs into String[] arrays, the two
> tests are not equivalent.
>
> Using varargs exclusively can hide bugs.
>
> For example, method(String, String[]) is not the same as method(String
> ...).
While we're at it, looking at such a method, we should consider to test
method(string);
instead of its explicit equivalent
method(string, (String[])null);
Also, when introducing varargs we may not forget to update Javadoc. E.g.
StringUitls.startsWithAny documents:
============= %< ================
* StringUtils.startsWithAny(null, null) = false
* StringUtils.startsWithAny("abcxyz", null) = false
============= %< ================
both calls are ambiguous now and will not compile. Actually there are now 4
cases:
============= %< ================
* StringUtils.startsWithAny(null) = false
* StringUtils.startsWithAny("abcxyz") = false
* StringUtils.startsWithAny(null, (String)null) = false
* StringUtils.startsWithAny("abcxyz", (String)null) = false
============= %< ================
- Jörg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r983219 - /commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/StringUtilsTest.java
Posted by sebb <se...@gmail.com>.
On 9 August 2010 04:52, James Carman <ja...@carmanconsulting.com> wrote:
> On Sun, Aug 8, 2010 at 11:39 PM, sebb <se...@gmail.com> wrote:
>> IMO the tests are now less stringent, e.g. they no longer distinguish
>> leading string parameters from varargs when calling
>> StringUtils.startsWithAny.
>>
>
> And, neither would the user's code. We're using the code exactly as
> the user would be. If we think this makes our API less-friendly, then
> why are we making the change?
>
>> As I wrote before, by all means add new vararg tests, but dropping
>> existing tests does not make sense to me.
>>
>
> I guess we'll have to agree to disagree on this one. I'm not going to
> argue with you if you want to revert it. I was just trying to do some
> code clean-ups. If we're moving to JDK5, then we should code like
> we're using JDK5. Our unit test code should reflect that.
Yes, but the library is also supposed to be usable with user code that
does not use the new features of JDK5.
> Remember, unit test code also serves as documentation to the user about how the
> API is supposed to be used. It's sometimes the only "documentation"
> that actually stays in synch with the code itself. If we have
> explicit String[] creation stuff in our test cases, folks might think
> that's required (or wonder why we're not just using varargs when we
> can).
Exactly.
If we don't have the String[] tests, then users may wonder if they
have to change all their code in order to use the new version.
We need to use both methods to show that both approaches are supported.
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r983219 - /commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/StringUtilsTest.java
Posted by James Carman <ja...@carmanconsulting.com>.
On Sun, Aug 8, 2010 at 11:39 PM, sebb <se...@gmail.com> wrote:
> IMO the tests are now less stringent, e.g. they no longer distinguish
> leading string parameters from varargs when calling
> StringUtils.startsWithAny.
>
And, neither would the user's code. We're using the code exactly as
the user would be. If we think this makes our API less-friendly, then
why are we making the change?
> As I wrote before, by all means add new vararg tests, but dropping
> existing tests does not make sense to me.
>
I guess we'll have to agree to disagree on this one. I'm not going to
argue with you if you want to revert it. I was just trying to do some
code clean-ups. If we're moving to JDK5, then we should code like
we're using JDK5. Our unit test code should reflect that. Remember,
unit test code also serves as documentation to the user about how the
API is supposed to be used. It's sometimes the only "documentation"
that actually stays in synch with the code itself. If we have
explicit String[] creation stuff in our test cases, folks might think
that's required (or wonder why we're not just using varargs when we
can).
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r983219 - /commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/StringUtilsTest.java
Posted by sebb <se...@gmail.com>.
On 9 August 2010 04:08, James Carman <ja...@carmanconsulting.com> wrote:
> On Sun, Aug 8, 2010 at 10:51 PM, sebb <se...@gmail.com> wrote:
>>
>> Although the compiler changes varargs into String[] arrays, the two
>> tests are not equivalent.
>>
>> Using varargs exclusively can hide bugs.
>>
>> For example, method(String, String[]) is not the same as method(String ...).
>>
>
> We're talking about specifically the code I checked in. I believe I'm
> exercising the implementation of the methods exactly the same as it
> was before. It's just being called differently (using varargs).
IMO the tests are now less stringent, e.g. they no longer distinguish
leading string parameters from varargs when calling
StringUtils.startsWithAny.
As I wrote before, by all means add new vararg tests, but dropping
existing tests does not make sense to me.
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r983219 - /commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/StringUtilsTest.java
Posted by James Carman <ja...@carmanconsulting.com>.
On Sun, Aug 8, 2010 at 10:51 PM, sebb <se...@gmail.com> wrote:
>
> Although the compiler changes varargs into String[] arrays, the two
> tests are not equivalent.
>
> Using varargs exclusively can hide bugs.
>
> For example, method(String, String[]) is not the same as method(String ...).
>
We're talking about specifically the code I checked in. I believe I'm
exercising the implementation of the methods exactly the same as it
was before. It's just being called differently (using varargs).
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r983219 - /commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/StringUtilsTest.java
Posted by sebb <se...@gmail.com>.
On 8 August 2010 22:34, James Carman <ja...@carmanconsulting.com> wrote:
> To make sure I'm actually passing in the exact same values (by letting
> the compiler create the arrays using the varargs feature) I added the
> following output to the startsWithAny() method:
>
> System.out.println("startsWithAny(): Received " + string + " input
> with " + (searchStrings == null ? null :
> Arrays.asList(searchStrings).toString()) + " input array");
>
> When running the old version of the test code to the new version,
> there was no difference in the output.
>
> I also ran a similar test by changing getCommonPrefix() method:
>
> System.out.println("getCommonPrefix(): Received " + (strs == null ?
> null : Arrays.asList(strs).toString()) + " input array");
>
> Again, no changes in the output. So, we're at least exercising the
> "guts" of the methods the same. Now I can understand that you might
> be saying that we're not exercising how the code is called the same
> way in both cases, but the varargs support vs. the manual array
> creation is identical. If you think we should add back in the manual
> String[] stuff, I guess we could, but I really don't see much value in
> it other than testing that varargs == manual String[].
>
Although the compiler changes varargs into String[] arrays, the two
tests are not equivalent.
Using varargs exclusively can hide bugs.
For example, method(String, String[]) is not the same as method(String ...).
>
> On Sun, Aug 8, 2010 at 4:26 PM, James Carman <ja...@carmanconsulting.com> wrote:
>> On Sun, Aug 8, 2010 at 1:39 PM, sebb <se...@gmail.com> wrote:
>>>> + assertFalse(StringUtils.startsWithAny("abcxyz", null, "xyz", "abcd"));
>>>
>>> Most of the above changes seem wrong to me, as they change what is being tested.
>>>
>>> By all means add tests for varargs, but surely the array tests need to
>>> be kept as well?
>>>
>>
>> How do they change what is being tested exactly?
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r983219 - /commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/StringUtilsTest.java
Posted by James Carman <ja...@carmanconsulting.com>.
To make sure I'm actually passing in the exact same values (by letting
the compiler create the arrays using the varargs feature) I added the
following output to the startsWithAny() method:
System.out.println("startsWithAny(): Received " + string + " input
with " + (searchStrings == null ? null :
Arrays.asList(searchStrings).toString()) + " input array");
When running the old version of the test code to the new version,
there was no difference in the output.
I also ran a similar test by changing getCommonPrefix() method:
System.out.println("getCommonPrefix(): Received " + (strs == null ?
null : Arrays.asList(strs).toString()) + " input array");
Again, no changes in the output. So, we're at least exercising the
"guts" of the methods the same. Now I can understand that you might
be saying that we're not exercising how the code is called the same
way in both cases, but the varargs support vs. the manual array
creation is identical. If you think we should add back in the manual
String[] stuff, I guess we could, but I really don't see much value in
it other than testing that varargs == manual String[].
On Sun, Aug 8, 2010 at 4:26 PM, James Carman <ja...@carmanconsulting.com> wrote:
> On Sun, Aug 8, 2010 at 1:39 PM, sebb <se...@gmail.com> wrote:
>>> + assertFalse(StringUtils.startsWithAny("abcxyz", null, "xyz", "abcd"));
>>
>> Most of the above changes seem wrong to me, as they change what is being tested.
>>
>> By all means add tests for varargs, but surely the array tests need to
>> be kept as well?
>>
>
> How do they change what is being tested exactly?
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r983219 - /commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/StringUtilsTest.java
Posted by James Carman <ja...@carmanconsulting.com>.
On Sun, Aug 8, 2010 at 1:39 PM, sebb <se...@gmail.com> wrote:
>> + assertFalse(StringUtils.startsWithAny("abcxyz", null, "xyz", "abcd"));
>
> Most of the above changes seem wrong to me, as they change what is being tested.
>
> By all means add tests for varargs, but surely the array tests need to
> be kept as well?
>
How do they change what is being tested exactly?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r983219 - /commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/StringUtilsTest.java
Posted by James Carman <ja...@carmanconsulting.com>.
On Sun, Aug 8, 2010 at 1:39 PM, sebb <se...@gmail.com> wrote:
> StringUtils.length() is now deprecated, and is replaced by
> CharSequenceUtils.length(), but surely the deprecated methods should
> still be tested?
> The deprecation warnings can be suppressed.
>
> When the deprecated method is deleted, the tests can be dropped entirely.
>
> If the corresponding CharSequenceUtils.length() tests don't exist,
> then they should be added.
>
Yes, you're right. I didn't realize the name of the methods and what
they're testing. I was in clean-up mode trying to remove all the
little yellow lines from the right side of my code editor. I'll try
to revert at least these. I don't agree that I'm changing the tests
in the varargs cases, though.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r983219 - /commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/StringUtilsTest.java
Posted by sebb <se...@gmail.com>.
On 7 August 2010 13:04, <jc...@apache.org> wrote:
> Author: jcarman
> Date: Sat Aug 7 12:04:46 2010
> New Revision: 983219
>
> URL: http://svn.apache.org/viewvc?rev=983219&view=rev
> Log:
> Some code clean-up. Removing unnecessary boxing, array creation for varargs, etc.
>
> Modified:
> commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/StringUtilsTest.java
>
> Modified: commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/StringUtilsTest.java
> URL: http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/StringUtilsTest.java?rev=983219&r1=983218&r2=983219&view=diff
> ==============================================================================
> --- commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/StringUtilsTest.java (original)
> +++ commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/StringUtilsTest.java Sat Aug 7 12:04:46 2010
...
> @@ -1330,28 +1330,28 @@ public class StringUtilsTest extends Tes
> }
>
> public void testLengthString() {
> - assertEquals(0, StringUtils.length(null));
> - assertEquals(0, StringUtils.length(""));
> - assertEquals(0, StringUtils.length(StringUtils.EMPTY));
> - assertEquals(1, StringUtils.length("A"));
> - assertEquals(1, StringUtils.length(" "));
> - assertEquals(8, StringUtils.length("ABCDEFGH"));
> + assertEquals(0, CharSequenceUtils.length(null));
> + assertEquals(0, CharSequenceUtils.length(""));
> + assertEquals(0, CharSequenceUtils.length(StringUtils.EMPTY));
> + assertEquals(1, CharSequenceUtils.length("A"));
> + assertEquals(1, CharSequenceUtils.length(" "));
> + assertEquals(8, CharSequenceUtils.length("ABCDEFGH"));
> }
StringUtils.length() is now deprecated, and is replaced by
CharSequenceUtils.length(), but surely the deprecated methods should
still be tested?
The deprecation warnings can be suppressed.
When the deprecated method is deleted, the tests can be dropped entirely.
If the corresponding CharSequenceUtils.length() tests don't exist,
then they should be added.
>
> public void testLengthStringBuffer() {
> - assertEquals(0, StringUtils.length(new StringBuffer("")));
> - assertEquals(0, StringUtils.length(new StringBuffer(StringUtils.EMPTY)));
> - assertEquals(1, StringUtils.length(new StringBuffer("A")));
> - assertEquals(1, StringUtils.length(new StringBuffer(" ")));
> - assertEquals(8, StringUtils.length(new StringBuffer("ABCDEFGH")));
> + assertEquals(0, CharSequenceUtils.length(new StringBuffer("")));
> + assertEquals(0, CharSequenceUtils.length(new StringBuffer(StringUtils.EMPTY)));
> + assertEquals(1, CharSequenceUtils.length(new StringBuffer("A")));
> + assertEquals(1, CharSequenceUtils.length(new StringBuffer(" ")));
> + assertEquals(8, CharSequenceUtils.length(new StringBuffer("ABCDEFGH")));
> }
As above
>
> public void testLengthStringBuilder() {
> - assertEquals(0, StringUtils.length(new StringBuilder("")));
> - assertEquals(0, StringUtils.length(new StringBuilder(StringUtils.EMPTY)));
> - assertEquals(1, StringUtils.length(new StringBuilder("A")));
> - assertEquals(1, StringUtils.length(new StringBuilder(" ")));
> - assertEquals(8, StringUtils.length(new StringBuilder("ABCDEFGH")));
> + assertEquals(0, CharSequenceUtils.length(new StringBuilder("")));
> + assertEquals(0, CharSequenceUtils.length(new StringBuilder(StringUtils.EMPTY)));
> + assertEquals(1, CharSequenceUtils.length(new StringBuilder("A")));
> + assertEquals(1, CharSequenceUtils.length(new StringBuilder(" ")));
> + assertEquals(8, CharSequenceUtils.length(new StringBuilder("ABCDEFGH")));
> }
As above
> @@ -1826,33 +1826,33 @@ public class StringUtilsTest extends Tes
> }
>
> public void testGetCommonPrefix_StringArray(){
> - assertEquals("", StringUtils.getCommonPrefix(null));
> - assertEquals("", StringUtils.getCommonPrefix(new String[] {}));
> - assertEquals("abc", StringUtils.getCommonPrefix(new String[] {"abc"}));
> - assertEquals("", StringUtils.getCommonPrefix(new String[] {null, null}));
> - assertEquals("", StringUtils.getCommonPrefix(new String[] {"", ""}));
> - assertEquals("", StringUtils.getCommonPrefix(new String[] {"", null}));
> - assertEquals("", StringUtils.getCommonPrefix(new String[] {"abc", null, null}));
> - assertEquals("", StringUtils.getCommonPrefix(new String[] {null, null, "abc"}));
> - assertEquals("", StringUtils.getCommonPrefix(new String[] {"", "abc"}));
> - assertEquals("", StringUtils.getCommonPrefix(new String[] {"abc", ""}));
> - assertEquals("abc", StringUtils.getCommonPrefix(new String[] {"abc", "abc"}));
> - assertEquals("a", StringUtils.getCommonPrefix(new String[] {"abc", "a"}));
> - assertEquals("ab", StringUtils.getCommonPrefix(new String[] {"ab", "abxyz"}));
> - assertEquals("ab", StringUtils.getCommonPrefix(new String[] {"abcde", "abxyz"}));
> - assertEquals("", StringUtils.getCommonPrefix(new String[] {"abcde", "xyz"}));
> - assertEquals("", StringUtils.getCommonPrefix(new String[] {"xyz", "abcde"}));
> - assertEquals("i am a ", StringUtils.getCommonPrefix(new String[] {"i am a machine", "i am a robot"}));
> + assertEquals("", StringUtils.getCommonPrefix((String[])null));
> + assertEquals("", StringUtils.getCommonPrefix());
> + assertEquals("abc", StringUtils.getCommonPrefix("abc"));
> + assertEquals("", StringUtils.getCommonPrefix(null, null));
> + assertEquals("", StringUtils.getCommonPrefix("", ""));
> + assertEquals("", StringUtils.getCommonPrefix("", null));
> + assertEquals("", StringUtils.getCommonPrefix("abc", null, null));
> + assertEquals("", StringUtils.getCommonPrefix(null, null, "abc"));
> + assertEquals("", StringUtils.getCommonPrefix("", "abc"));
> + assertEquals("", StringUtils.getCommonPrefix("abc", ""));
> + assertEquals("abc", StringUtils.getCommonPrefix("abc", "abc"));
> + assertEquals("a", StringUtils.getCommonPrefix("abc", "a"));
> + assertEquals("ab", StringUtils.getCommonPrefix("ab", "abxyz"));
> + assertEquals("ab", StringUtils.getCommonPrefix("abcde", "abxyz"));
> + assertEquals("", StringUtils.getCommonPrefix("abcde", "xyz"));
> + assertEquals("", StringUtils.getCommonPrefix("xyz", "abcde"));
> + assertEquals("i am a ", StringUtils.getCommonPrefix("i am a machine", "i am a robot"));
> }
>
> public void testStartsWithAny() {
> - assertFalse(StringUtils.startsWithAny(null, null));
> - assertFalse(StringUtils.startsWithAny(null, new String[] {"abc"}));
> - assertFalse(StringUtils.startsWithAny("abcxyz", null));
> - assertFalse(StringUtils.startsWithAny("abcxyz", new String[] {}));
> - assertTrue(StringUtils.startsWithAny("abcxyz", new String[] {"abc"}));
> - assertTrue(StringUtils.startsWithAny("abcxyz", new String[] {null, "xyz", "abc"}));
> - assertFalse(StringUtils.startsWithAny("abcxyz", new String[] {null, "xyz", "abcd"}));
> + assertFalse(StringUtils.startsWithAny(null, (String[])null));
> + assertFalse(StringUtils.startsWithAny(null, "abc"));
> + assertFalse(StringUtils.startsWithAny("abcxyz", (String[])null));
> + assertFalse(StringUtils.startsWithAny("abcxyz"));
> + assertTrue(StringUtils.startsWithAny("abcxyz", "abc"));
> + assertTrue(StringUtils.startsWithAny("abcxyz", null, "xyz", "abc"));
> + assertFalse(StringUtils.startsWithAny("abcxyz", null, "xyz", "abcd"));
Most of the above changes seem wrong to me, as they change what is being tested.
By all means add tests for varargs, but surely the array tests need to
be kept as well?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org