add fixturing for notifying us when a previously failing unit test passes (better than @Ignore)

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

2 files changed, 63 insertions, 9 deletions

diff --git a/src/testcases/org/apache/poi/POITestCase.java b/src/testcases/org/apache/poi/POITestCase.java
index 786c6e1124..a8cc8e1db6 100644
--- a/src/testcases/org/apache/poi/POITestCase.java
+++ b/src/testcases/org/apache/poi/POITestCase.java
@@ -23,6 +23,7 @@ import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
+import static org.junit.Assume.assumeTrue;
 
 import java.lang.reflect.AccessibleObject;
 import java.lang.reflect.Field;
@@ -174,4 +175,62 @@ public final class POITestCase {
             }
         }
     }
+    
+    /**
+     * Rather than adding {@literal @}Ignore to known-failing tests,
+     * write the test so that it notifies us if it starts passing.
+     * This is useful for closing related or forgotten bugs.
+     * 
+     * An Example:
+     * <code><pre>
+     * public static int add(int a, int b) {
+     *     // a known bug in behavior that has not been fixed yet
+     *     raise UnsupportedOperationException("add");
+     * }
+     * 
+     * {@literal @}Test
+     * public void knownFailingUnitTest() {
+     *     try {
+     *         assertEquals(2, add(1,1));
+     *         // this test fails because the assumption that this bug had not been fixed is false
+     *         testPassesNow(12345);
+     *     } catch (UnsupportedOperationException e) {
+     *         // test is skipped because the assumption that this bug had not been fixed is true
+     *         skipTest(e);
+     *     }
+     * }
+     * 
+     * Once passing, this unit test can be rewritten as:
+     * {@literal @}Test
+     * public void knownPassingUnitTest() {
+     *     assertEquals(2, add(1,1));
+     * }
+     * 
+     * If you have a better idea how to simplify test code while still notifying
+     * us when a previous known-failing test now passes, please improve these.
+     * As a bonus, a known-failing test that fails should not be counted as a
+     * passing test.
+     * 
+     * One possible alternative is to expect the known exception, but without
+     * a clear message that it is a good thing to no longer get the expected
+     * exception once the test passes.
+     * {@literal @}Test(expected=UnsupportedOperationException.class)
+     * public void knownFailingUnitTest() {
+     *     assertEquals(2, add(1,1));
+     * }
+     *
+     * @param e  the exception that was caught that will no longer
+     * be raised when the bug is fixed 
+     */
+    public static void skipTest(Throwable e) {
+        assumeTrue("This test currently fails with " + e, false);
+    }
+    /**
+     * @see #skipTest(Throwable)
+     *
+     * @param bug  the bug number corresponding to a known bug in bugzilla
+     */
+    public static void testPassesNow(int bug) {
+        fail("This test passes now. Please update the unit test and bug " + bug + ".");
+    }
 }
diff --git a/src/testcases/org/apache/poi/poifs/macros/TestVBAMacroReader.java b/src/testcases/org/apache/poi/poifs/macros/TestVBAMacroReader.java
index 63fff7f967..614b4effaf 100644
--- a/src/testcases/org/apache/poi/poifs/macros/TestVBAMacroReader.java
+++ b/src/testcases/org/apache/poi/poifs/macros/TestVBAMacroReader.java
@@ -18,16 +18,15 @@
 package org.apache.poi.poifs.macros;
 
 import static org.apache.poi.POITestCase.assertContains;
+import static org.apache.poi.POITestCase.skipTest;
+import static org.apache.poi.POITestCase.testPassesNow;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.fail;
-import static org.junit.Assume.assumeTrue;
 
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.InputStream;
-import java.io.StringWriter;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
@@ -263,17 +262,13 @@ public class TestVBAMacroReader {
         reader.close();
     }
     
-    private static void skipTest(Throwable e) {
-        assumeTrue("This test currently fails with " + e, false);
-    }
-    
     // This test is written as expected-to-fail and should be rewritten
     // as expected-to-pass when the bug is fixed.
     @Test
     public void bug59858() throws IOException {
         try {
             fromFile(POIDataSamples.getSpreadSheetInstance(), "59858.xls");
-            fail("This test passes now. Please update the unit test and bug 59858.");
+            testPassesNow(59858);
         } catch (IOException e) {
             if (e.getMessage().matches("Module offset for '.+' was never read.")) {
                 //e.printStackTrace();
@@ -292,7 +287,7 @@ public class TestVBAMacroReader {
     public void bug60158() throws IOException {
         try {
             fromFile(POIDataSamples.getDocumentInstance(), "60158.docm");
-            fail("This test passes now. Please update the unit test and bug 60158.");
+            testPassesNow(60158);
         } catch (ArrayIndexOutOfBoundsException e) {
             skipTest(e);
         }