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



##########
File path: 
src/test/java/org/apache/commons/io/FileUtilsCopyDirectoryToDirectoryTestCase.java
##########
@@ -73,6 +83,48 @@ public void 
copyDirectoryToDirectoryThrowsNullPointerExceptionWithCorrectMessage
         assertExceptionTypeAndMessage(srcDir, destDir, 
NullPointerException.class, "destinationDir");
     }
 
+
+    @Test
+    public void copyFileWrongPermissions() throws IOException {
+        
+        
+        final File destDir = createTemporaryFolderIn(null);
+        final  Path srcFile = Files.createTempFile("tmp-output", ".xml");
+        final Path path = Paths.get(destDir.getAbsolutePath(), "newFile.xml");
+
+        try {
+            FileUtils.copyFile(srcFile.toFile(), path.toFile());
+        } catch (IllegalArgumentException iae) {
+               iae.printStackTrace();

Review comment:
       Don't catch and log to sys err. Just let the exception percolate up.

##########
File path: 
src/test/java/org/apache/commons/io/FileUtilsCopyDirectoryToDirectoryTestCase.java
##########
@@ -35,6 +40,11 @@
 
     @TempDir
     public File temporaryFolder;
+    

Review comment:
       Delete extra blank line.

##########
File path: 
src/test/java/org/apache/commons/io/FileUtilsCopyDirectoryToDirectoryTestCase.java
##########
@@ -73,6 +83,48 @@ public void 
copyDirectoryToDirectoryThrowsNullPointerExceptionWithCorrectMessage
         assertExceptionTypeAndMessage(srcDir, destDir, 
NullPointerException.class, "destinationDir");
     }
 
+
+    @Test
+    public void copyFileWrongPermissions() throws IOException {
+        
+        
+        final File destDir = createTemporaryFolderIn(null);
+        final  Path srcFile = Files.createTempFile("tmp-output", ".xml");
+        final Path path = Paths.get(destDir.getAbsolutePath(), "newFile.xml");
+
+        try {
+            FileUtils.copyFile(srcFile.toFile(), path.toFile());
+        } catch (IllegalArgumentException iae) {
+               iae.printStackTrace();
+        }
+        
+        
assertTrue(Files.getPosixFilePermissions(path).contains(PosixFilePermission.OTHERS_READ),
 Files.getPosixFilePermissions(path).toString());
+
+    }
+    
+    
+    
+    private File createTemporaryFolderIn(File parentFolder) throws IOException 
{

Review comment:
       What is this all about? Since we are testing `FileUtils.copyFile()`, why 
do we need an extra level of temp folder beyond what the existing 
`temporaryFolder` provide?

##########
File path: 
src/test/java/org/apache/commons/io/FileUtilsCopyDirectoryToDirectoryTestCase.java
##########
@@ -73,6 +83,48 @@ public void 
copyDirectoryToDirectoryThrowsNullPointerExceptionWithCorrectMessage
         assertExceptionTypeAndMessage(srcDir, destDir, 
NullPointerException.class, "destinationDir");
     }
 
+
+    @Test
+    public void copyFileWrongPermissions() throws IOException {
+        

Review comment:
       Delete extra blank lines.




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


Reply via email to