Refactor BaseXLSIteratingTest into a Parameterized test to better show which files failed

Simplify exclusion handling
Exclude testEXCEL_3.xls and testEXCEL_4.xls in two tests, not sure why this worked before?!

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1697601 13f79535-47bb-0310-9956-ffa450edef68

7 files changed, 112 insertions, 128 deletions

diff --git a/src/testcases/org/apache/poi/hssf/dev/BaseXLSIteratingTest.java b/src/testcases/org/apache/poi/hssf/dev/BaseXLSIteratingTest.java
index 0adb2cf984..ca15d74ddc 100644
--- a/src/testcases/org/apache/poi/hssf/dev/BaseXLSIteratingTest.java
+++ b/src/testcases/org/apache/poi/hssf/dev/BaseXLSIteratingTest.java
@@ -17,7 +17,6 @@
 package org.apache.poi.hssf.dev;
 
 import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
 
 import java.io.File;
 import java.io.FileInputStream;
@@ -29,79 +28,79 @@ import java.util.List;
 
 import org.apache.poi.POIDataSamples;
 import org.apache.poi.hssf.usermodel.HSSFWorkbook;
+import org.junit.Assume;
 import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
 
 /**
  * Base class for integration-style tests which iterate over all test-files
  * and execute the same action to find out if any change breaks these applications.
+ * 
+ * This test uses {@link Parameterized} to run the test for each file separatedely.
  */
+@RunWith(Parameterized.class)
 public abstract class BaseXLSIteratingTest {
     protected static final OutputStream NULL_OUTPUT_STREAM = new NullOutputStream();
 
 	protected static final List<String> EXCLUDED = new ArrayList<String>();
-	protected static final List<String> SILENT_EXCLUDED = new ArrayList<String>();
 
+    @Parameters(name="{index}: {0} using {1}")
+    public static Iterable<Object[]> files() {
+        String dataDirName = System.getProperty(POIDataSamples.TEST_PROPERTY);
+        if(dataDirName == null) {
+            dataDirName = "test-data";
+        }
+
+        List<Object[]> files = new ArrayList<Object[]>();
+        findFile(files, dataDirName + "/spreadsheet");
+        findFile(files, dataDirName + "/hpsf");
+        
+        return files;
+    }
+	
+    private static void findFile(List<Object[]> list, String dir) {
+        String[] files = new File(dir).list(new FilenameFilter() {
+            @Override
+            public boolean accept(File arg0, String arg1) {
+                return arg1.toLowerCase().endsWith(".xls");
+            }
+        });
+        
+        assertNotNull("Did not find any xls files in directory " + dir, files);
+        
+        for(String file : files) {
+            list.add(new Object[] { new File(dir, file) });
+        }
+    }
+    
+    @Parameter
+    public File file;
+    
 	@Test
 	public void testMain() throws Exception {
-	    String dataDirName = System.getProperty(POIDataSamples.TEST_PROPERTY);
-	    if(dataDirName == null) {
-	        dataDirName = "test-data";
-	    }
-
-	    int count = runWithDir(dataDirName + "/spreadsheet");
-		count += runWithDir(dataDirName + "/hpsf");
-		
-		System.out.println("Had " + count + " files");
-	}
-
-	private int runWithDir(String dir) throws IOException {
-		List<String> failed = new ArrayList<String>();
-
-		String[] files = new File(dir).list(new FilenameFilter() {
-			@Override
-            public boolean accept(File arg0, String arg1) {
-				return arg1.toLowerCase().endsWith(".xls");
-			}
-		});
-		
-		assertNotNull("Did not find any xls files in directory " + dir, files);
-		
-		runWithArrayOfFiles(files, dir, failed);
-
-		assertTrue("Expected to have no failed except the ones excluded, but had: " + failed, 
-				failed.isEmpty());
-		
-		return files.length;
-	}
-
-	private void runWithArrayOfFiles(String[] files, String dir, List<String> failed) throws IOException {
-		for(String file : files) {
+		try {
+			runOneFile(file);
+		} catch (Exception e) {
+		    Assume.assumeFalse("File " + file + " is excluded currently",
+		            EXCLUDED.contains(file.getName()));
+
+			System.out.println("Failed: " + file);
+			e.printStackTrace();
+			
+			// try to read it in HSSFWorkbook to quickly fail if we cannot read the file there at all and thus probably should use EXCLUDED instead
+			FileInputStream stream = new FileInputStream(file);
 			try {
-				runOneFile(dir, file, failed);
-			} catch (Exception e) {
-				if(SILENT_EXCLUDED.contains(file)) {
-					continue;
-				}
-
-				System.out.println("Failed: " + file);
-				e.printStackTrace();
-				
-				// try to read it in HSSFWorkbook to quickly fail if we cannot read the file there at all and thus probably can use SILENT_EXCLUDED instead
-				FileInputStream stream = new FileInputStream(new File(dir, file));
-				try {
-					assertNotNull(new HSSFWorkbook(stream));
-				} finally {
-					stream.close();
-				}
-				
-				if(!EXCLUDED.contains(file)) {
-					failed.add(file);
-				}
+				assertNotNull(new HSSFWorkbook(stream));
+			} finally {
+				stream.close();
 			}
 		}
 	}
 
-	abstract void runOneFile(String dir, String file, List<String> failed) throws Exception;
+	abstract void runOneFile(File file) throws Exception;
 
 	/**
 	 * Implementation of an OutputStream which does nothing, used
diff --git a/src/testcases/org/apache/poi/hssf/dev/TestBiffDrawingToXml.java b/src/testcases/org/apache/poi/hssf/dev/TestBiffDrawingToXml.java
index e141608959..2a14b906ca 100644
--- a/src/testcases/org/apache/poi/hssf/dev/TestBiffDrawingToXml.java
+++ b/src/testcases/org/apache/poi/hssf/dev/TestBiffDrawingToXml.java
@@ -20,7 +20,6 @@ import java.io.File;
 import java.io.FileInputStream;
 import java.io.InputStream;
 import java.io.PrintStream;
-import java.util.List;
 
 import org.junit.Ignore;
 import org.junit.Test;
@@ -42,13 +41,13 @@ public class TestBiffDrawingToXml extends BaseXLSIteratingTest {
 	}
 	
 	@Override
-	void runOneFile(String dir, String file, List<String> failed)
+	void runOneFile(File file)
 			throws Exception {
 		PrintStream save = System.out;
 		try {
 			//System.setOut(new PrintStream(TestBiffViewer.NULL_OUTPUT_STREAM));
 			// use a NullOutputStream to not write the bytes anywhere for best runtime 
-		    InputStream wb = new FileInputStream(new File(dir, file));
+		    InputStream wb = new FileInputStream(file);
 		    try {
 		    	BiffDrawingToXml.writeToFile(NULL_OUTPUT_STREAM, wb, false, new String[] {});
 		    } finally {
diff --git a/src/testcases/org/apache/poi/hssf/dev/TestBiffViewer.java b/src/testcases/org/apache/poi/hssf/dev/TestBiffViewer.java
index 5d27990dab..a1d190ab58 100644
--- a/src/testcases/org/apache/poi/hssf/dev/TestBiffViewer.java
+++ b/src/testcases/org/apache/poi/hssf/dev/TestBiffViewer.java
@@ -20,34 +20,30 @@ import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.PrintStream;
-import java.util.List;
 
 public class TestBiffViewer extends BaseXLSIteratingTest {
 	static {
-		// Look at the output of the test for the detailed stacktrace of the failures...
-		//EXCLUDED.add("");
-		
 		// these are likely ok to fail
-		SILENT_EXCLUDED.add("XRefCalc.xls"); 	// "Buffer overrun"
-		SILENT_EXCLUDED.add("50833.xls"); 		// "Name is too long" when setting username
-		SILENT_EXCLUDED.add("OddStyleRecord.xls");		
-		SILENT_EXCLUDED.add("NoGutsRecords.xls"); 
-		SILENT_EXCLUDED.add("51832.xls");	// password 
-		SILENT_EXCLUDED.add("43493.xls");	// HSSFWorkbook cannot open it as well
-		SILENT_EXCLUDED.add("password.xls"); 
-		SILENT_EXCLUDED.add("46904.xls");
-        SILENT_EXCLUDED.add("35897-type4.xls"); // unsupported crypto api header 
-		SILENT_EXCLUDED.add("xor-encryption-abc.xls"); // unsupported XOR-encryption
-        SILENT_EXCLUDED.add("testEXCEL_2.xls");  // Biff 2 / Excel 2, pre-OLE2
-        SILENT_EXCLUDED.add("testEXCEL_3.xls");  // Biff 3 / Excel 3, pre-OLE2
-        SILENT_EXCLUDED.add("testEXCEL_4.xls");  // Biff 4 / Excel 4, pre-OLE2
-        SILENT_EXCLUDED.add("testEXCEL_5.xls");  // Biff 5 / Excel 5
-        SILENT_EXCLUDED.add("testEXCEL_95.xls"); // Biff 5 / Excel 95
+		EXCLUDED.add("XRefCalc.xls"); 	// "Buffer overrun"
+		EXCLUDED.add("50833.xls"); 		// "Name is too long" when setting username
+		EXCLUDED.add("OddStyleRecord.xls");		
+		EXCLUDED.add("NoGutsRecords.xls"); 
+		EXCLUDED.add("51832.xls");	// password 
+		EXCLUDED.add("43493.xls");	// HSSFWorkbook cannot open it as well
+		EXCLUDED.add("password.xls"); 
+		EXCLUDED.add("46904.xls");
+        EXCLUDED.add("35897-type4.xls"); // unsupported crypto api header 
+		EXCLUDED.add("xor-encryption-abc.xls"); // unsupported XOR-encryption
+        EXCLUDED.add("testEXCEL_2.xls");  // Biff 2 / Excel 2, pre-OLE2
+        EXCLUDED.add("testEXCEL_3.xls");  // Biff 3 / Excel 3, pre-OLE2
+        EXCLUDED.add("testEXCEL_4.xls");  // Biff 4 / Excel 4, pre-OLE2
+        EXCLUDED.add("testEXCEL_5.xls");  // Biff 5 / Excel 5
+        EXCLUDED.add("testEXCEL_95.xls"); // Biff 5 / Excel 95
 	}
 
 	@Override
-	void runOneFile(String dir, String file, List<String> failed) throws IOException {
-		InputStream is = BiffViewer.getPOIFSInputStream(new File(dir, file));
+	void runOneFile(File file) throws IOException {
+		InputStream is = BiffViewer.getPOIFSInputStream(file);
 		try {
 			// use a NullOutputStream to not write the bytes anywhere for best runtime 
 			BiffViewer.runBiffViewer(new PrintStream(NULL_OUTPUT_STREAM), is, true, true, true, false);
diff --git a/src/testcases/org/apache/poi/hssf/dev/TestEFBiffViewer.java b/src/testcases/org/apache/poi/hssf/dev/TestEFBiffViewer.java
index c3730bc043..32eaa924f4 100644
--- a/src/testcases/org/apache/poi/hssf/dev/TestEFBiffViewer.java
+++ b/src/testcases/org/apache/poi/hssf/dev/TestEFBiffViewer.java
@@ -19,36 +19,33 @@ package org.apache.poi.hssf.dev;
 import java.io.File;
 import java.io.IOException;
 import java.io.PrintStream;
-import java.util.List;
 
 public class TestEFBiffViewer extends BaseXLSIteratingTest {
 	static {
-		// Look at the output of the test for the detailed stacktrace of the failures...
-		//EXCLUDED.add("");
-
 		// these are likely ok to fail
-		SILENT_EXCLUDED.add("XRefCalc.xls"); 
-		SILENT_EXCLUDED.add("password.xls"); 
-		SILENT_EXCLUDED.add("51832.xls"); 		// password
-		SILENT_EXCLUDED.add("xor-encryption-abc.xls");    // password, ty again later!
-		SILENT_EXCLUDED.add("43493.xls");	// HSSFWorkbook cannot open it as well
-		SILENT_EXCLUDED.add("44958_1.xls");   // known bad file
-		SILENT_EXCLUDED.add("46904.xls");     // Exception, too old
-		SILENT_EXCLUDED.add("47251_1.xls");   // Broken test file
-		SILENT_EXCLUDED.add("testEXCEL_4.xls");   // old unsupported format
-		SILENT_EXCLUDED.add("testEXCEL_5.xls");   // old unsupported format
-		SILENT_EXCLUDED.add("testEXCEL_95.xls");   // old unsupported format
-		SILENT_EXCLUDED.add("35897-type4.xls");   // unsupported encryption
+		EXCLUDED.add("XRefCalc.xls"); 
+		EXCLUDED.add("password.xls"); 
+		EXCLUDED.add("51832.xls"); 		// password
+		EXCLUDED.add("xor-encryption-abc.xls");    // password, ty again later!
+		EXCLUDED.add("43493.xls");	// HSSFWorkbook cannot open it as well
+		EXCLUDED.add("44958_1.xls");   // known bad file
+		EXCLUDED.add("46904.xls");     // Exception, too old
+		EXCLUDED.add("47251_1.xls");   // Broken test file
+        EXCLUDED.add("testEXCEL_3.xls");  // Biff 3 / Excel 3, pre-OLE2
+		EXCLUDED.add("testEXCEL_4.xls");   // old unsupported format
+		EXCLUDED.add("testEXCEL_5.xls");   // old unsupported format
+		EXCLUDED.add("testEXCEL_95.xls");   // old unsupported format
+		EXCLUDED.add("35897-type4.xls");   // unsupported encryption
 	}
 	
 	@Override
-	void runOneFile(String dir, String file, List<String> failed) throws IOException {
+	void runOneFile(File file) throws IOException {
 		PrintStream save = System.out;
 		try {
 			// redirect standard out during the test to avoid spamming the console with output
 			System.setOut(new PrintStream(NULL_OUTPUT_STREAM));
 
-			EFBiffViewer.main(new String[] { new File(dir, file).getAbsolutePath() });
+			EFBiffViewer.main(new String[] { file.getAbsolutePath() });
 		} finally {
 			System.setOut(save);
 		}
diff --git a/src/testcases/org/apache/poi/hssf/dev/TestFormulaViewer.java b/src/testcases/org/apache/poi/hssf/dev/TestFormulaViewer.java
index 8683b78dcf..b6014376c5 100644
--- a/src/testcases/org/apache/poi/hssf/dev/TestFormulaViewer.java
+++ b/src/testcases/org/apache/poi/hssf/dev/TestFormulaViewer.java
@@ -18,7 +18,6 @@ package org.apache.poi.hssf.dev;
 
 import java.io.File;
 import java.io.PrintStream;
-import java.util.List;
 
 import org.junit.Ignore;
 import org.junit.Test;
@@ -41,14 +40,14 @@ public class TestFormulaViewer extends BaseXLSIteratingTest {
 	}
 
 	@Override
-	void runOneFile(String dir, String file, List<String> failed) throws Exception {
+	void runOneFile(File file) throws Exception {
 		PrintStream save = System.out;
 		try {
 			// redirect standard out during the test to avoid spamming the console with output
 			System.setOut(new PrintStream(NULL_OUTPUT_STREAM));
 
             FormulaViewer viewer = new FormulaViewer();
-            viewer.setFile(new File(dir, file).getAbsolutePath());
+            viewer.setFile(file.getAbsolutePath());
             viewer.setList(true);
             viewer.run();
 		} finally {
diff --git a/src/testcases/org/apache/poi/hssf/dev/TestReSave.java b/src/testcases/org/apache/poi/hssf/dev/TestReSave.java
index d42af8858f..e26bffea3a 100644
--- a/src/testcases/org/apache/poi/hssf/dev/TestReSave.java
+++ b/src/testcases/org/apache/poi/hssf/dev/TestReSave.java
@@ -24,26 +24,21 @@ import java.util.ArrayList;
 import java.util.List;
 
 import org.apache.poi.POIDataSamples;
-import org.junit.Test;
 
 public class TestReSave extends BaseXLSIteratingTest {
 	static {
-		// TODO: is it ok to fail these? 
-		// Look at the output of the test for the detailed stacktrace of the failures...
-		EXCLUDED.add("49931.xls");
-
 		// these are likely ok to fail
-		SILENT_EXCLUDED.add("password.xls"); 
-		SILENT_EXCLUDED.add("43493.xls");	// HSSFWorkbook cannot open it as well
-		SILENT_EXCLUDED.add("46904.xls"); 
-		SILENT_EXCLUDED.add("51832.xls");	// password 
-        SILENT_EXCLUDED.add("44958_1.xls");   // known bad file
+		EXCLUDED.add("password.xls"); 
+		EXCLUDED.add("43493.xls");	// HSSFWorkbook cannot open it as well
+		EXCLUDED.add("46904.xls"); 
+		EXCLUDED.add("51832.xls");	// password 
+        EXCLUDED.add("44958_1.xls");   // known bad file
 	}
 	
 	@Override
-	void runOneFile(String dir, String file, List<String> failed) throws Exception {
+	void runOneFile(File file) throws Exception {
 		// avoid running on files leftover from previous failed runs
-		if(file.endsWith("-saved.xls")) {
+		if(file.getName().endsWith("-saved.xls")) {
 			return;
 		}
 
@@ -52,22 +47,23 @@ public class TestReSave extends BaseXLSIteratingTest {
 			// redirect standard out during the test to avoid spamming the console with output
 			System.setOut(new PrintStream(NULL_OUTPUT_STREAM));
 
+			File reSavedFile = new File(file.getParentFile(), file.getName().replace(".xls", "-saved.xls"));
 			try {
-				ReSave.main(new String[] { new File(dir, file).getAbsolutePath() });
+				ReSave.main(new String[] { file.getAbsolutePath() });
 				
 				// also try BiffViewer on the saved file
-				new TestBiffViewer().runOneFile(dir, file.replace(".xls", "-saved.xls"), failed);
+                new TestBiffViewer().runOneFile(reSavedFile);
 
     			try {
         			// had one case where the re-saved could not be re-saved!
-        			ReSave.main(new String[] { new File(dir, file.replace(".xls", "-saved.xls")).getAbsolutePath() });
+        			ReSave.main(new String[] { reSavedFile.getAbsolutePath() });
     			} finally {
     				// clean up the re-re-saved file
-    				new File(dir, file.replace(".xls", "-saved.xls").replace(".xls", "-saved.xls")).delete();
+    			    new File(file.getParentFile(), reSavedFile.getName().replace(".xls", "-saved.xls")).delete();
     			}
 			} finally {
 				// clean up the re-saved file
-				new File(dir, file.replace(".xls", "-saved.xls")).delete();
+				reSavedFile.delete();
 			}
 
 		} finally {
@@ -75,7 +71,8 @@ public class TestReSave extends BaseXLSIteratingTest {
 		}
 	}
 
-	@Test
+	//Only used for local testing
+	//@Test
 	public void testOneFile() throws Exception {
         String dataDirName = System.getProperty(POIDataSamples.TEST_PROPERTY);
         if(dataDirName == null) {
@@ -83,7 +80,7 @@ public class TestReSave extends BaseXLSIteratingTest {
         }
 
 		List<String> failed = new ArrayList<String>();
-		runOneFile(dataDirName + "/spreadsheet", "49219.xls", failed);
+		runOneFile(new File(dataDirName + "/spreadsheet", "49931.xls"));
 
 		assertTrue("Expected to have no failed except the ones excluded, but had: " + failed, 
 				failed.isEmpty());
diff --git a/src/testcases/org/apache/poi/hssf/dev/TestRecordLister.java b/src/testcases/org/apache/poi/hssf/dev/TestRecordLister.java
index c74816fd1e..6c40b8b5ce 100644
--- a/src/testcases/org/apache/poi/hssf/dev/TestRecordLister.java
+++ b/src/testcases/org/apache/poi/hssf/dev/TestRecordLister.java
@@ -19,27 +19,24 @@ package org.apache.poi.hssf.dev;
 import java.io.File;
 import java.io.IOException;
 import java.io.PrintStream;
-import java.util.List;
 
 public class TestRecordLister extends BaseXLSIteratingTest {
 	static {
-		// TODO: is it ok to fail these? 
-		// Look at the output of the test for the detailed stacktrace of the failures...
-		//EXCLUDED.add("");
-
 		// these are likely ok to fail
-		SILENT_EXCLUDED.add("46904.xls"); 
+		EXCLUDED.add("46904.xls"); 
+        EXCLUDED.add("testEXCEL_3.xls");  // Biff 3 / Excel 3, pre-OLE2
+        EXCLUDED.add("testEXCEL_4.xls");   // old unsupported format
 	}
 	
 	@Override
-	void runOneFile(String dir, String file, List<String> failed) throws IOException {
+	void runOneFile(File file) throws IOException {
 		PrintStream save = System.out;
 		try {
 			// redirect standard out during the test to avoid spamming the console with output
 			System.setOut(new PrintStream(NULL_OUTPUT_STREAM));
 
 			RecordLister viewer = new RecordLister();
-            viewer.setFile(new File(dir, file).getAbsolutePath());
+            viewer.setFile(file.getAbsolutePath());
             viewer.run();
 		} finally {
 			System.setOut(save);