Merge branch 'master' into mirror

JOSM · Aug 12, 2018 · 524e294 · 524e294
2 parents 3b3e033 + bb6792f
commit 524e294
Show file tree

Hide file tree

Showing 10 changed files with 196 additions and 114 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -1,5 +1,5 @@
 language: java
-script: (eval "$ANT_INVOCATION $(test $TRAVIS_JDK_VERSION = openjdk8 && echo -DnoJavaFX=true) test")
+script: (eval "$ANT_INVOCATION $(test $TRAVIS_JDK_VERSION = openjdk8 && echo -DnoJavaFX=true) test-unit-hardfail")
 os: linux
 dist: trusty
 env:

diff --git a/build.xml b/build.xml
@@ -506,7 +506,7 @@ Build-Date: ${build.tstamp}
             <echo message="Running @{testfamily}@{testITsuffix} tests with JUnit"/>
             <jacoco:coverage destfile="${test.dir}/jacoco@{testITsuffix}.exec" enabled="@{coverage}" includes="${jacoco.includes}"
                 inclbootstrapclasses="${jacoco.inclbootstrapclasses}" inclnolocationclasses="${jacoco.inclnolocationclasses}">
-                <junit printsummary="yes" fork="true" forkmode="once">
+                <junit printsummary="yes" fork="true" forkmode="once" failureproperty="test.@{testfamily}@{testITsuffix}.failed">
                     <jvmarg value="-Dfile.encoding=UTF-8"/>
                     <jvmarg value="-javaagent:${test.dir}/lib/jmockit-1.41.jar"/>
                     <jvmarg value="--add-modules" if:set="isJava9" unless:set="isJava11" />
@@ -558,15 +558,45 @@ Build-Date: ${build.tstamp}
         <call-junit testfamily="unit"/>
         <call-junit testfamily="functional"/>
     </target>
+    <target name="test-hardfail" depends="test" description="Run 'test' target but abort if tests failed">
+        <fail message="'test' failed">
+            <condition>
+                <or>
+                    <isset property="test.unit.failed"/>
+                    <isset property="test.functional.failed"/>
+                </or>
+            </condition>
+        </fail>
+    </target>
+    <target name="test-unit" depends="test-compile" unless="test-unit.notRequired"
+        description="Run unit tests. OSM API (TEST) account shall be set with -Dosm.username and -Dosm.password">
+        <call-junit testfamily="unit"/>
+    </target>
+    <target name="test-unit-hardfail" depends="test-unit" description="Run 'test-unit' target but abort if tests failed">
+        <fail message="'test-unit' failed" if="test.unit.failed"/>
+    </target>
     <target name="test-it" depends="test-compile" unless="test-it.notRequired"
         description="Run integration tests. OSM API (TEST) account shall be set with -Dosm.username and -Dosm.password">
         <call-junit testfamily="unit" testITsuffix="IT"/>
         <call-junit testfamily="functional" testITsuffix="IT"/>
     </target>
+    <target name="test-it-hardfail" depends="test-it" description="Run 'test-it' target but abort if tests failed">
+        <fail message="'test-it' failed">
+            <condition>
+                <or>
+                    <isset property="test.unitIT.failed"/>
+                    <isset property="test.functionalIT.failed"/>
+                </or>
+            </condition>
+        </fail>
+    </target>
     <target name="test-perf" depends="test-compile" unless="test-perf.notRequired"
         description="Run performance tests. OSM API (TEST) account shall be set with -Dosm.username and -Dosm.password">
         <call-junit testfamily="performance" coverage="false"/>
     </target>
+    <target name="test-perf-hardfail" depends="test-perf" description="Run 'test-perf' target but abort if tests failed">
+        <fail message="'test-perf' failed" if="test.performance.failed"/>
+    </target>
     <target name="test-html" depends="test, test-it, test-perf" description="Generate HTML test reports">
         <!-- May require additional ant dependencies like ant-trax package -->
         <junitreport todir="${test.dir}/report">

diff --git a/src/org/openstreetmap/josm/Main.java b/src/org/openstreetmap/josm/Main.java
@@ -1,22 +1,15 @@
 // License: GPL. For details, see LICENSE file.
 package org.openstreetmap.josm;
 
-import static org.openstreetmap.josm.tools.I18n.tr;
-
 import java.awt.Component;
-import java.io.IOException;
 import java.net.URL;
-import java.nio.file.InvalidPathException;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Map;
 import java.util.Set;
 
 import org.openstreetmap.josm.data.Preferences;
 import org.openstreetmap.josm.data.UndoRedoHandler;
-import org.openstreetmap.josm.data.coor.conversion.CoordinateFormatManager;
-import org.openstreetmap.josm.data.coor.conversion.DecimalDegreesCoordinateFormat;
-import org.openstreetmap.josm.data.coor.conversion.ICoordinateFormat;
 import org.openstreetmap.josm.data.osm.DataSet;
 import org.openstreetmap.josm.data.osm.IPrimitive;
 import org.openstreetmap.josm.data.osm.OsmData;
@@ -28,10 +21,9 @@
 import org.openstreetmap.josm.io.FileWatcher;
 import org.openstreetmap.josm.io.NetworkManager;
 import org.openstreetmap.josm.io.OnlineResource;
+import org.openstreetmap.josm.spi.lifecycle.Lifecycle;
 import org.openstreetmap.josm.spi.preferences.Config;
 import org.openstreetmap.josm.spi.preferences.IUrls;
-import org.openstreetmap.josm.tools.ImageProvider;
-import org.openstreetmap.josm.tools.Logging;
 import org.openstreetmap.josm.tools.PlatformHook;
 import org.openstreetmap.josm.tools.PlatformManager;
 
@@ -146,47 +138,17 @@ public Collection<? extends IPrimitive> getInProgressISelection() {
     //  Implementation part
     ///////////////////////////////////////////////////////////////////////////
 
-    /**
-     * Should be called before the main constructor to setup some parameter stuff
-     */
-    public static void preConstructorInit() {
-        // init default coordinate format
-        ICoordinateFormat fmt = CoordinateFormatManager.getCoordinateFormat(Config.getPref().get("coordinates"));
-        if (fmt == null) {
-            fmt = DecimalDegreesCoordinateFormat.INSTANCE;
-        }
-        CoordinateFormatManager.setCoordinateFormat(fmt);
-    }
-
     /**
      * Closes JOSM and optionally terminates the Java Virtual Machine (JVM).
      * @param exit If {@code true}, the JVM is terminated by running {@link System#exit} with a given return code.
      * @param exitCode The return code
      * @return {@code true}
      * @since 12636
+     * @deprecated Use {@link Lifecycle#exitJosm}
      */
+    @Deprecated
     public static boolean exitJosm(boolean exit, int exitCode) {
-        if (Main.main != null) {
-            Main.main.shutdown();
-        }
-
-        if (exit) {
-            System.exit(exitCode);
-        }
-        return true;
-    }
-
-    /**
-     * Shutdown JOSM.
-     */
-    protected void shutdown() {
-        ImageProvider.shutdown(false);
-        try {
-            pref.saveDefaults();
-        } catch (IOException | InvalidPathException ex) {
-            Logging.log(Logging.LEVEL_WARN, tr("Failed to save default preferences."), ex);
-        }
-        ImageProvider.shutdown(true);
+        return Lifecycle.exitJosm(exit, exitCode);
     }
 
     /**

diff --git a/src/org/openstreetmap/josm/gui/MainApplication.java b/src/org/openstreetmap/josm/gui/MainApplication.java
@@ -86,7 +86,6 @@
 import org.openstreetmap.josm.data.UndoRedoHandler;
 import org.openstreetmap.josm.data.UndoRedoHandler.CommandQueueListener;
 import org.openstreetmap.josm.data.Version;
-import org.openstreetmap.josm.data.cache.JCSCacheManager;
 import org.openstreetmap.josm.data.oauth.OAuthAccessTokenHolder;
 import org.openstreetmap.josm.data.osm.DataSet;
 import org.openstreetmap.josm.data.osm.IPrimitive;
@@ -346,6 +345,7 @@ public MainApplication(MainFrame mainFrame) {
         this.mainFrame = mainFrame;
         getLayerManager().addLayerChangeListener(undoRedoCleaner);
         ProjectionRegistry.setboundsProvider(mainBoundsProvider);
+        Lifecycle.setShutdownSequence(new MainTermination(this));
     }
 
     /**
@@ -408,31 +408,13 @@ protected void initializeMainWindow() {
         mainPanel.reAddListeners();
     }
 
-    @Override
-    protected void shutdown() {
-        try {
-            worker.shutdown();
-        } catch (SecurityException e) {
-            Logging.log(Logging.LEVEL_ERROR, "Unable to shutdown worker", e);
-        }
-        JCSCacheManager.shutdown();
-
-        if (mainFrame != null) {
-            mainFrame.storeState();
-        }
-        if (map != null) {
-            map.rememberToggleDialogWidth();
-        }
-        // Remove all layers because somebody may rely on layerRemoved events (like AutosaveTask)
-        layerManager.resetState();
-        super.shutdown();
-
-        try {
-            // in case the current task still hasn't finished
-            worker.shutdownNow();
-        } catch (SecurityException e) {
-            Logging.log(Logging.LEVEL_ERROR, "Unable to shutdown worker", e);
-        }
+    /**
+     * Returns the JOSM main frame.
+     * @return the JOSM main frame
+     * @since 14140
+     */
+    public final MainFrame getMainFrame() {
+        return mainFrame;
     }
 
     @Override
@@ -565,7 +547,7 @@ public static boolean exitJosm(boolean exit, int exitCode, SaveLayersDialog.Reas
                 SaveLayersDialog.saveUnsavedModifications(layerManager.getLayers(),
                         reason != null ? reason : SaveLayersDialog.Reason.EXIT)));
         if (proceed) {
-            return Main.exitJosm(exit, exitCode);
+            return Lifecycle.exitJosm(exit, exitCode);
         }
         return false;
     }
@@ -979,7 +961,6 @@ public void finish(Object status) {
         ProjectionPreference.setProjection();
         setupNadGridSources();
         GuiHelper.translateJavaInternalMessages();
-        preConstructorInit();
 
         monitor.indeterminateSubTask(tr("Creating main GUI"));
         Lifecycle.initialize(new MainInitialization(new MainApplication(mainFrame)));

diff --git a/src/org/openstreetmap/josm/gui/MainInitialization.java b/src/org/openstreetmap/josm/gui/MainInitialization.java
@@ -13,6 +13,9 @@
 
 import org.openstreetmap.gui.jmapviewer.FeatureAdapter;
 import org.openstreetmap.josm.data.UndoRedoHandler;
+import org.openstreetmap.josm.data.coor.conversion.CoordinateFormatManager;
+import org.openstreetmap.josm.data.coor.conversion.DecimalDegreesCoordinateFormat;
+import org.openstreetmap.josm.data.coor.conversion.ICoordinateFormat;
 import org.openstreetmap.josm.data.validation.OsmValidator;
 import org.openstreetmap.josm.gui.layer.ImageryLayer;
 import org.openstreetmap.josm.gui.layer.Layer;
@@ -27,6 +30,7 @@
 import org.openstreetmap.josm.io.OsmTransferCanceledException;
 import org.openstreetmap.josm.spi.lifecycle.InitializationSequence;
 import org.openstreetmap.josm.spi.lifecycle.InitializationTask;
+import org.openstreetmap.josm.spi.preferences.Config;
 import org.openstreetmap.josm.tools.I18n;
 import org.openstreetmap.josm.tools.Logging;
 import org.openstreetmap.josm.tools.OpenBrowser;
@@ -56,6 +60,13 @@ public MainInitialization(MainApplication application) {
     @Override
     public List<InitializationTask> beforeInitializationTasks() {
         return Arrays.asList(
+            new InitializationTask(tr("Initializing coordinate format"), () -> {
+                ICoordinateFormat fmt = CoordinateFormatManager.getCoordinateFormat(Config.getPref().get("coordinates"));
+                if (fmt == null) {
+                    fmt = DecimalDegreesCoordinateFormat.INSTANCE;
+                }
+                CoordinateFormatManager.setCoordinateFormat(fmt);
+            }),
             new InitializationTask(tr("Starting file watcher"), FileWatcher.getDefaultInstance()::start),
             new InitializationTask(tr("Executing platform startup hook"),
                     () -> PlatformManager.getPlatform().startupHook(MainApplication::askUpdateJava)),

diff --git a/src/org/openstreetmap/josm/gui/MainTermination.java b/src/org/openstreetmap/josm/gui/MainTermination.java
@@ -0,0 +1,64 @@
+// License: GPL. For details, see LICENSE file.
+package org.openstreetmap.josm.gui;
+
+import static org.openstreetmap.josm.tools.I18n.tr;
+
+import java.io.IOException;
+import java.nio.file.InvalidPathException;
+import java.util.Objects;
+
+import org.openstreetmap.josm.Main;
+import org.openstreetmap.josm.data.cache.JCSCacheManager;
+import org.openstreetmap.josm.tools.ImageProvider;
+import org.openstreetmap.josm.tools.Logging;
+
+/**
+ * JOSM termination sequence.
+ * @since 14140
+ */
+public class MainTermination implements Runnable {
+
+    private final MainApplication application;
+
+    /**
+     * Constructs a new {@code MainTermination}
+     * @param application Main application. Must not be null
+     */
+    public MainTermination(MainApplication application) {
+        this.application = Objects.requireNonNull(application);
+    }
+
+    @Override
+    public void run() {
+        try {
+            MainApplication.worker.shutdown();
+        } catch (SecurityException e) {
+            Logging.log(Logging.LEVEL_ERROR, "Unable to shutdown worker", e);
+        }
+        JCSCacheManager.shutdown();
+
+        if (application.getMainFrame() != null) {
+            application.getMainFrame().storeState();
+        }
+        if (MainApplication.getMap() != null) {
+            MainApplication.getMap().rememberToggleDialogWidth();
+        }
+        // Remove all layers because somebody may rely on layerRemoved events (like AutosaveTask)
+        MainApplication.getLayerManager().resetState();
+        ImageProvider.shutdown(false);
+        try {
+            Main.pref.saveDefaults();
+        } catch (IOException | InvalidPathException ex) {
+            Logging.log(Logging.LEVEL_WARN, tr("Failed to save default preferences."), ex);
+        }
+        ImageProvider.shutdown(true);
+
+        try {
+            // in case the current task still hasn't finished
+            MainApplication.worker.shutdownNow();
+        } catch (SecurityException e) {
+            Logging.log(Logging.LEVEL_ERROR, "Unable to shutdown worker", e);
+        }
+    }
+}
+
diff --git a/src/org/openstreetmap/josm/spi/lifecycle/Lifecycle.java b/src/org/openstreetmap/josm/spi/lifecycle/Lifecycle.java
@@ -8,6 +8,7 @@
 import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
 
+import org.openstreetmap.josm.tools.JosmRuntimeException;
 import org.openstreetmap.josm.tools.Logging;
 import org.openstreetmap.josm.tools.Utils;
 import org.openstreetmap.josm.tools.bugreport.BugReport;
@@ -20,6 +21,8 @@ public final class Lifecycle {
 
     private static volatile InitStatusListener initStatusListener;
 
+    private static volatile Runnable shutdownSequence;
+
     private Lifecycle() {
         // Hide constructor
     }
@@ -40,6 +43,24 @@ public static void setInitStatusListener(InitStatusListener listener) {
         initStatusListener = Objects.requireNonNull(listener);
     }
 
+    /**
+     * Gets shutdown sequence.
+     * @return shutdown sequence
+     * @since 14140
+     */
+    public static Runnable getShutdownSequence() {
+        return shutdownSequence;
+    }
+
+    /**
+     * Sets shutdown sequence.
+     * @param sequence shutdown sequence. Must not be null
+     * @since 14140
+     */
+    public static void setShutdownSequence(Runnable sequence) {
+        shutdownSequence = Objects.requireNonNull(sequence);
+    }
+
     /**
      * Initializes the main object. A lot of global variables are initialized here.
      * @param initSequence Initialization sequence
@@ -65,7 +86,7 @@ public static void initialize(InitializationSequence initSequence) {
                 Logging.log(Logging.LEVEL_ERROR, "Unable to shutdown executor service", e);
             }
         } catch (InterruptedException | ExecutionException ex) {
-            throw new RuntimeException(ex);
+            throw new JosmRuntimeException(ex);
         }
 
         // Initializes tasks that must be run after parallel tasks
@@ -76,11 +97,29 @@ private static void runInitializationTasks(List<InitializationTask> tasks) {
         for (InitializationTask task : tasks) {
             try {
                 task.call();
-            } catch (RuntimeException e) {
+            } catch (JosmRuntimeException e) {
                 // Can happen if the current projection needs NTV2 grid which is not available
                 // In this case we want the user be able to change his projection
                 BugReport.intercept(e).warn();
             }
         }
     }
+
+    /**
+     * Closes JOSM and optionally terminates the Java Virtual Machine (JVM).
+     * @param exit If {@code true}, the JVM is terminated by running {@link System#exit} with a given return code.
+     * @param exitCode The return code
+     * @return {@code true}
+     * @since 14140
+     */
+    public static boolean exitJosm(boolean exit, int exitCode) {
+        if (shutdownSequence != null) {
+            shutdownSequence.run();
+        }
+
+        if (exit) {
+            System.exit(exitCode);
+        }
+        return true;
+    }
 }