From c531d4f5d35e15a50ee3c4225d0527c61f3c7dc6 Mon Sep 17 00:00:00 2001 From: Harald Kuhr Date: Tue, 4 Apr 2023 16:30:45 +0200 Subject: [PATCH] Reverted test, glossed over flakyness in library instead. --- .../image/BufferedImageFactory.java | 45 ++++++++++--------- .../image/BufferedImageFactoryTest.java | 12 ++--- 2 files changed, 27 insertions(+), 30 deletions(-) diff --git a/common/common-image/src/main/java/com/twelvemonkeys/image/BufferedImageFactory.java b/common/common-image/src/main/java/com/twelvemonkeys/image/BufferedImageFactory.java index b8458985..8ebf6238 100755 --- a/common/common-image/src/main/java/com/twelvemonkeys/image/BufferedImageFactory.java +++ b/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) { - throw new ImageConversionException("Image conversion failed: " + consumerException.getMessage(), consumerException); - } + try { + if (consumerException != null) { + throw new ImageConversionException("Image conversion failed: " + consumerException.getMessage(), consumerException); + } - if (colorModelOnly) { - createColorModel(); + if (colorModelOnly) { + createColorModel(); + } + else { + createBuffered(); + } } - else { - createBuffered(); + finally { + // 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 - freeResources(); + if (buffered == null) { + 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 * higher 16 bits of the {@code int}. - * + *

* For any given {@code int}, the {@code short} value is computed as * follows: *

{@code * short value = (short) (intValue & 0x0000ffff); * }
+ *

* * @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) { - case ImageConsumer.IMAGEERROR: - consumerException = new ImageConversionException("ImageConsumer.IMAGEERROR"); - break; + if (status == ImageConsumer.IMAGEERROR) { + consumerException = new ImageConversionException("ImageConsumer.IMAGEERROR"); } synchronized (BufferedImageFactory.this) { diff --git a/common/common-image/src/test/java/com/twelvemonkeys/image/BufferedImageFactoryTest.java b/common/common-image/src/test/java/com/twelvemonkeys/image/BufferedImageFactoryTest.java index c8989f69..4bb5d00a 100644 --- a/common/common-image/src/test/java/com/twelvemonkeys/image/BufferedImageFactoryTest.java +++ b/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); - 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... - } + BufferedImageFactory factory = new BufferedImageFactory(source); + factory.getBufferedImage(); } @Test