You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@struts.apache.org by GitBox <gi...@apache.org> on 2020/04/26 07:32:12 UTC

[GitHub] [struts] lukaszlenart opened a new pull request #408: [WW-4043] Moves TestUtils into junit-plugin

lukaszlenart opened a new pull request #408:
URL: https://github.com/apache/struts/pull/408


   Resolves [WW-4043](https://issues.apache.org/jira/browse/WW-4043)


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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

[GitHub] [struts] lukaszlenart commented on a change in pull request #408: [WW-4043] Moves TestUtils into junit-plugin

Posted by GitBox <gi...@apache.org>.
lukaszlenart commented on a change in pull request #408:
URL: https://github.com/apache/struts/pull/408#discussion_r416344768



##########
File path: plugins/junit/src/main/java/org/apache/struts2/util/TestUtils.java
##########
@@ -85,13 +79,13 @@ public static boolean compare(URL url, String text) throws Exception {
     public static void assertEquals(URL source, String text) throws Exception {
         String writerString = TestUtils.normalize(text, true);
         String bufferString = TestUtils.normalize(readContent(source), true);
-        Assert.assertEquals(bufferString,writerString);
+        Assert.assertEquals(bufferString, writerString);
     }
 
     public static String readContent(URL url) throws Exception {
         if (url == null)
             throw new Exception("unable to verify a null URL");
 
-        return IOUtils.toString(url.openStream());
+        return IOUtils.toString(url.openStream(), StandardCharsets.UTF_8);

Review comment:
       Done :)




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [struts] coveralls edited a comment on pull request #408: [WW-4043] Moves TestUtils into junit-plugin

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #408:
URL: https://github.com/apache/struts/pull/408#issuecomment-619503812


   
   [![Coverage Status](https://coveralls.io/builds/30391170/badge)](https://coveralls.io/builds/30391170)
   
   Coverage decreased (-0.02%) to 49.164% when pulling **1e1443871b4fc93f37cbf8347f980d3ee49b260c on WW-4043-moves-test-utils** into **3ccbcc6f9c9f84f6f1e5644e1e266754d88a8017 on master**.
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [struts] lukaszlenart commented on pull request #408: [WW-4043] Moves TestUtils into junit-plugin

Posted by GitBox <gi...@apache.org>.
lukaszlenart commented on pull request #408:
URL: https://github.com/apache/struts/pull/408#issuecomment-622250771


   LGTM 👍 


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [struts] JCgH4164838Gh792C124B5 commented on a change in pull request #408: [WW-4043] Moves TestUtils into junit-plugin

Posted by GitBox <gi...@apache.org>.
JCgH4164838Gh792C124B5 commented on a change in pull request #408:
URL: https://github.com/apache/struts/pull/408#discussion_r415400292



##########
File path: plugins/junit/src/main/java/org/apache/struts2/util/TestUtils.java
##########
@@ -85,13 +79,13 @@ public static boolean compare(URL url, String text) throws Exception {
     public static void assertEquals(URL source, String text) throws Exception {
         String writerString = TestUtils.normalize(text, true);
         String bufferString = TestUtils.normalize(readContent(source), true);
-        Assert.assertEquals(bufferString,writerString);
+        Assert.assertEquals(bufferString, writerString);
     }
 
     public static String readContent(URL url) throws Exception {
         if (url == null)
             throw new Exception("unable to verify a null URL");
 
-        return IOUtils.toString(url.openStream());
+        return IOUtils.toString(url.openStream(), StandardCharsets.UTF_8);

Review comment:
       It appears the previous implementation would have implicitly used `Charset.defaultCharset()` for processing the `URL`.  Some users might want (or need) a way to specify the `Charset` for processing the `URL`, so maybe a small refactoring into:
   
   ```
       public static String readContent(final URL url) throws Exception {
           return readContent(url, StandardCharsets.UTF_8);
       }
   
       public static String readContent(final URL url, final Charset encoding) throws Exception {
           if (url == null) {
               throw new IllegalArgumentException("Unable to verify a null URL");
           }
           if (encoding == null) {
               throw new IllegalArgumentException("Unable to verify the URL using a null Charset");
           }
   
           return IOUtils.toString(url.openStream(), encoding);
       }
   ```
   might be useful ?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [struts] coveralls commented on pull request #408: [WW-4043] Moves TestUtils into junit-plugin

Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #408:
URL: https://github.com/apache/struts/pull/408#issuecomment-619503812


   
   [![Coverage Status](https://coveralls.io/builds/30345949/badge)](https://coveralls.io/builds/30345949)
   
   Coverage decreased (-0.01%) to 49.167% when pulling **592c942c764c3eeca69ca04a4e93f824dcbd6a53 on WW-4043-moves-test-utils** into **3ccbcc6f9c9f84f6f1e5644e1e266754d88a8017 on master**.
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org