Sonar fixes

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

6 files changed, 43 insertions, 92 deletions

diff --git a/src/testcases/org/apache/poi/POITestCase.java b/src/testcases/org/apache/poi/POITestCase.java
index 8dfd5e0ce0..8848a37695 100644
--- a/src/testcases/org/apache/poi/POITestCase.java
+++ b/src/testcases/org/apache/poi/POITestCase.java
@@ -26,10 +26,9 @@ import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
-import static org.junit.Assume.assumeTrue;
+import static org.junit.Assume.assumeNoException;
 
 import java.lang.reflect.Field;
-import java.lang.reflect.Method;
 import java.security.AccessController;
 import java.security.PrivilegedActionException;
 import java.security.PrivilegedExceptionAction;
@@ -38,7 +37,6 @@ import java.util.Map;
 import java.util.Set;
 
 import org.apache.poi.util.Internal;
-import org.apache.poi.util.SuppressForbidden;
 import org.mockito.internal.matchers.apachecommons.ReflectionEquals;
 
 /**
@@ -47,6 +45,9 @@ import org.mockito.internal.matchers.apachecommons.ReflectionEquals;
 @Internal
 public final class POITestCase {
 
+    private POITestCase() {
+    }
+
     public static void assertStartsWith(String string, String prefix) {
         assertNotNull(string);
         assertNotNull(prefix);
@@ -118,18 +119,14 @@ public final class POITestCase {
      * Utility method to get the value of a private/protected field.
      * Only use this method in test cases!!!
      */
+    @SuppressWarnings({"unused", "unchecked"})
     public static <R,T> R getFieldValue(final Class<? super T> clazz, final T instance, final Class<R> fieldType, final String fieldName) {
         assertTrue("Reflection of private fields is only allowed for POI classes.", clazz.getName().startsWith("org.apache.poi."));
         try {
-            return AccessController.doPrivileged(new PrivilegedExceptionAction<R>() {
-                @Override
-                @SuppressWarnings("unchecked")
-                @SuppressForbidden("For test usage only")
-                public R run() throws Exception {
-                    Field f = clazz.getDeclaredField(fieldName);
-                    f.setAccessible(true);
-                    return (R) f.get(instance);
-                }
+            return AccessController.doPrivileged((PrivilegedExceptionAction<R>) () -> {
+                Field f = clazz.getDeclaredField(fieldName);
+                f.setAccessible(true);
+                return (R) f.get(instance);
             });
         } catch (PrivilegedActionException pae) {
             throw new RuntimeException("Cannot access field '" + fieldName + "' of class " + clazz, pae.getException());
@@ -137,33 +134,10 @@ public final class POITestCase {
     }
 
     /**
-     * Utility method to call a private/protected method.
-     * Only use this method in test cases!!!
-     */
-    public static <R,T> R callMethod(final Class<? super T> clazz, final T instance, final Class<R> returnType, final String methodName,
-        final Class<?>[] parameterTypes, final Object[] parameters) {
-        assertTrue("Reflection of private methods is only allowed for POI classes.", clazz.getName().startsWith("org.apache.poi."));
-        try {
-            return AccessController.doPrivileged(new PrivilegedExceptionAction<R>() {
-                @Override
-                @SuppressWarnings("unchecked")
-                @SuppressForbidden("For test usage only")
-                public R run() throws Exception {
-                    Method m = clazz.getDeclaredMethod(methodName, parameterTypes);
-                    m.setAccessible(true);
-                    return (R) m.invoke(instance, parameters);
-                }
-            });
-        } catch (PrivilegedActionException pae) {
-            throw new RuntimeException("Cannot access method '" + methodName + "' of class " + clazz, pae.getException());
-        }
-    }
-
-    /**
      * Utility method to shallow compare all fields of the objects
      * Only use this method in test cases!!!
      */
-    public static void assertReflectEquals(final Object expected, Object actual) throws Exception {
+    public static void assertReflectEquals(final Object expected, Object actual) {
         // as long as ReflectionEquals is provided by Mockito, use it ... otherwise use commons.lang for the tests
 
         // JaCoCo Code Coverage adds its own field, don't look at this one here
@@ -217,7 +191,7 @@ public final class POITestCase {
      * be raised when the bug is fixed
      */
     public static void skipTest(Throwable e) {
-        assumeTrue("This test currently fails with " + e, false);
+        assumeNoException("This test currently fails with", e);
     }
     /**
      * @see #skipTest(Throwable)
diff --git a/src/testcases/org/apache/poi/TestPOITestCase.java b/src/testcases/org/apache/poi/TestPOITestCase.java
index 42310738fa..60f5fb9cb7 100644
--- a/src/testcases/org/apache/poi/TestPOITestCase.java
+++ b/src/testcases/org/apache/poi/TestPOITestCase.java
@@ -18,20 +18,14 @@
 package org.apache.poi;
 
 import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
 
-import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.util.Collections;
 import java.util.Locale;
 import java.util.Map;
 
-import org.apache.poi.poifs.filesystem.DirectoryNode;
-import org.apache.poi.poifs.filesystem.DocumentEntry;
-import org.apache.poi.poifs.filesystem.EntryNode;
 import org.apache.poi.poifs.filesystem.POIFSFileSystem;
 import org.apache.poi.poifs.property.PropertyTable;
-import org.junit.Ignore;
 import org.junit.Test;
 
 /**
@@ -55,37 +49,27 @@ public final class TestPOITestCase {
     @Test
     public void assertContains() {
         POITestCase.assertContains("There is a needle in this haystack", "needle");
-        /*try {
-            POITestCase.assertContains("There is gold in this haystack", "needle");
-            fail("found a needle");
-        } catch (final junit.framework.AssertionFailedError e) {
-            // expected
-        }*/
     }
 
     @Test
     public void assertContainsIgnoreCase_Locale() {
         POITestCase.assertContainsIgnoreCase("There is a Needle in this haystack", "needlE", Locale.ROOT);
-        // FIXME: test failing case
     }
 
     @Test
     public void assertContainsIgnoreCase() {
         POITestCase.assertContainsIgnoreCase("There is a Needle in this haystack", "needlE");
-        // FIXME: test failing case
     }
 
     @Test
     public void assertNotContained() {
         POITestCase.assertNotContained("There is a needle in this haystack", "gold");
-        // FIXME: test failing case
     }
 
     @Test
     public void assertMapContains() {
         Map<String, String> haystack = Collections.singletonMap("needle", "value");
         POITestCase.assertContains(haystack, "needle");
-        // FIXME: test failing case
     }
 
 
@@ -100,33 +84,4 @@ public final class TestPOITestCase {
             assertNotNull(actual);
         }
     }
-
-    /**
-     * Utility method to call a private/protected method.
-     * Only use this method in test cases!!!
-     */
-    @Ignore
-    @Test
-    public void callMethod() throws IOException {
-        try (POIFSFileSystem fs = new POIFSFileSystem()) {
-            DirectoryNode root = fs.getRoot();
-            DocumentEntry doc = fs.createDocument(new ByteArrayInputStream(new byte[]{1, 2, 3}), "foobaa");
-            boolean actual = POITestCase.callMethod(DirectoryNode.class, root, boolean.class, "deleteEntry", new Class[]{EntryNode.class}, new Object[]{doc});
-            assertTrue(actual);
-        }
-    }
-
-    /**
-     * Utility method to shallow compare all fields of the objects
-     * Only use this method in test cases!!!
-     */
-    @Ignore
-    @Test
-    public void assertReflectEquals() throws Exception {
-        /*
-        final Object expected;
-        final Object actual;
-        POITestCase.assertReflectEquals(expected, actual);
-        */
-    }
 }
diff --git a/src/testcases/org/apache/poi/hssf/dev/TestBiffViewer.java b/src/testcases/org/apache/poi/hssf/dev/TestBiffViewer.java
index 32fa81383e..a62431bc55 100644
--- a/src/testcases/org/apache/poi/hssf/dev/TestBiffViewer.java
+++ b/src/testcases/org/apache/poi/hssf/dev/TestBiffViewer.java
@@ -37,10 +37,10 @@ public class TestBiffViewer extends BaseXLSIteratingTest {
     @BeforeClass
     public static void setup() {
         EXCLUDED.clear();
-        EXCLUDED.put("35897-type4.xls", EncryptedDocumentException.class); // unsupported crypto api header 
+        EXCLUDED.put("35897-type4.xls", EncryptedDocumentException.class); // unsupported crypto api header
         EXCLUDED.put("51832.xls", EncryptedDocumentException.class);
-        EXCLUDED.put("xor-encryption-abc.xls", EncryptedDocumentException.class); 
-        EXCLUDED.put("password.xls", EncryptedDocumentException.class); 
+        EXCLUDED.put("xor-encryption-abc.xls", EncryptedDocumentException.class);
+        EXCLUDED.put("password.xls", EncryptedDocumentException.class);
         EXCLUDED.put("46904.xls", OldExcelFormatException.class);
         EXCLUDED.put("59074.xls", OldExcelFormatException.class);
         EXCLUDED.put("testEXCEL_2.xls", OldExcelFormatException.class);  // Biff 2 / Excel 2, pre-OLE2
@@ -68,6 +68,7 @@ public class TestBiffViewer extends BaseXLSIteratingTest {
 
     @Test
     @Ignore("only used for manual tests")
+    @SuppressWarnings("java:S2699")
     public void testOneFile() throws Exception {
         POIDataSamples samples = POIDataSamples.getSpreadSheetInstance();
         runOneFile(samples.getFile("43493.xls"));
diff --git a/src/testcases/org/apache/poi/hssf/eventusermodel/TestHSSFEventFactory.java b/src/testcases/org/apache/poi/hssf/eventusermodel/TestHSSFEventFactory.java
index f3d4569963..cb1015f729 100644
--- a/src/testcases/org/apache/poi/hssf/eventusermodel/TestHSSFEventFactory.java
+++ b/src/testcases/org/apache/poi/hssf/eventusermodel/TestHSSFEventFactory.java
@@ -36,7 +36,6 @@ import org.apache.poi.hssf.record.DVRecord;
 import org.apache.poi.hssf.record.EOFRecord;
 import org.apache.poi.hssf.record.FeatHdrRecord;
 import org.apache.poi.hssf.record.NumberRecord;
-import org.apache.poi.hssf.record.Record;
 import org.apache.poi.hssf.record.SelectionRecord;
 import org.apache.poi.hssf.record.WindowTwoRecord;
 import org.apache.poi.hssf.record.crypto.Biff8EncryptionKey;
@@ -104,17 +103,20 @@ public final class TestHSSFEventFactory {
      * (the test file was provided in a reopen of bug #42844)
      */
     @Test
+    @SuppressWarnings("java:S2699")
     public void testUnknownContinueRecords() throws Exception {
         openSample("42844.xls");
     }
 
     @Test
+    @SuppressWarnings("java:S2699")
     public void testWithDifferentWorkbookName() throws Exception {
         openSample("BOOK_in_capitals.xls");
         openSample("WORKBOOK_in_capitals.xls");
     }
 
     @Test(expected = EncryptedDocumentException.class)
+    @SuppressWarnings("java:S2699")
     public void testWithPasswordProtectedWorkbooksNoPass() throws Exception {
         // Without a password, can't be read
         openSample("xor-encryption-abc.xls");
diff --git a/src/testcases/org/apache/poi/hssf/record/TestFeatRecord.java b/src/testcases/org/apache/poi/hssf/record/TestFeatRecord.java
index b2e75a642b..fe56976545 100644
--- a/src/testcases/org/apache/poi/hssf/record/TestFeatRecord.java
+++ b/src/testcases/org/apache/poi/hssf/record/TestFeatRecord.java
@@ -23,6 +23,8 @@ import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
+import java.io.IOException;
+
 import org.apache.poi.hssf.HSSFTestDataSamples;
 import org.apache.poi.hssf.model.InternalSheet;
 import org.apache.poi.hssf.model.InternalWorkbook;
@@ -181,9 +183,16 @@ public final class TestFeatRecord {
      *  cloning sheets with feat records
      */
 	@Test
-    public void testCloneSheetWithFeatRecord() {
-        HSSFWorkbook wb =
-            HSSFTestDataSamples.openSampleWorkbook("46136-WithWarnings.xls");
-        wb.cloneSheet(0);
+    public void testCloneSheetWithFeatRecord() throws IOException {
+        try (HSSFWorkbook wb =
+            HSSFTestDataSamples.openSampleWorkbook("46136-WithWarnings.xls")) {
+			HSSFSheet src = wb.getSheetAt(0);
+			HSSFSheet dst = wb.cloneSheet(0);
+
+			InternalSheet isrc = HSSFTestHelper.getSheetForTest(src);
+			InternalSheet idst = HSSFTestHelper.getSheetForTest(dst);
+
+			assertEquals(isrc.getRecords().size(), idst.getRecords().size());
+		}
     }
 }
diff --git a/src/testcases/org/apache/poi/hssf/record/TestLabelRecord.java b/src/testcases/org/apache/poi/hssf/record/TestLabelRecord.java
index f244a71b26..ec5ed36689 100644
--- a/src/testcases/org/apache/poi/hssf/record/TestLabelRecord.java
+++ b/src/testcases/org/apache/poi/hssf/record/TestLabelRecord.java
@@ -17,9 +17,12 @@
 
 package org.apache.poi.hssf.record;
 
+import static org.junit.Assert.assertEquals;
+
 import java.io.IOException;
 
 import org.apache.poi.hssf.HSSFTestDataSamples;
+import org.apache.poi.hssf.usermodel.HSSFSheet;
 import org.apache.poi.hssf.usermodel.HSSFWorkbook;
 import org.junit.Test;
 
@@ -30,8 +33,15 @@ public final class TestLabelRecord  {
 
 	@Test
 	public void testEmptyString() throws IOException {
-		try (HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("ex42570-20305.xls")) {
-			HSSFTestDataSamples.writeOutAndReadBack(wb);
+		try (HSSFWorkbook wb1 = HSSFTestDataSamples.openSampleWorkbook("ex42570-20305.xls");
+			 HSSFWorkbook wb2 = HSSFTestDataSamples.writeOutAndReadBack(wb1)) {
+			HSSFSheet s1 = wb1.getSheetAt(0);
+			HSSFSheet s2 = wb2.getSheetAt(0);
+			for (int c=0; c<2; c++) {
+				for (int r=0; r<146; r++) {
+					assertEquals(s1.getRow(r).getCell(c).getNumericCellValue(), s2.getRow(r).getCell(c).getNumericCellValue(), 0);
+				}
+			}
 		}
 	}
 }