[jira] [Commented] (ZOOKEEPER-2284) LogFormatter and SnapshotFormatter does not handle FileNotFoundException gracefully

[Edward Ribeiro (JIRA)]

    [ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2284?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14935339#comment-14935339
 ] 

Edward Ribeiro commented on ZOOKEEPER-2284:
-------------------------------------------

Wow, very nice addition of a helper class + unit test. Some minor observations:

1. We can use temporary files in {{testValidateInpuFile}}:

{code}
testValidateFileInput() throws IOException {
     File file = new File(testData, "file.txt");
     file.createNewFile();
{code}

can be rewritten as:
{code}
testValidateFileInput() throws IOException {
        File file = File.createTempFile("file-", "txt", testData);
        file.deleteOnExit();
{code}

2. We can add a unit test to check the readability of the file as:

{code}
testUnreadableFileInput() throws IOException {
        File file = File.createTempFile("file-", "txt", testData);
        file.setReadable(false, false);
        file.deleteOnExit();
{code}

*IMHO*, I would write {{validateFileInput}} method by throwing exceptions and 
returning void if everything is okay. Therefore, in the unit tests I would 
structure them as:

{code}
   try {
      validateFileInput(...)
      fail("Should throw IOException")
   }
   catch (IOException ie) {
         assertEquals("File file-202.txt doesn't exists", ie.getMessage());
   }
{code}

*But I am definitely okay with the current way they are, so up to you. :)*

> LogFormatter and SnapshotFormatter does not handle FileNotFoundException 
> gracefully
> -----------------------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-2284
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2284
>             Project: ZooKeeper
>          Issue Type: Bug
>    Affects Versions: 3.5.0
>            Reporter: Arshad Mohammad
>            Assignee: Arshad Mohammad
>            Priority: Minor
>             Fix For: 3.5.2
>
>         Attachments: ZOOKEEPER-2284-01.patch, ZOOKEEPER-2284-02.patch
>
>
> {{LogFormatter}} and {{SnapshotFormatter}} does not handle 
> FileNotFoundException gracefully. If file no exist then these classes 
> propagate the exception to console.
> {code}
> Exception in thread "main" java.io.FileNotFoundException: log.1 (The system 
> cannot find the file specified)
>         at java.io.FileInputStream.open(Native Method)
>         at java.io.FileInputStream.<init>(FileInputStream.java:146)
>         at java.io.FileInputStream.<init>(FileInputStream.java:101)
>         at org.apache.zookeeper.server.LogFormatter.main(LogFormatter.java:49)
> {code}
>  File existence should be validated and appropriate message should be 
> displayed on console if file does not exist



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)