Sonar fixes

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

7 files changed, 183 insertions, 245 deletions

diff --git a/src/excelant/testcases/org/apache/poi/ss/excelant/TestBuildFile.java b/src/excelant/testcases/org/apache/poi/ss/excelant/TestBuildFile.java
index af9e317a42..703b97be04 100644
--- a/src/excelant/testcases/org/apache/poi/ss/excelant/TestBuildFile.java
+++ b/src/excelant/testcases/org/apache/poi/ss/excelant/TestBuildFile.java
@@ -345,6 +345,8 @@ public class TestBuildFile {
     @Test
     public void testPassOnError() {
         executeTarget("test-passonerror");
+        assertLogContaining("Using input file: " + TestBuildFile.getDataDir() + "/spreadsheet/excelant.xls");
+        assertLogContaining("Test named failonerror failed because 1 of 0 evaluations failed to evaluate correctly.");
     }
 
     @Test
diff --git a/src/testcases/org/apache/poi/POITestCase.java b/src/testcases/org/apache/poi/POITestCase.java
index 7861030a1f..8dfd5e0ce0 100644
--- a/src/testcases/org/apache/poi/POITestCase.java
+++ b/src/testcases/org/apache/poi/POITestCase.java
@@ -17,37 +17,32 @@
 
 package org.apache.poi;
 
-import static org.junit.Assert.assertArrayEquals;
-import static org.junit.Assert.assertEquals;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.endsWith;
+import static org.hamcrest.CoreMatchers.hasItem;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.CoreMatchers.startsWith;
 import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
 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.hamcrest.CoreMatchers.containsString;
-import static org.hamcrest.CoreMatchers.startsWith;
-import static org.hamcrest.CoreMatchers.endsWith;
-import static org.hamcrest.CoreMatchers.not;
-import static org.hamcrest.CoreMatchers.hasItem;
-
-import java.lang.reflect.AccessibleObject;
 import java.lang.reflect.Field;
 import java.lang.reflect.Method;
 import java.security.AccessController;
 import java.security.PrivilegedActionException;
 import java.security.PrivilegedExceptionAction;
-import java.util.ArrayList;
-import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 import java.util.Set;
 
-import org.apache.poi.util.SuppressForbidden;
 import org.apache.poi.util.Internal;
+import org.apache.poi.util.SuppressForbidden;
+import org.mockito.internal.matchers.apachecommons.ReflectionEquals;
 
 /**
- * Util class for POI JUnit TestCases, which provide additional features 
+ * Util class for POI JUnit TestCases, which provide additional features
  */
 @Internal
 public final class POITestCase {
@@ -69,7 +64,7 @@ public final class POITestCase {
         assertNotNull(suffix);
         assertThat(string, endsWith(suffix));
     }
-    
+
     public static void assertContains(String haystack, String needle) {
         assertNotNull(haystack);
         assertNotNull(needle);
@@ -94,13 +89,13 @@ public final class POITestCase {
     public static void assertContainsIgnoreCase(String haystack, String needle) {
         assertContainsIgnoreCase(haystack, needle, Locale.ROOT);
     }
-    
+
     public static void assertNotContained(String haystack, String needle) {
         assertNotNull(haystack);
         assertNotNull(needle);
         assertThat(haystack, not(containsString(needle)));
     }
-    
+
     /**
      * @param map haystack
      * @param key needle
@@ -118,7 +113,7 @@ public final class POITestCase {
             fail("Set should not contain " + element);
         }*/
     }
-     
+
     /**
      * Utility method to get the value of a private/protected field.
      * Only use this method in test cases!!!
@@ -140,7 +135,7 @@ public final class POITestCase {
             throw new RuntimeException("Cannot access field '" + fieldName + "' of class " + clazz, pae.getException());
         }
     }
-     
+
     /**
      * Utility method to call a private/protected method.
      * Only use this method in test cases!!!
@@ -169,60 +164,24 @@ public final class POITestCase {
      * Only use this method in test cases!!!
      */
     public static void assertReflectEquals(final Object expected, Object actual) throws Exception {
-        final List<Field> fields;
-        try {
-            fields = AccessController.doPrivileged(new PrivilegedExceptionAction<List<Field>>() {
-                @Override
-                @SuppressForbidden("Test only")
-                public List<Field> run() throws Exception {
-                    List<Field> flds = new ArrayList<>();
-                    for (Class<?> c = expected.getClass(); c != null; c = c.getSuperclass()) {
-                        Field[] fs = c.getDeclaredFields();
-                        AccessibleObject.setAccessible(fs, true);                        
-                        for (Field f : fs) {
-                            // JaCoCo Code Coverage adds it's own field, don't look at this one here
-                            if(f.getName().equals("$jacocoData")) {
-                                continue;
-                            }
-                            
-                            flds.add(f);
-                        }
-                    }
-                    return flds;
-                }
-            });
-        } catch (PrivilegedActionException pae) {
-            throw pae.getException();
-        }
-        
-        for (Field f : fields) {
-            Class<?> t = f.getType();
-            if (t.isArray()) {
-                if (Object[].class.isAssignableFrom(t)) {
-                    assertArrayEquals((Object[])f.get(expected), (Object[])f.get(actual));
-                } else if (byte[].class.isAssignableFrom(t)) {
-                    assertArrayEquals((byte[])f.get(expected), (byte[])f.get(actual));
-                } else {
-                    fail("Array type is not yet implemented ... add it!");
-                }
-            } else {
-                assertEquals(f.get(expected), f.get(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
+        assertTrue(new ReflectionEquals(expected, "$jacocoData").matches(actual));
     }
-    
+
     /**
      * 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 {
@@ -234,18 +193,18 @@ public final class POITestCase {
      *         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.
@@ -255,7 +214,7 @@ public final class POITestCase {
      * }
      *
      * @param e  the exception that was caught that will no longer
-     * be raised when the bug is fixed 
+     * be raised when the bug is fixed
      */
     public static void skipTest(Throwable e) {
         assumeTrue("This test currently fails with " + e, false);
@@ -268,18 +227,11 @@ public final class POITestCase {
     public static void testPassesNow(int bug) {
         fail("This test passes now. Please update the unit test and bug " + bug + ".");
     }
-    
+
     public static void assertBetween(String message, int value, int min, int max) {
         assertTrue(message + ": " + value + " is less than the minimum value of " + min,
                 min <= value);
         assertTrue(message + ": " + value + " is greater than the maximum value of " + max,
                 value <= max);
     }
-    public static void assertStrictlyBetween(String message, int value, int min, int max) {
-        assertTrue(message + ": " + value + " is less than or equal to the minimum value of " + min,
-                min < value);
-        assertTrue(message + ": " + value + " is greater than or equal to the maximum value of " + max,
-                value < max);
-    }
-
 }
diff --git a/src/testcases/org/apache/poi/TestPOITestCase.java b/src/testcases/org/apache/poi/TestPOITestCase.java
index 796f51335f..42310738fa 100644
--- a/src/testcases/org/apache/poi/TestPOITestCase.java
+++ b/src/testcases/org/apache/poi/TestPOITestCase.java
@@ -17,10 +17,20 @@
 
 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;
 
@@ -41,7 +51,7 @@ public final class TestPOITestCase {
         POITestCase.assertEndsWith("Apache POI", "POI");
         POITestCase.assertEndsWith("Apache POI", "Apache POI");
     }
-    
+
     @Test
     public void assertContains() {
         POITestCase.assertContains("There is a needle in this haystack", "needle");
@@ -78,45 +88,32 @@ public final class TestPOITestCase {
         // FIXME: test failing case
     }
 
-     
+
     /**
      * Utility method to get the value of a private/protected field.
      * Only use this method in test cases!!!
      */
-    @Ignore
     @Test
-    public void getFieldValue() {
-        /*
-        final Class<? super T> clazz;
-        final T instance;
-        final Class<R> fieldType;
-        final String fieldName;
-
-        final R expected;
-        final R actual = POITestCase.getFieldValue(clazz, instance, fieldType, fieldName);
-        assertEquals(expected, actual);
-        */
+    public void getFieldValue() throws IOException {
+        try (POIFSFileSystem fs = new POIFSFileSystem()) {
+            PropertyTable actual = POITestCase.getFieldValue(POIFSFileSystem.class, fs, PropertyTable.class, "_property_table");
+            assertNotNull(actual);
+        }
     }
-     
+
     /**
      * Utility method to call a private/protected method.
      * Only use this method in test cases!!!
      */
     @Ignore
     @Test
-    public void callMethod() {
-        /*
-        final Class<? super T> clazz;
-        final T instance;
-        final Class<R> returnType;
-        final String methodName;
-        final Class<?>[] parameterTypes;
-        final Object[] parameters;
-
-        final R expected;
-        final R actual = POITestCase.callMethod(clazz, instance, returnType, methodName, parameterTypes, parameters);
-        assertEquals(expected, actual);
-        */
+    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);
+        }
     }
 
     /**
diff --git a/src/testcases/org/apache/poi/ddf/TestEscherContainerRecord.java b/src/testcases/org/apache/poi/ddf/TestEscherContainerRecord.java
index 4d8b810e5e..2f77f6c28f 100644
--- a/src/testcases/org/apache/poi/ddf/TestEscherContainerRecord.java
+++ b/src/testcases/org/apache/poi/ddf/TestEscherContainerRecord.java
@@ -17,6 +17,7 @@
 
 package org.apache.poi.ddf;
 
+import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotSame;
@@ -186,12 +187,16 @@ public final class TestEscherContainerRecord {
 	 *  but hopefully we now read the correct size.
 	 */
     @Test
-	public void testBug44857() throws Exception {
+	public void testBug44857() {
 		byte[] data = _samples.readFile("Container.dat");
 
 		// This used to fail with an OutOfMemory
 		EscherContainerRecord record = new EscherContainerRecord();
 		record.fillFields(data, 0, new DefaultEscherRecordFactory());
+
+		Class<?>[] exp = { EscherDggRecord.class, EscherContainerRecord.class, EscherOptRecord.class, EscherSplitMenuColorsRecord.class };
+		Class<?>[] act = record.getChildRecords().stream().map(Object::getClass).toArray(Class[]::new);
+		assertArrayEquals(exp, act);
 	}
 
 	/**
diff --git a/src/testcases/org/apache/poi/ddf/TestEscherDump.java b/src/testcases/org/apache/poi/ddf/TestEscherDump.java
index a8aa0510cf..b18a7cc790 100644
--- a/src/testcases/org/apache/poi/ddf/TestEscherDump.java
+++ b/src/testcases/org/apache/poi/ddf/TestEscherDump.java
@@ -18,26 +18,25 @@
 package org.apache.poi.ddf;
 
 import static org.junit.Assert.assertArrayEquals;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
 
+import java.io.ByteArrayOutputStream;
 import java.io.InputStream;
 import java.io.PrintStream;
 import java.io.UnsupportedEncodingException;
-import java.util.ArrayList;
-import java.util.List;
+import java.nio.charset.StandardCharsets;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 import org.apache.poi.POIDataSamples;
 import org.apache.poi.hssf.HSSFTestDataSamples;
 import org.apache.poi.poifs.storage.RawDataUtil;
 import org.apache.poi.util.IOUtils;
-import org.apache.poi.util.LocaleUtil;
-import org.apache.poi.util.NullOutputStream;
-import org.junit.BeforeClass;
+import org.junit.Before;
 import org.junit.Test;
 
 public class TestEscherDump {
-    private static NullPrinterStream nullPS;
-
     private static final String recordData =
         "H4sIAAAAAAAAAL2UaVCSWxjHX0SBChABLRXM1FxSEzXTzHK7dpVIcMmwxXCP9KaGTaWlGYLrtGmGmYEmYmqF2qIt4ppmjNG+" +
         "2dWulUtOUdq1NHjva8v90HT7eM+Z5znP/M9/zpk5v3mONgAoc5AANBDKeVDW0gQAjZkVCti3mKnpAExpB/m8AKTyEiTCNd2J" +
@@ -64,45 +63,52 @@ public class TestEscherDump {
         "cT19LR+PfTgjN4CKCS5Es4LS+7nLt9hQ7ejwGQnEyxebOgJzlHjotWUACpoZsFkAgGqBeUDZAzB6h4N2MFCNhmIuFJMAgPsH" +
         "eJr+iZEHAAA=";
 
+    private EscherDump dumper = new EscherDump();
+    private ByteArrayOutputStream baos = new ByteArrayOutputStream();
+    private PrintStream stream;
 
-    @BeforeClass
-    public static void init() throws UnsupportedEncodingException {
-        nullPS = new NullPrinterStream();
+    @Before
+    public void setup() throws UnsupportedEncodingException {
+        stream = new PrintStream(baos, true, StandardCharsets.UTF_8.name());
     }
 
     // simple test to at least cover some parts of the class
     @Test
     public void testSimple() throws Exception {
-        // Create a new instance of the escher dumper
-        EscherDump dumper = new EscherDump();
-
         // Decode the stream to bytes
         byte[] bytes = RawDataUtil.decompress(recordData);
-        // Dump the contents of scher to screen.
-        dumper.dump(bytes, 0, bytes.length, nullPS);
+        // Dump the contents of escher to stream.
+        dumper.dump(bytes, 0, bytes.length, stream);
+        assertEquals(216, countProperties());
+
+        baos.reset();
+        dumper.dump(0, new byte[0], stream);
+        assertEquals(0, countProperties());
 
-        dumper.dump(0, new byte[] {}, nullPS);
-        dumper.dump(new byte[] {}, 0, 0, nullPS);
+        baos.reset();
+        dumper.dump(new byte[0], 0, 0, stream);
+        assertEquals(0, countProperties());
     }
 
     @Test
     public void testWithData() {
-        new EscherDump().dump(8, new byte[] {0, 0, 0, 0, 0, 0, 0, 0}, nullPS);
+        dumper.dump(8, new byte[] {0, 0, 0, 0, 0, 0, 0, 0}, stream);
+        assertEquals(6, countProperties());
     }
 
     @Test
     public void testWithSamplefile() throws Exception {
         //InputStream stream = HSSFTestDataSamples.openSampleFileStream(")
         byte[] data = POIDataSamples.getDDFInstance().readFile("Container.dat");
-        new EscherDump().dump(data.length, data, nullPS);
-        //new EscherDump().dumpOld(data.length, new ByteArrayInputStream(data), System.out);
+        dumper.dump(data.length, data, stream);
+        assertEquals(127, countProperties());
 
         data = new byte[2586114];
-        try (InputStream stream = HSSFTestDataSamples.openSampleFileStream("44593.xls")) {
-            int bytes = IOUtils.readFully(stream, data);
-            assertTrue(bytes != -1);
-            //new EscherDump().dump(bytes, data, System.out);
-            //new EscherDump().dumpOld(bytes, new ByteArrayInputStream(data), System.out);
+        try (InputStream is = HSSFTestDataSamples.openSampleFileStream("44593.xls")) {
+            int bytes = IOUtils.readFully(is, data);
+            assertNotEquals(-1, bytes);
+            dumper.dump(data, 0, bytes, stream);
+            assertEquals(163, countProperties());
         }
     }
 
@@ -110,7 +116,6 @@ public class TestEscherDump {
     public void testCopy() throws Exception {
         byte[] data1 = RawDataUtil.decompress(recordData);
 
-        List<EscherRecord> records = new ArrayList<>();
         EscherRecordFactory recordFactory = new DefaultEscherRecordFactory();
         EscherRecord r = recordFactory.createRecord(data1, 0);
         r.fillFields(data1, recordFactory);
@@ -120,14 +125,13 @@ public class TestEscherDump {
         assertArrayEquals(data1, data2);
     }
 
-    /**
-     * Implementation of an OutputStream which does nothing, used
-     * to redirect stdout to avoid spamming the console with output
-     */
-    private static class NullPrinterStream extends PrintStream {
-        @SuppressWarnings("resource")
-        private NullPrinterStream() throws UnsupportedEncodingException {
-            super(new NullOutputStream(),true,LocaleUtil.CHARSET_1252.name());
+    private int countProperties() {
+        String data = new String(baos.toByteArray(), StandardCharsets.UTF_8);
+        Matcher matcher = Pattern.compile(",? \"[^\"]+\": ").matcher(data);
+        int count = 0;
+        while (matcher.find()) {
+            count++;
         }
+        return count;
     }
 }
diff --git a/src/testcases/org/apache/poi/hpsf/basic/TestReadAllFiles.java b/src/testcases/org/apache/poi/hpsf/basic/TestReadAllFiles.java
index 24eccf9419..4b6884165a 100644
--- a/src/testcases/org/apache/poi/hpsf/basic/TestReadAllFiles.java
+++ b/src/testcases/org/apache/poi/hpsf/basic/TestReadAllFiles.java
@@ -56,7 +56,7 @@ import org.junit.runners.Parameterized.Parameters;
 @RunWith(Parameterized.class)
 public class TestReadAllFiles {
     private static final POIDataSamples _samples = POIDataSamples.getHPSFInstance();
-    
+
     @Parameters(name="{index}: {0} using {1}")
     public static Iterable<Object[]> files() {
         File hpsfTestDir = _samples.getFile("");
@@ -82,12 +82,13 @@ public class TestReadAllFiles {
         /* Read the POI filesystem's property set streams: */
         for (POIFile pf : Util.readPropertySets(file)) {
             try (InputStream in = new ByteArrayInputStream(pf.getBytes())) {
-                PropertySetFactory.create(in);
+                PropertySet ps = PropertySetFactory.create(in);
+                assertNotNull(ps);
             }
         }
     }
-    
-    
+
+
     /**
      * This test method does a write and read back test with all POI
      * filesystems in the "data" directory by performing the following
@@ -105,7 +106,7 @@ public class TestReadAllFiles {
     public void recreate() throws IOException, HPSFException {
         /* Read the POI filesystem's property set streams: */
         Map<String,PropertySet> psMap = new HashMap<>();
-        
+
         /* Create a new POI filesystem containing the origin file's
          * property set streams: */
         ByteArrayOutputStream bos = new ByteArrayOutputStream();
@@ -125,20 +126,20 @@ public class TestReadAllFiles {
             final PropertySet ps1 = me.getValue();
             final PropertySet ps2 = PropertySetFactory.create(poiFs.getRoot(), me.getKey());
             assertNotNull(ps2);
-            
+
             /* Compare the property set stream with the corresponding one
              * from the origin file and check whether they are equal. */
-            
+
             // Because of missing 0-paddings in the original input files, the bytes might differ.
             // This fixes the comparison
             String ps1str = ps1.toString().replace(" 00", "   ").replace(".", " ").replaceAll("(?m)( +$|(size|offset): [0-9]+)","");
             String ps2str = ps2.toString().replace(" 00", "   ").replace(".", " ").replaceAll("(?m)( +$|(size|offset): [0-9]+)","");
-            
+
             assertEquals("Equality for file " + file.getName(), ps1str, ps2str);
         }
         poiFs.close();
     }
-    
+
     /**
      * This test method checks whether DocumentSummary information streams
      * can be read. This is done by opening all "Test*" files in the 'poifs' directrory
@@ -181,7 +182,7 @@ public class TestReadAllFiles {
             }
         }
     }
-    
+
     /**
      * Tests the simplified custom properties by reading them from the
      * available test files.
diff --git a/src/testcases/org/apache/poi/hpsf/basic/TestWrite.java b/src/testcases/org/apache/poi/hpsf/basic/TestWrite.java
index ff1fb0be45..a609ef7a26 100644
--- a/src/testcases/org/apache/poi/hpsf/basic/TestWrite.java
+++ b/src/testcases/org/apache/poi/hpsf/basic/TestWrite.java
@@ -35,6 +35,7 @@ import java.io.InputStream;
 import java.io.OutputStream;
 import java.io.UnsupportedEncodingException;
 import java.nio.charset.Charset;
+import java.util.ArrayList;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.List;
@@ -43,6 +44,7 @@ import java.util.Map;
 
 import org.apache.poi.POIDataSamples;
 import org.apache.poi.hpsf.ClassID;
+import org.apache.poi.hpsf.ClassIDPredefined;
 import org.apache.poi.hpsf.DocumentSummaryInformation;
 import org.apache.poi.hpsf.HPSFException;
 import org.apache.poi.hpsf.NoFormatIDException;
@@ -58,7 +60,6 @@ import org.apache.poi.hpsf.VariantSupport;
 import org.apache.poi.hpsf.WritingNotSupportedException;
 import org.apache.poi.hpsf.wellknown.PropertyIDMap;
 import org.apache.poi.poifs.eventfilesystem.POIFSReader;
-import org.apache.poi.poifs.eventfilesystem.POIFSReaderEvent;
 import org.apache.poi.poifs.eventfilesystem.POIFSReaderListener;
 import org.apache.poi.poifs.filesystem.DirectoryEntry;
 import org.apache.poi.poifs.filesystem.DocumentInputStream;
@@ -67,13 +68,10 @@ import org.apache.poi.poifs.filesystem.DocumentOutputStream;
 import org.apache.poi.poifs.filesystem.POIFSDocument;
 import org.apache.poi.poifs.filesystem.POIFSFileSystem;
 import org.apache.poi.util.CodePageUtil;
-import org.apache.poi.util.CommonsLogger;
 import org.apache.poi.util.IOUtils;
 import org.apache.poi.util.LittleEndianConsts;
 import org.apache.poi.util.TempFile;
-import org.junit.AfterClass;
 import org.junit.Assume;
-import org.junit.BeforeClass;
 import org.junit.Test;
 
 /**
@@ -93,6 +91,7 @@ public class TestWrite {
         "LANG environment variable to a proper value, e.g. " +
         "\"de_DE\".";
 
+    /*
     private static String loggerBefore;
 
     @BeforeClass
@@ -114,10 +113,10 @@ public class TestWrite {
             System.setProperty("org.apache.poi.util.POILogger", loggerBefore);
         }
     }
-    
+     */
+
     /**
-     * <p>Writes an empty property set to a POIFS and reads it back
-     * in.</p>
+     * Writes an empty property set to a POIFS and reads it back in.
      *
      * @exception IOException if an I/O exception occurs
      */
@@ -133,20 +132,16 @@ public class TestWrite {
 
         /* Write it to a POIFS and the latter to disk: */
         try (OutputStream out = new FileOutputStream(filename);
-             POIFSFileSystem poiFs = new POIFSFileSystem()) {
-            final ByteArrayOutputStream psStream = new ByteArrayOutputStream();
+             POIFSFileSystem poiFs = new POIFSFileSystem();
+             ByteArrayOutputStream psStream = new ByteArrayOutputStream()) {
             ps.write(psStream);
-            psStream.close();
-            final byte[] streamData = psStream.toByteArray();
-            poiFs.createDocument(new ByteArrayInputStream(streamData),
-                    SummaryInformation.DEFAULT_STREAM_NAME);
+            poiFs.createDocument(new ByteArrayInputStream(psStream.toByteArray()), SummaryInformation.DEFAULT_STREAM_NAME);
             poiFs.writeFilesystem(out);
         }
     }
 
     /**
-     * <p>Writes an empty property set to a POIFS and reads it back
-     * in.</p>
+     * Writes an empty property set to a POIFS and reads it back in.
      *
      * @exception IOException if an I/O exception occurs
      * @exception UnsupportedVariantTypeException if HPSF does not yet support
@@ -160,27 +155,24 @@ public class TestWrite {
         filename.deleteOnExit();
 
         /* Create a mutable property set and write it to a POIFS: */
-        final OutputStream out = new FileOutputStream(filename);
-        final POIFSFileSystem poiFs = new POIFSFileSystem();
-        final PropertySet ps = new PropertySet();
-        final Section s = ps.getSections().get(0);
-        s.setFormatID(SummaryInformation.FORMAT_ID);
-
-        final ByteArrayOutputStream psStream = new ByteArrayOutputStream();
-        ps.write(psStream);
-        psStream.close();
-        final byte[] streamData = psStream.toByteArray();
-        poiFs.createDocument(new ByteArrayInputStream(streamData),
-                             SummaryInformation.DEFAULT_STREAM_NAME);
-        poiFs.writeFilesystem(out);
-        poiFs.close();
-        out.close();
+        try (OutputStream out = new FileOutputStream(filename);
+            POIFSFileSystem poiFs = new POIFSFileSystem();
+             ByteArrayOutputStream psStream = new ByteArrayOutputStream()) {
+            final PropertySet ps = new PropertySet();
+            final Section s = ps.getSections().get(0);
+            s.setFormatID(SummaryInformation.FORMAT_ID);
+            ps.write(psStream);
+            poiFs.createDocument(new ByteArrayInputStream(psStream.toByteArray()), SummaryInformation.DEFAULT_STREAM_NAME);
+            poiFs.writeFilesystem(out);
+        }
 
         /* Read the POIFS: */
         final POIFSReader r = new POIFSReader();
-        r.registerListener(new MyPOIFSReaderListener(),
-                           SummaryInformation.DEFAULT_STREAM_NAME);
+        final List<PropertySet> psa = new ArrayList<>();
+
+        r.registerListener(getListener(psa), SummaryInformation.DEFAULT_STREAM_NAME);
         r.read(filename);
+        assertEquals(1, psa.size());
     }
 
     /**
@@ -199,45 +191,35 @@ public class TestWrite {
         final File dataDir = _samples.getFile("");
         final File filename = new File(dataDir, POI_FS);
         filename.deleteOnExit();
-        final OutputStream out = new FileOutputStream(filename);
-        final POIFSFileSystem poiFs = new POIFSFileSystem();
-
-        final PropertySet ps = new PropertySet();
-        final Section si = new Section();
-        si.setFormatID(SummaryInformation.FORMAT_ID);
-        ps.clearSections();
-        ps.addSection(si);
-
-        final Property p = new Property();
-        p.setID(PropertyIDMap.PID_AUTHOR);
-        p.setType(Variant.VT_LPWSTR);
-        p.setValue(AUTHOR);
-        si.setProperty(p);
-        si.setProperty(PropertyIDMap.PID_TITLE, Variant.VT_LPSTR, TITLE);
-
-        poiFs.createDocument(ps.toInputStream(),
-                             SummaryInformation.DEFAULT_STREAM_NAME);
-        poiFs.writeFilesystem(out);
-        poiFs.close();
-        out.close();
+        try (OutputStream out = new FileOutputStream(filename);
+            POIFSFileSystem poiFs = new POIFSFileSystem()) {
+
+            final PropertySet ps = new PropertySet();
+            final Section si = new Section();
+            si.setFormatID(SummaryInformation.FORMAT_ID);
+            ps.clearSections();
+            ps.addSection(si);
+
+            final Property p = new Property();
+            p.setID(PropertyIDMap.PID_AUTHOR);
+            p.setType(Variant.VT_LPWSTR);
+            p.setValue(AUTHOR);
+            si.setProperty(p);
+            si.setProperty(PropertyIDMap.PID_TITLE, Variant.VT_LPSTR, TITLE);
+
+            poiFs.createDocument(ps.toInputStream(), SummaryInformation.DEFAULT_STREAM_NAME);
+            poiFs.writeFilesystem(out);
+        }
 
         /* Read the POIFS: */
-        final PropertySet[] psa = new PropertySet[1];
+        final List<PropertySet> psa = new ArrayList<>();
         final POIFSReader r = new POIFSReader();
-        final POIFSReaderListener listener = event -> {
-            try {
-                psa[0] = PropertySetFactory.create(event.getStream());
-            } catch (Exception ex) {
-                fail(ex.getMessage());
-            }
-        };
-        r.registerListener(listener, SummaryInformation.DEFAULT_STREAM_NAME);
-
+        r.registerListener(getListener(psa), SummaryInformation.DEFAULT_STREAM_NAME);
         r.read(filename);
-        assertNotNull(psa[0]);
-        assertTrue(psa[0].isSummaryInformation());
+        assertEquals(1, psa.size());
+        assertTrue(psa.get(0).isSummaryInformation());
 
-        final Section s = (psa[0].getSections().get(0));
+        final Section s = psa.get(0).getSections().get(0);
         Object p1 = s.getProperty(PropertyIDMap.PID_AUTHOR);
         Object p2 = s.getProperty(PropertyIDMap.PID_TITLE);
         assertEquals(AUTHOR, p1);
@@ -247,8 +229,8 @@ public class TestWrite {
 
 
     /**
-     * <p>Writes a simple property set with two sections to a POIFS and reads it
-     * back in.</p>
+     * Writes a simple property set with two sections to a POIFS and reads it
+     * back in.
      *
      * @exception IOException if an I/O exception occurs
      * @exception WritingNotSupportedException if HPSF does not yet support
@@ -259,33 +241,30 @@ public class TestWrite {
         final String STREAM_NAME = "PropertySetStream";
         final String SECTION1 = "Section 1";
         final String SECTION2 = "Section 2";
+        final ClassID FORMATID = ClassIDPredefined.EXCEL_V12.getClassID();
 
         final File dataDir = _samples.getFile("");
         final File filename = new File(dataDir, POI_FS);
         filename.deleteOnExit();
-        final OutputStream out = new FileOutputStream(filename);
 
-        final POIFSFileSystem poiFs = new POIFSFileSystem();
-        final PropertySet ps = new PropertySet();
-        ps.clearSections();
+        try (OutputStream out = new FileOutputStream(filename);
+            POIFSFileSystem poiFs = new POIFSFileSystem()) {
+            final PropertySet ps = new PropertySet();
+            ps.clearSections();
 
-        final ClassID formatID = new ClassID();
-        formatID.setBytes(new byte[]{0, 1,  2,  3,  4,  5,  6,  7,
-                                     8, 9, 10, 11, 12, 13, 14, 15});
-        final Section s1 = new Section();
-        s1.setFormatID(formatID);
-        s1.setProperty(2, SECTION1);
-        ps.addSection(s1);
+            final Section s1 = new Section();
+            s1.setFormatID(FORMATID);
+            s1.setProperty(2, SECTION1);
+            ps.addSection(s1);
 
-        final Section s2 = new Section();
-        s2.setFormatID(formatID);
-        s2.setProperty(2, SECTION2);
-        ps.addSection(s2);
+            final Section s2 = new Section();
+            s2.setFormatID(FORMATID);
+            s2.setProperty(2, SECTION2);
+            ps.addSection(s2);
 
-        poiFs.createDocument(ps.toInputStream(), STREAM_NAME);
-        poiFs.writeFilesystem(out);
-        poiFs.close();
-        out.close();
+            poiFs.createDocument(ps.toInputStream(), STREAM_NAME);
+            poiFs.writeFilesystem(out);
+        }
 
         /* Read the POIFS: */
         final PropertySet[] psa = new PropertySet[1];
@@ -303,7 +282,7 @@ public class TestWrite {
 
         assertNotNull(psa[0]);
         Section s = (psa[0].getSections().get(0));
-        assertEquals(s.getFormatID(), formatID);
+        assertEquals(s.getFormatID(), FORMATID);
         Object p = s.getProperty(2);
         assertEquals(SECTION1, p);
         s = (psa[0].getSections().get(1));
@@ -311,16 +290,14 @@ public class TestWrite {
         assertEquals(SECTION2, p);
     }
 
-
-    static class MyPOIFSReaderListener implements POIFSReaderListener {
-        @Override
-        public void processPOIFSReaderEvent(final POIFSReaderEvent event) {
+    private static POIFSReaderListener getListener(List<PropertySet> psa) {
+        return event -> {
             try {
-                PropertySetFactory.create(event.getStream());
+                psa.add(PropertySetFactory.create(event.getStream()));
             } catch (Exception ex) {
                 fail(ex.getMessage());
             }
-        }
+        };
     }
 
     /**
@@ -419,7 +396,7 @@ public class TestWrite {
         String title = (String) sr.getProperty(PropertyIDMap.PID_TITLE);
         assertEquals(TITLE, title);
     }
-    
+
     private void checkString(final long variantType, final String value, final int codepage)
     throws UnsupportedVariantTypeException, IOException {
         for (int i=0; i<value.length(); i++) {
@@ -503,7 +480,7 @@ public class TestWrite {
         // We need to work on a File for in-place changes, so create a temp one
         final File copy = TempFile.createTempFile("Test-HPSF", "ole2");
         copy.deleteOnExit();
-        
+
         // Copy a test file over to our temp location
         try (FileOutputStream out = new FileOutputStream(copy);
              InputStream inp = _samples.openResourceAsStream("TestShiftJIS.doc")) {
@@ -710,7 +687,7 @@ public class TestWrite {
     public void dictionaryWithInvalidCodepage() throws IOException, HPSFException {
         final File copy = TempFile.createTempFile("Test-HPSF", "ole2");
         copy.deleteOnExit();
-        
+
         /* Write: */
 
         final PropertySet ps1 = new PropertySet();