[ 
https://issues.apache.org/jira/browse/DERBY-2342?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12473861
 ] 

Suresh Thalamati commented on DERBY-2342:
-----------------------------------------

Thanks for converting this test to junit , Andrew.   Patch looks good to me., I 
don't know much about junit.. 
Just couple of  trivial  comments/questions.

In the old tests harness I thought the  policy was  test  files shoud  not be 
accessed under priveleged blocks.  In  the new junit framework
is it necessary to add priveleged blocks to access test files also ?
 
some comments :


File : 
++ java/testing/org/apache/derbyTesting/junit/BaseTestCase.java (working copy)


1) 
+                                       assertEquals(f1, f2);
+                               } catch (FileNotFoundException e) {
+                                       fail("FileNotFoundException in 
assertEquals(File,File): " + e.toString());
+                               } catch (IOException e) {
+                                       fail("IOException in assertEquals(File, 
File): " + e.toString());
+                               }

It might be better to throw the exception up  or a print stack trace also.    
If it ever fails on some platform , stacks will be helpful. 

2) 
 +                              
+                               return new Boolean(true);

Any reson for returning   a Boolean object  from the run() method  ?   I 
thought you are actaully passing it up , 
 but looks like  it just getting ignored. May you should just return null. 


+++ 
java/testing/org/apache/derbyTesting/functionTests/tests/tools/ImportExportTest.java
        (revision 0)

1) 
+   Derby - Class org.apache.derbyTesting.functionTests.tests.tools.importExport

--  Name is copyright notices is correct. 


2) 
+               ps.setString(3, (fromTable==null ?  fromTable : "extinout/" + 
fromTable + ".dat" ));

support files exttinout /extin are hard coded in this test.   May be you want 
to use the  methods defined in SupportFilesSetup class.
to access/create support files.  p SupportFilesSetup : ublic static File 
getReadOnly(String name) ..etc.

3)   This test  is easy to understand.  you may want to add still some comments 
to the test , especially  the data verification case. 
 

Thanks
-suresh







> convert importExport.java to junit
> ----------------------------------
>
>                 Key: DERBY-2342
>                 URL: https://issues.apache.org/jira/browse/DERBY-2342
>             Project: Derby
>          Issue Type: Improvement
>          Components: Tools
>    Affects Versions: 10.3.0.0
>            Reporter: Andrew McIntyre
>         Assigned To: Andrew McIntyre
>             Fix For: 10.3.0.0
>
>         Attachments: derby-2342-v1.diff, derby-2342-v1.stat, 
> derby-2342-v2.diff
>
>
> Convert org.apache.derbyTesting.functionTests.tests.tools.importExport to 
> junit. New test is called ImportExportTest.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to