You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Gary Gregory <ga...@gmail.com> on 2014/03/01 17:13:52 UTC

Re: svn commit: r1573188 - /commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/StringEscapeUtilsTest.java

Shouldn't a try/finally clause be used here?

Or, move to Java 7 after 3.3 and use try-with-resources all over.

I know this is test code... but I am sure there are plenty of other bullet
proofing opportunities.

Gary


On Sat, Mar 1, 2014 at 11:11 AM, <se...@apache.org> wrote:

> Author: sebb
> Date: Sat Mar  1 16:11:01 2014
> New Revision: 1573188
>
> URL: http://svn.apache.org/r1573188
> Log:
> Close resource
>
> Modified:
>
> commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/StringEscapeUtilsTest.java
>
> Modified:
> commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/StringEscapeUtilsTest.java
> URL:
> http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/StringEscapeUtilsTest.java?rev=1573188&r1=1573187&r2=1573188&view=diff
>
> ==============================================================================
> ---
> commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/StringEscapeUtilsTest.java
> (original)
> +++
> commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/StringEscapeUtilsTest.java
> Sat Mar  1 16:11:01 2014
> @@ -557,12 +557,14 @@ public class StringEscapeUtilsTest {
>       */
>      @Test
>      public void testLang708() throws IOException {
> -        final String input = IOUtils.toString(new
> FileInputStream("src/test/resources/lang-708-input.txt"), "UTF-8");
> +        final FileInputStream fis = new
> FileInputStream("src/test/resources/lang-708-input.txt");
> +        final String input = IOUtils.toString(fis, "UTF-8");
>          final String escaped = StringEscapeUtils.escapeEcmaScript(input);
>          // just the end:
>          assertTrue(escaped, escaped.endsWith("}]"));
>          // a little more:
>          assertTrue(escaped,
> escaped.endsWith("\"valueCode\\\":\\\"\\\"}]"));
> +        fis.close();
>      }
>
>      /**
>
>
>


-- 
E-Mail: garydgregory@gmail.com | ggregory@apache.org
Java Persistence with Hibernate, Second Edition<http://www.manning.com/bauer3/>
JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
Spring Batch in Action <http://www.manning.com/templier/>
Blog: http://garygregory.wordpress.com
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory

Re: svn commit: r1573188 - /commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/StringEscapeUtilsTest.java

Posted by sebb <se...@gmail.com>.
On 1 March 2014 16:13, Gary Gregory <ga...@gmail.com> wrote:
> Shouldn't a try/finally clause be used here?

I don't think it's necessary to tidyup test code that fails, because
the intention is that test code does not generally fail.
But it's not a good idea if successful test code leaks resources, as
that could potentially cause a difficult to debug failure elsewhere.
Or the leak might leave behind test files that mess up later tests. Etc.

However main code is expected to throw errors and should of course
ensure that resources are always closed.

> Or, move to Java 7 after 3.3 and use try-with-resources all over.
>
> I know this is test code... but I am sure there are plenty of other bullet
> proofing opportunities.

In this case it's not a bullet-prrofing exercise.

It's an easy fix which reduces the number of warnings.
This makes it easier to spot any real issues.

> Gary
>
>
> On Sat, Mar 1, 2014 at 11:11 AM, <se...@apache.org> wrote:
>
>> Author: sebb
>> Date: Sat Mar  1 16:11:01 2014
>> New Revision: 1573188
>>
>> URL: http://svn.apache.org/r1573188
>> Log:
>> Close resource
>>
>> Modified:
>>
>> commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/StringEscapeUtilsTest.java
>>
>> Modified:
>> commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/StringEscapeUtilsTest.java
>> URL:
>> http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/StringEscapeUtilsTest.java?rev=1573188&r1=1573187&r2=1573188&view=diff
>>
>> ==============================================================================
>> ---
>> commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/StringEscapeUtilsTest.java
>> (original)
>> +++
>> commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/StringEscapeUtilsTest.java
>> Sat Mar  1 16:11:01 2014
>> @@ -557,12 +557,14 @@ public class StringEscapeUtilsTest {
>>       */
>>      @Test
>>      public void testLang708() throws IOException {
>> -        final String input = IOUtils.toString(new
>> FileInputStream("src/test/resources/lang-708-input.txt"), "UTF-8");
>> +        final FileInputStream fis = new
>> FileInputStream("src/test/resources/lang-708-input.txt");
>> +        final String input = IOUtils.toString(fis, "UTF-8");
>>          final String escaped = StringEscapeUtils.escapeEcmaScript(input);
>>          // just the end:
>>          assertTrue(escaped, escaped.endsWith("}]"));
>>          // a little more:
>>          assertTrue(escaped,
>> escaped.endsWith("\"valueCode\\\":\\\"\\\"}]"));
>> +        fis.close();
>>      }
>>
>>      /**
>>
>>
>>
>
>
> --
> E-Mail: garydgregory@gmail.com | ggregory@apache.org
> Java Persistence with Hibernate, Second Edition<http://www.manning.com/bauer3/>
> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
> Spring Batch in Action <http://www.manning.com/templier/>
> Blog: http://garygregory.wordpress.com
> Home: http://garygregory.com/
> Tweet! http://twitter.com/GaryGregory

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org