You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2012/04/06 20:44:19 UTC

svn commit: r1310541 - in /tomcat/trunk/test/javax: el/TestCompositeELResolver.java servlet/jsp/TestPageContext.java

Author: markt
Date: Fri Apr  6 18:44:19 2012
New Revision: 1310541

URL: http://svn.apache.org/viewvc?rev=1310541&view=rev
Log:
Don't use static imports

Modified:
    tomcat/trunk/test/javax/el/TestCompositeELResolver.java
    tomcat/trunk/test/javax/servlet/jsp/TestPageContext.java

Modified: tomcat/trunk/test/javax/el/TestCompositeELResolver.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/javax/el/TestCompositeELResolver.java?rev=1310541&r1=1310540&r2=1310541&view=diff
==============================================================================
--- tomcat/trunk/test/javax/el/TestCompositeELResolver.java (original)
+++ tomcat/trunk/test/javax/el/TestCompositeELResolver.java Fri Apr  6 18:44:19 2012
@@ -20,8 +20,7 @@ import java.io.File;
 
 import javax.servlet.http.HttpServletResponse;
 
-import static org.junit.Assert.assertEquals;
-
+import org.junit.Assert;
 import org.junit.Test;
 
 import org.apache.catalina.core.StandardContext;
@@ -49,6 +48,6 @@ public class TestCompositeELResolver ext
         int rc = getUrl("http://localhost:" + getPort() +
                 "/test/bug50408.jsp", new ByteChunk(), null);
 
-        assertEquals(HttpServletResponse.SC_OK, rc);
+        Assert.assertEquals(HttpServletResponse.SC_OK, rc);
     }
 }

Modified: tomcat/trunk/test/javax/servlet/jsp/TestPageContext.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/javax/servlet/jsp/TestPageContext.java?rev=1310541&r1=1310540&r2=1310541&view=diff
==============================================================================
--- tomcat/trunk/test/javax/servlet/jsp/TestPageContext.java (original)
+++ tomcat/trunk/test/javax/servlet/jsp/TestPageContext.java Fri Apr  6 18:44:19 2012
@@ -19,8 +19,7 @@ package javax.servlet.jsp;
 
 import java.io.File;
 
-import static org.junit.Assert.assertTrue;
-
+import org.junit.Assert;
 import org.junit.Test;
 
 import org.apache.catalina.startup.Tomcat;
@@ -43,6 +42,6 @@ public class TestPageContext extends Tom
                 "/test/bug49nnn/bug49196.jsp");
 
         String result = res.toString();
-        assertTrue(result.contains("OK"));
+        Assert.assertTrue(result.contains("OK"));
     }
 }



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


Re: svn commit: r1310541 - in /tomcat/trunk/test/javax: el/TestCompositeELResolver.java servlet/jsp/TestPageContext.java

Posted by Konstantin Kolinko <kn...@gmail.com>.
2012/4/7 Konstantin Kolinko <kn...@gmail.com>:
> 2012/4/7 Mark Thomas <ma...@apache.org>:
>> On 06/04/2012 22:16, Konstantin Kolinko wrote:
>>> 2012/4/6  <ma...@apache.org>:
>>>> Author: markt
>>>> Date: Fri Apr  6 18:44:19 2012
>>>> New Revision: 1310541
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1310541&view=rev
>>>> Log:
>>>> Don't use static imports
>>>
>>> ?
>>>
>>> 1. I would say that I'd prefer to use static imports in JUnit tests.
>>> They cause no confusion here and are easier to read
>>>
>>> (and using static imports for those assert methods is what is
>>> recommended in many places).
>>
>> Having seen other projects avoid them and then compared the two styles I
>> prefer not using them.
>
> In general I'd also prefer not to use them, but JUnit tests are a
> special exception where this usage is well-known.
>
> I think we already discussed this (and thus are our checkstyle rules).
>

Maybe it was not really discussed, but here are related threads:

http://tomcat.markmail.org/thread/slydzz6hu6mrkzgd
- my proposal to update to JUnit 4  and discussion of it

http://svn.apache.org/viewvc?view=revision&revision=1150644
- my update to checkstyle configuration, as part of migration to JUnit 4
allowing this static import

http://svn.apache.org/viewvc?view=revision&revision=1150690
- my update to checkstyle configuration, as part of migration to JUnit 4
updating import order rule


>>> 2. It should be already allowed by configuration in checkstyle.xml,
>>> so was there a problem?
>>
>> No problem. Just a personal itch.
>>
>> I was going to work my way through the remainder if nobody complained
>> but I am happy to revert it if folks prefer the static imports.
>>
>>

Best regards,
Konstantin Kolinko

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


Re: svn commit: r1310541 - in /tomcat/trunk/test/javax: el/TestCompositeELResolver.java servlet/jsp/TestPageContext.java

Posted by Konstantin Kolinko <kn...@gmail.com>.
2012/4/7 Mark Thomas <ma...@apache.org>:
> On 06/04/2012 22:16, Konstantin Kolinko wrote:
>> 2012/4/6  <ma...@apache.org>:
>>> Author: markt
>>> Date: Fri Apr  6 18:44:19 2012
>>> New Revision: 1310541
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1310541&view=rev
>>> Log:
>>> Don't use static imports
>>
>> ?
>>
>> 1. I would say that I'd prefer to use static imports in JUnit tests.
>> They cause no confusion here and are easier to read
>>
>> (and using static imports for those assert methods is what is
>> recommended in many places).
>
> Having seen other projects avoid them and then compared the two styles I
> prefer not using them.

In general I'd also prefer not to use them, but JUnit tests are a
special exception where this usage is well-known.

I think we already discussed this (and thus are our checkstyle rules).

>> 2. It should be already allowed by configuration in checkstyle.xml,
>> so was there a problem?
>
> No problem. Just a personal itch.
>
> I was going to work my way through the remainder if nobody complained
> but I am happy to revert it if folks prefer the static imports.
>
>

Best regards,
Konstantin Kolinko

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


Re: svn commit: r1310541 - in /tomcat/trunk/test/javax: el/TestCompositeELResolver.java servlet/jsp/TestPageContext.java

Posted by Mark Thomas <ma...@apache.org>.
On 06/04/2012 22:16, Konstantin Kolinko wrote:
> 2012/4/6  <ma...@apache.org>:
>> Author: markt
>> Date: Fri Apr  6 18:44:19 2012
>> New Revision: 1310541
>>
>> URL: http://svn.apache.org/viewvc?rev=1310541&view=rev
>> Log:
>> Don't use static imports
> 
> ?
> 
> 1. I would say that I'd prefer to use static imports in JUnit tests.
> They cause no confusion here and are easier to read
> 
> (and using static imports for those assert methods is what is
> recommended in many places).

Having seen other projects avoid them and then compared the two styles I
prefer not using them.

> 2. It should be already allowed by configuration in checkstyle.xml,
> so was there a problem?

No problem. Just a personal itch.

I was going to work my way through the remainder if nobody complained
but I am happy to revert it if folks prefer the static imports.

Mark

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


Re: svn commit: r1310541 - in /tomcat/trunk/test/javax: el/TestCompositeELResolver.java servlet/jsp/TestPageContext.java

Posted by Konstantin Kolinko <kn...@gmail.com>.
2012/4/6  <ma...@apache.org>:
> Author: markt
> Date: Fri Apr  6 18:44:19 2012
> New Revision: 1310541
>
> URL: http://svn.apache.org/viewvc?rev=1310541&view=rev
> Log:
> Don't use static imports

?

1. I would say that I'd prefer to use static imports in JUnit tests.
They cause no confusion here and are easier to read

(and using static imports for those assert methods is what is
recommended in many places).

2. It should be already allowed by configuration in checkstyle.xml,
so was there a problem?

    <module name="AvoidStaticImport">
        <property name="excludes"
                  value="org.apache.catalina.startup.SimpleHttpClient.CRLF"/>
        <property name="excludes" value="org.junit.Assert.*"/>
    </module>

Best regards,
Konstantin Kolinko

> Modified:
>    tomcat/trunk/test/javax/el/TestCompositeELResolver.java
>    tomcat/trunk/test/javax/servlet/jsp/TestPageContext.java
>
> Modified: tomcat/trunk/test/javax/el/TestCompositeELResolver.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/test/javax/el/TestCompositeELResolver.java?rev=1310541&r1=1310540&r2=1310541&view=diff
> ==============================================================================
> --- tomcat/trunk/test/javax/el/TestCompositeELResolver.java (original)
> +++ tomcat/trunk/test/javax/el/TestCompositeELResolver.java Fri Apr  6 18:44:19 2012
> @@ -20,8 +20,7 @@ import java.io.File;
>
>  import javax.servlet.http.HttpServletResponse;
>
> -import static org.junit.Assert.assertEquals;
> -
> +import org.junit.Assert;
>  import org.junit.Test;
>
>  import org.apache.catalina.core.StandardContext;
> @@ -49,6 +48,6 @@ public class TestCompositeELResolver ext
>         int rc = getUrl("http://localhost:" + getPort() +
>                 "/test/bug50408.jsp", new ByteChunk(), null);
>
> -        assertEquals(HttpServletResponse.SC_OK, rc);
> +        Assert.assertEquals(HttpServletResponse.SC_OK, rc);
>     }
>  }
>
> Modified: tomcat/trunk/test/javax/servlet/jsp/TestPageContext.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/test/javax/servlet/jsp/TestPageContext.java?rev=1310541&r1=1310540&r2=1310541&view=diff
> ==============================================================================
> --- tomcat/trunk/test/javax/servlet/jsp/TestPageContext.java (original)
> +++ tomcat/trunk/test/javax/servlet/jsp/TestPageContext.java Fri Apr  6 18:44:19 2012
> @@ -19,8 +19,7 @@ package javax.servlet.jsp;
>
>  import java.io.File;
>
> -import static org.junit.Assert.assertTrue;
> -
> +import org.junit.Assert;
>  import org.junit.Test;
>
>  import org.apache.catalina.startup.Tomcat;
> @@ -43,6 +42,6 @@ public class TestPageContext extends Tom
>                 "/test/bug49nnn/bug49196.jsp");
>
>         String result = res.toString();
> -        assertTrue(result.contains("OK"));
> +        Assert.assertTrue(result.contains("OK"));
>     }
>  }
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>

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