Reverted test, glossed over flakyness in library instead.

common/common-image/src/main/java/com/twelvemonkeys/image/BufferedImageFactory.java

@@ -79,7 +79,7 @@ public final class BufferedImageFactory {
     private int scanSize;
 
     private ColorModel sourceColorModel;
-    private Hashtable sourceProperties; // ImageConsumer API dictates Hashtable
+    private Hashtable<?, ?> sourceProperties; // ImageConsumer API dictates Hashtable
 
     private Object sourcePixels;
 
@@ -214,31 +214,34 @@ public final class BufferedImageFactory {
             // Wait until the producer wakes us up, by calling imageComplete
             while (fetching) {
                 try {
-                    wait(200l);
+                    wait(200L);
                 }
                 catch (InterruptedException e) {
                     throw new ImageConversionException("Image conversion aborted: " + e.getMessage(), e);
                 }
             }
 
-            if (consumerException != null) {
+            try {
-                throw new ImageConversionException("Image conversion failed: " + consumerException.getMessage(), consumerException);
+                if (consumerException != null) {
-            }
+                    throw new ImageConversionException("Image conversion failed: " + consumerException.getMessage(), consumerException);
+                }
 
-            if (colorModelOnly) {
+                if (colorModelOnly) {
-                createColorModel();
+                    createColorModel();
+                }
+                else {
+                    createBuffered();
+                }
             }
-            else {
+            finally {
-                createBuffered();
+                // Clean up, in case any objects are copied/cloned, so we can free resources
+                freeResources();
             }
         }
     }
 
     private void createColorModel() {
         colorModel = sourceColorModel;
-
-        // Clean up, in case any objects are copied/cloned, so we can free resources
-        freeResources();
     }
 
     private void createBuffered() {
@@ -253,8 +256,9 @@ public final class BufferedImageFactory {
             }
         }
 
-        // Clean up, in case any objects are copied/cloned, so we can free resources
+        if (buffered == null) {
-        freeResources();
+            throw new ImageConversionException("Could not create BufferedImage");
+        }
     }
 
     private void freeResources() {
@@ -324,12 +328,13 @@ public final class BufferedImageFactory {
      * Converts an array of {@code int} pixels to an array of {@code short}
      * pixels. The conversion is done, by masking out the
      * <em>higher 16 bits</em> of the {@code int}.
-     *
+     * <p>
      * For any given {@code int}, the {@code short} value is computed as
      * follows:
      * <blockquote>{@code
      * short value = (short) (intValue & 0x0000ffff);
      * }</blockquote>
+     * </p>
      *
      * @param inputPixels the pixel data to convert
      * @return an array of {@code short}s, same length as {@code inputPixels}
@@ -351,7 +356,7 @@ public final class BufferedImageFactory {
      * @see BufferedImageFactory#addProgressListener
      * @see BufferedImageFactory#removeProgressListener
      */
-    public static interface ProgressListener extends EventListener {
+    public interface ProgressListener extends EventListener {
 
         /**
          * Reports progress to this listener.
@@ -456,7 +461,7 @@ public final class BufferedImageFactory {
             // later replaces it with the default RGB in the first setPixels call
             // (this is probably allowed according to the spec, but it's a waste of time and space).
             if (sourceColorModel != colorModel) {
-                if (/*sourceColorModel == null ||*/ sourcePixels == null) {
+                if (sourcePixels == null) {
                     sourceColorModel = colorModel;
                 }
                 else {
@@ -478,10 +483,8 @@ public final class BufferedImageFactory {
                 producer.removeConsumer(this);
             }
 
-            switch (status) {
+            if (status == ImageConsumer.IMAGEERROR) {
-                case ImageConsumer.IMAGEERROR:
+                consumerException = new ImageConversionException("ImageConsumer.IMAGEERROR");
-                    consumerException = new ImageConversionException("ImageConsumer.IMAGEERROR");
-                break;
             }
 
             synchronized (BufferedImageFactory.this) {

common/common-image/src/test/java/com/twelvemonkeys/image/BufferedImageFactoryTest.java

@@ -40,7 +40,6 @@ import java.net.URL;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
 /**
@@ -89,17 +88,12 @@ public class BufferedImageFactoryTest {
 
     // This is a little random, and it would be nicer if we could throw an IllegalArgumentException on create.
     // Unfortunately, the API doesn't allow this...
-    @Test(timeout = 1000)
+    @Test(timeout = 1000, expected = ImageConversionException.class)
     public void testGetBufferedImageErrorSourceURL() {
         Image source = Toolkit.getDefaultToolkit().createImage(getClass().getResource("/META-INF/MANIFEST.MF"));
 
-        try {
+        BufferedImageFactory factory = new BufferedImageFactory(source);
-            BufferedImageFactory factory = new BufferedImageFactory(source);
+        factory.getBufferedImage();
-            assertNull(factory.getBufferedImage()); // Should normally not be reached
-        }
-        catch (ImageConversionException ignore) {
-            // This exception is allowed and *expected*, however this behavior is flaky in some environments...
-        }
     }
 
     @Test