More unit tests, better test coverage

src/main/java/org/gedcom4j/writer/GedcomWriter.java

@@ -127,7 +127,7 @@ public class GedcomWriter extends AbstractEmitter<Gedcom> {
     /**
      * The list of observers on string construction
      */
-    private final List<WeakReference<ConstructProgressListener>> constructObservers = new CopyOnWriteArrayList<>();
+    final List<WeakReference<ConstructProgressListener>> constructObservers = new CopyOnWriteArrayList<>();
 
     /**
      * Send a notification whenever more than this many lines are written to a file
@@ -137,7 +137,7 @@ public class GedcomWriter extends AbstractEmitter<Gedcom> {
     /**
      * The list of observers on file operations
      */
-    private final List<WeakReference<FileProgressListener>> fileObservers = new CopyOnWriteArrayList<>();
+    final List<WeakReference<FileProgressListener>> fileObservers = new CopyOnWriteArrayList<>();
 
     /**
      * The number of lines constructed as last reported to the observers
@@ -381,7 +381,6 @@ public void unregisterConstructObserver(ConstructProgressListener observer) {
                 i++;
             }
         }
-        constructObservers.add(new WeakReference<>(observer));
     }
 
     /**
@@ -400,7 +399,6 @@ public void unregisterFileObserver(FileProgressListener observer) {
                 i++;
             }
         }
-        fileObservers.add(new WeakReference<>(observer));
     }
 
     /**
@@ -507,43 +505,13 @@ protected void emit() throws GedcomWriterException {
         emitTrailer();
     }
 
-    /**
-     * Notify construct observers if more than 100 lines have been constructed since last time we notified them
-     */
-    void notifyConstructObserversIfNeeded() {
-        if (lines.size() - lastLineCountNotified > constructionNotificationRate) {
-            notifyConstructObservers(new ConstructProgressEvent(this, lines.size(), true));
-        }
-    }
-
-    /**
-     * Checks that the gedcom version specified is compatible with the data in the model. Not a perfect exhaustive check.
-     * 
-     * @throws GedcomWriterException
-     *             if data is detected that is incompatible with the selected version
-     */
-    private void checkVersionCompatibility() throws GedcomWriterException {
-
-        if (writeFrom.getHeader().getGedcomVersion() == null) {
-            // If there's not one specified, set up a default one that specifies
-            // 5.5.1
-            writeFrom.getHeader().setGedcomVersion(new GedcomVersion());
-        }
-        if (SupportedVersion.V5_5.toString().equals(writeFrom.getHeader().getGedcomVersion().getVersionNumber().getValue())) {
-            checkVersionCompatibility55();
-        } else {
-            checkVersionCompatibility551();
-        }
-
-    }
-
     /**
      * Check that the data is compatible with 5.5 style Gedcom files
      * 
      * @throws GedcomWriterVersionDataMismatchException
      *             if a data point is detected that is incompatible with the 5.5 standard
      */
-    private void checkVersionCompatibility55() throws GedcomWriterVersionDataMismatchException {
+    void checkVersionCompatibility55() throws GedcomWriterVersionDataMismatchException {
         // Now that we know if we're working with a 5.5.1 file or not, let's
         // check some data points
         if (writeFrom.getHeader().getCopyrightData() != null && writeFrom.getHeader().getCopyrightData().size() > 1) {
@@ -600,6 +568,35 @@ private void checkVersionCompatibility55() throws GedcomWriterVersionDataMismatc
         }
     }
 
+    /**
+     * Notify construct observers if more than 100 lines have been constructed since last time we notified them
+     */
+    void notifyConstructObserversIfNeeded() {
+        if (lines.size() - lastLineCountNotified > constructionNotificationRate) {
+            notifyConstructObservers(new ConstructProgressEvent(this, lines.size(), true));
+        }
+    }
+
+    /**
+     * Checks that the gedcom version specified is compatible with the data in the model. Not a perfect exhaustive check.
+     * 
+     * @throws GedcomWriterException
+     *             if data is detected that is incompatible with the selected version
+     */
+    private void checkVersionCompatibility() throws GedcomWriterException {
+
+        if (writeFrom.getHeader().getGedcomVersion() == null) {
+            // If there's not one specified, set up a default one that specifies
+            // 5.5.1
+            writeFrom.getHeader().setGedcomVersion(new GedcomVersion());
+        }
+        if (SupportedVersion.V5_5.toString().equals(writeFrom.getHeader().getGedcomVersion().getVersionNumber().getValue())) {
+            checkVersionCompatibility55();
+        } else {
+            checkVersionCompatibility551();
+        }
+    }
+
     /**
      * Check that the data is compatible with 5.5.1 style Gedcom files
      * 

src/test/java/org/gedcom4j/writer/GedcomWriterConstructionProgressAndCancellationTest.java

@@ -40,6 +40,7 @@
 import org.gedcom4j.writer.event.ConstructProgressEvent;
 import org.gedcom4j.writer.event.ConstructProgressListener;
 import org.junit.Assert;
+import org.junit.Before;
 import org.junit.Test;
 
 /**
@@ -52,7 +53,7 @@ public class GedcomWriterConstructionProgressAndCancellationTest implements Cons
     /**
      * Number of notifications received
      */
-    private int notificationCount = 0;
+    private int constructionNotificationCount = 0;
 
     /**
      * How many notifications to cancel after
@@ -69,14 +70,22 @@ public class GedcomWriterConstructionProgressAndCancellationTest implements Cons
      */
     @Override
     public void progressNotification(ConstructProgressEvent e) {
-        notificationCount++;
+        constructionNotificationCount++;
         Assert.assertTrue(e.isComplete());
         Assert.assertTrue(e.toString().startsWith("ConstructProgressEvent"));
-        if (notificationCount >= cancelAfter) {
+        if (constructionNotificationCount >= cancelAfter) {
             gw.cancel();
         }
     }
 
+    /**
+     * Set up before each test
+     */
+    @Before
+    public void setUp() {
+        constructionNotificationCount = 0;
+    }
+
     /**
      * Test with cancelling after getting a couple notifications
      * 
@@ -103,6 +112,34 @@ public void testCancellation() throws IOException, GedcomParserException, Gedcom
         gw.write(new NullOutputStream());
     }
 
+    /**
+     * Test changing construction notification rate
+     * 
+     * @throws IOException
+     *             if the file can't be read
+     * @throws GedcomParserException
+     *             if the file can't be parsed
+     * @throws GedcomWriterException
+     *             if the file can't be written (or is cancelled)
+     */
+    @SuppressWarnings("resource")
+    @Test(expected = WriterCancelledException.class)
+    public void testChangingConstructionNotificationRate() throws IOException, GedcomParserException, GedcomWriterException {
+        GedcomParser gp = new GedcomParser();
+        gp.load("sample/willis-ascii.ged");
+        Gedcom g = gp.getGedcom();
+        Validator gv = new Validator(g);
+        gv.setAutoRepairResponder(Validator.AUTO_REPAIR_ALL);
+        gv.validate(); // Cleanup whatever can be cleaned up
+        gw = new GedcomWriter(g);
+        gw.setValidationSuppressed(true);
+        gw.registerConstructObserver(this);
+        gw.setConstructionNotificationRate(1);
+        cancelAfter = 5;
+        gw.write(new NullOutputStream());
+        assertEquals(5, constructionNotificationCount);
+    }
+
     /**
      * Test without cancelling
      * 
@@ -126,7 +163,18 @@ public void testNoCancellation() throws IOException, GedcomParserException, Gedc
         gw.setValidationSuppressed(true);
         gw.registerConstructObserver(this);
         gw.write(new NullOutputStream());
-        assertEquals(40, notificationCount);
+        assertEquals(40, constructionNotificationCount);
+    }
+
+    /**
+     * Try setting the notification rate below the minimum value
+     * 
+     * @throws WriterCancelledException
+     *             if the writer gets cancelled, which won't happen in this test
+     */
+    @Test(expected = IllegalArgumentException.class)
+    public void testSetConstructionNotificationRateNegativeTest() throws WriterCancelledException {
+        new GedcomWriter(new Gedcom()).setConstructionNotificationRate(0);
     }
 
 }

src/test/java/org/gedcom4j/writer/GedcomWriterFileProgressAndCancellationTest.java

@@ -571,6 +571,66 @@ public void testNoCancellationUtf8CrOnly() throws IOException, GedcomParserExcep
         assertEquals(557056, bytesWritten);
     }
 
+    /**
+     * Test registering/unregistering observers.
+     *
+     * @throws WriterCancelledException
+     *             the writer cancelled exception
+     */
+    @Test
+    public void testRegisterUnregisterObservers() throws WriterCancelledException {
+        ConstructProgressListener c = new ConstructProgressListener() {
+            /**
+             * {@inheritDoc}
+             */
+            @Override
+            public void progressNotification(ConstructProgressEvent e) {
+                // Do nothing
+            }
+        };
+        FileProgressListener f = new FileProgressListener() {
+            /**
+             * {@inheritDoc}
+             */
+            @Override
+            public void progressNotification(FileProgressEvent e) {
+                // Do nothing
+            }
+        };
+        gw = new GedcomWriter(new Gedcom());
+        assertEquals(0, gw.constructObservers.size());
+        assertEquals(0, gw.fileObservers.size());
+        gw.registerConstructObserver(c);
+        assertEquals(1, gw.constructObservers.size());
+        assertEquals(0, gw.fileObservers.size());
+        gw.unregisterConstructObserver(c);
+        assertEquals(0, gw.constructObservers.size());
+        assertEquals(0, gw.fileObservers.size());
+
+        gw.registerFileObserver(f);
+        assertEquals(0, gw.constructObservers.size());
+        assertEquals(1, gw.fileObservers.size());
+        gw.unregisterFileObserver(f);
+        assertEquals(0, gw.constructObservers.size());
+        assertEquals(0, gw.fileObservers.size());
+
+        // Unregistering observers that aren't observing does nothing, silently
+        gw.unregisterConstructObserver(c);
+        gw.unregisterFileObserver(f);
+
+    }
+
+    /**
+     * Try setting the notification rate below the minimum value
+     * 
+     * @throws WriterCancelledException
+     *             if the writer gets cancelled, which won't happen in this test
+     */
+    @Test(expected = IllegalArgumentException.class)
+    public void testSetFileNotificationRateNegativeTest() throws WriterCancelledException {
+        new GedcomWriter(new Gedcom()).setFileNotificationRate(0);
+    }
+
     /**
      * Helper method to clean up bad data in the GEDCOM so it writes ok
      * 
@@ -601,5 +661,4 @@ private void cleanUpGedcom(GedcomParser gp, Encoding encoding) {
             }
         }
     }
-
 }