[GitHub] [commons-io] garydgregory commented on a change in pull request #117: Prevent NullPointerException in ReversedLinesFileReader constructor

[GitBox <gi...@apache.org>]

garydgregory commented on a change in pull request #117:
URL: https://github.com/apache/commons-io/pull/117#discussion_r432887640



##########
File path: src/test/java/org/apache/commons/io/input/ReversedLinesFileReaderTestSimple.java
##########
@@ -66,6 +67,10 @@ public void testUnsupportedEncodingBig5() throws URISyntaxException {
                 () -> new ReversedLinesFileReader(testFileEncodingBig5, IOUtils.DEFAULT_BUFFER_SIZE, "Big5").close());
     }
 
-
+    @Test
+    public void testNullEncoding() throws IOException, URISyntaxException {
+        new ReversedLinesFileReader(new File(this.getClass().getResource("/test-file-empty.bin").toURI()),
+                                    (Charset) null);
+    }

Review comment:
       Hi @mernst,
    
   A better way to test would be to write to the temp file with `Charset.defaultCharset()` and still read with a `null` `Charset`. 
   
   From a black-box perspective, writing with a `null` `Charset` does not prove anything since you're relying on the fact that another API does the right thing with a `null` `Charset`. This would really match the test with expectations.
   

##########
File path: src/test/java/org/apache/commons/io/input/ReversedLinesFileReaderTestSimple.java
##########
@@ -66,6 +74,14 @@ public void testUnsupportedEncodingBig5() throws URISyntaxException {
                 () -> new ReversedLinesFileReader(testFileEncodingBig5, IOUtils.DEFAULT_BUFFER_SIZE, "Big5").close());
     }
 
-
+    @Test
+    public void testNullEncoding() throws IOException, URISyntaxException {
+        final File file = new File(temporaryFolder, "write.txt");
+        final String text = "Hello /u1234";

Review comment:
       Ping. Why is the weird Unicode escape needed here? If it is needed, please add a comment, otherwise, we don't need it right?

##########
File path: src/test/java/org/apache/commons/io/input/ReversedLinesFileReaderTestSimple.java
##########
@@ -66,6 +74,14 @@ public void testUnsupportedEncodingBig5() throws URISyntaxException {
                 () -> new ReversedLinesFileReader(testFileEncodingBig5, IOUtils.DEFAULT_BUFFER_SIZE, "Big5").close());
     }
 
-
+    @Test
+    public void testNullEncoding() throws IOException, URISyntaxException {
+        final File file = new File(temporaryFolder, "write.txt");
+        final String text = "Hello /u1234";

Review comment:
       What is `/u1234` supposed to be?




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