Changeset 16768 in josm for trunk

Timestamp:

2020-07-14T20:19:40+02:00 (4 years ago)

Author:

simon04

Message:

fix #19449 ImageProviderTest.testGetPadded has slow startup, and a race condition (patch by johsin18, modified)

Location:

trunk

Files:

• src/org/openstreetmap/josm/gui/tagging/presets/TaggingPreset.java (modified) (2 diffs)
• test/unit/org/openstreetmap/josm/gui/preferences/map/TaggingPresetPreferenceTestIT.java (modified) (3 diffs)
• test/unit/org/openstreetmap/josm/gui/tagging/presets/TaggingPresetsTest.java (modified) (2 diffs)
• test/unit/org/openstreetmap/josm/tools/ImageProviderGetPaddedTest.java (added)
• test/unit/org/openstreetmap/josm/tools/ImageProviderTest.java (modified) (5 diffs)

trunk/src/org/openstreetmap/josm/gui/tagging/presets/TaggingPreset.java

@@ -229 +229 @@
         File arch = TaggingPresetReader.getZipIcons();
         final Collection<String> s = Config.getPref().getList("taggingpreset.icon.sources", null);
-        this.iconFuture = new ImageProvider(iconName)
+        this.iconFuture = new CompletableFuture<>();
+        new ImageProvider(iconName)
             .setDirs(s)
             .setId("presets")
@@ -242 +243 @@
                             Logging.warn(toString() + ": " + PRESET_ICON_ERROR_MSG_PREFIX + iconName);
                             Logging.warn(e);
+                        } finally {
+                            iconFuture.complete(null);
                         }
                     });
                 } else {
                     Logging.warn(toString() + ": " + PRESET_ICON_ERROR_MSG_PREFIX + iconName);
+                    iconFuture.complete(null);
                 }
             });

trunk/test/unit/org/openstreetmap/josm/gui/preferences/map/TaggingPresetPreferenceTestIT.java

@@ -13 +13 @@
 import java.util.List;
 import java.util.Locale;
-import java.util.Objects;
 import java.util.Set;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.TimeoutException;
 
 import org.junit.BeforeClass;
@@ -29 +25 @@
 import org.openstreetmap.josm.gui.tagging.presets.TaggingPreset;
 import org.openstreetmap.josm.gui.tagging.presets.TaggingPresetReader;
+import org.openstreetmap.josm.gui.tagging.presets.TaggingPresetsTest;
 import org.openstreetmap.josm.gui.tagging.presets.items.Link;
 import org.openstreetmap.josm.spi.preferences.Config;
@@ -116 +113 @@
         Collection<TaggingPreset> presets = TaggingPresetReader.readAll(source.url, true);
         assertFalse(presets.isEmpty());
-        // wait for asynchronous icon loading
-        presets.parallelStream().map(TaggingPreset::getIconLoadingTask).filter(Objects::nonNull).forEach(t -> {
-            try {
-                t.get(30, TimeUnit.SECONDS);
-            } catch (InterruptedException | ExecutionException | TimeoutException e) {
-                Logging.error(e);
-            }
-        });
+        TaggingPresetsTest.waitForIconLoading(presets);
         // check that links are correct and not redirections
         presets.parallelStream().flatMap(x -> x.data.stream().filter(i -> i instanceof Link).map(i -> ((Link) i).getUrl())).forEach(u -> {

trunk/test/unit/org/openstreetmap/josm/gui/tagging/presets/TaggingPresetsTest.java

@@ -1 +1 @@
 // License: GPL. For details, see LICENSE file.
 package org.openstreetmap.josm.gui.tagging.presets;
+
+import java.util.Collection;
+import java.util.Objects;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
 
 import org.junit.Rule;
 import org.junit.Test;
 import org.openstreetmap.josm.testutils.JOSMTestRules;
+import org.openstreetmap.josm.tools.Logging;
 
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
@@ -29 +36 @@
         UtilityClassTestUtil.assertUtilityClassWellDefined(TaggingPresets.class);
     }
+
+    /**
+     * Wait for asynchronous icon loading
+     * @param presets presets collection
+     */
+    public static void waitForIconLoading(Collection<TaggingPreset> presets) {
+        presets.parallelStream().map(TaggingPreset::getIconLoadingTask).filter(Objects::nonNull).forEach(t -> {
+            try {
+                t.get(30, TimeUnit.SECONDS);
+            } catch (InterruptedException | ExecutionException | TimeoutException e) {
+                Logging.error(e);
+            }
+        });
+    }
 }

trunk/test/unit/org/openstreetmap/josm/tools/ImageProviderTest.java

@@ -6 +6 @@
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
@@ -23 +22 @@
 import java.io.File;
 import java.io.IOException;
-import java.util.EnumSet;
 import java.util.List;
 import java.util.logging.Handler;
@@ -40 +38 @@
 import org.openstreetmap.josm.JOSMFixture;
 import org.openstreetmap.josm.TestUtils;
-import org.openstreetmap.josm.data.osm.Node;
-import org.openstreetmap.josm.data.osm.OsmUtils;
 import org.openstreetmap.josm.testutils.JOSMTestRules;
-import org.openstreetmap.josm.tools.ImageProvider.GetPaddedOptions;
 
 import com.kitfox.svg.SVGConst;
@@ -59 +54 @@
     @Rule
     @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD")
-    public JOSMTestRules test = new JOSMTestRules().mapStyles().presets();
+    public JOSMTestRules test = new JOSMTestRules();
 
     private static final class LogHandler14319 extends Handler {
@@ -151 +146 @@
                 "iVBORw0KGgoAAAANSUhEUgAAAAUAAAAFCAYAAACNbyblAAAAHElEQVQI12P4"+
                 "//8/w38GIAXDIBKE0DHxgljNBAAO9TXL0Y4OHwAAAABJRU5ErkJggg=="));
-    }
-
-    /**
-     * Unit test of {@link ImageProvider#getPadded}.
-     */
-    @Test
-    public void testGetPadded() {
-        final EnumSet<GetPaddedOptions> noDefault = EnumSet.of(GetPaddedOptions.NO_DEFAULT);
-        final Dimension iconSize = new Dimension(16, 16);
-
-        assertNull(ImageProvider.getPadded(new Node(), new Dimension(0, 0)));
-        assertNotNull(ImageProvider.getPadded(new Node(), iconSize));
-        assertNull(ImageProvider.getPadded(new Node(), iconSize, noDefault));
-        assertNotNull(ImageProvider.getPadded(OsmUtils.createPrimitive("node amenity=restaurant"), iconSize, noDefault));
-        assertNull(ImageProvider.getPadded(OsmUtils.createPrimitive("node barrier=hedge"), iconSize,
-                EnumSet.of(GetPaddedOptions.NO_DEFAULT, GetPaddedOptions.NO_DEPRECATED)));
-        assertNotNull(ImageProvider.getPadded(OsmUtils.createPrimitive("way waterway=stream"), iconSize, noDefault));
-        assertNotNull(ImageProvider.getPadded(OsmUtils.createPrimitive("relation type=route route=railway"), iconSize, noDefault));
     }