convert importExport.java to junit

• Key: DERBY-2342
• Type: Improvement
• Status: Closed
• Resolution: Fixed
• Priority: Major
• Assignee: Andrew McIntyre
• Reporter: Andrew McIntyre
• Component/s: Tools
• Affects Version/s: 10.3.1.4
• Fix Version/s: 10.3.1.4

Description

Convert org.apache.derbyTesting.functionTests.tests.tools.importExport to junit. New test is called ImportExportTest.

Comments

[Andrew McIntyre]

Patch that converts importExport.java to junit.

[Andrew McIntyre]

stat file for v1 patch

[Andrew McIntyre]

Attaching a second version of this patch which changes assertFilesEqual(String, String) to assertEquals(File,File) and moves it to BaseTestCase and reuses assertEquals for InputStreams instead of implementing something similar again.

[Suresh Thalamati]

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

[Andrew McIntyre]

Thanks very much for the review, Suresh! Attaching a patch which incorporates most of your suggestions.

I've used SupportFilesSetup.getReadWrite() to get the Files to pass to assertEquals(File,File). For the other cases where the directory is hardcoded, we need to pass a String to the import or export procedure. I could get a File or URL with getReadWrite or getReadWriteURL, but then I'd either have to get the String representation of the File or URL and do some manipulation of that String to get it into the format that the import/export procedures are expecting. I'm not sure that it is worth the trouble to do that when the location of the support file directories is unlikely to change.

Patch is still not quite ready to commit, though, as I'm seeing some failures on jdk16 in client mode.

[Andrew McIntyre]

Attaching an updated version of this patch which passes on jdk16 as well.

[Andrew McIntyre]

Attaching an updated patch for this issue which passes on jdk16 as well.

[Andrew McIntyre]

Attaching another updated patch, this catches the test up to the changes made by Suresh for DERBY-378.

[Andrew McIntyre]

Committed -v5 patch with revision 514040.