[ https://issues.apache.org/jira/browse/WW-4043?focusedWorklogId=427396&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-427396 ]
ASF GitHub Bot logged work on WW-4043: -------------------------------------- Author: ASF GitHub Bot Created on: 26/Apr/20 21:08 Start Date: 26/Apr/20 21:08 Worklog Time Spent: 10m Work Description: 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: us...@infra.apache.org Issue Time Tracking ------------------- Worklog Id: (was: 427396) Time Spent: 0.5h (was: 20m) > Duplicated class TestUtils > -------------------------- > > Key: WW-4043 > URL: https://issues.apache.org/jira/browse/WW-4043 > Project: Struts 2 > Issue Type: Improvement > Components: Plugin - JSON > Affects Versions: 2.3.14 > Reporter: Maurizio Cucchiara > Priority: Minor > Labels: JSON > Fix For: 2.6 > > Time Spent: 0.5h > Remaining Estimate: 0h > > Currently there are 2 TestUtils which basically do the same thing. > Since they are on test directory, ATM there is no way to reuse the one > present on core module. > The only way I can prefigure is by moving TestUtils class inside Junit Plugin > (after all is a test utility class and -either the core and- json plugin > depend on junit plugin). > -- This message was sent by Atlassian Jira (v8.3.4#803005)