Changeset 14052 in josm for trunk/src

Timestamp:

2018-07-26T22:01:31+02:00 (6 years ago)

Author:

Don-vip

Message:

see #16010 - use JMockit to enable more extensive test coverage (patch by ris, modified)

see ​https://github.com/openstreetmap/josm/pull/24/commits for details

Location:

trunk/src/org/openstreetmap/josm

Files:

• data/preferences/JosmBaseDirectories.java (modified) (1 diff)
• gui/ConditionalOptionPaneUtil.java (modified) (1 diff)
• gui/MainApplication.java (modified) (2 diffs)
• gui/MapViewState.java (modified) (2 diffs)
• gui/NavigatableComponent.java (modified) (2 diffs)
• gui/io/AsynchronousUploadPrimitivesTask.java (modified) (2 diffs)
• gui/preferences/PreferenceTabbedPane.java (modified) (4 diffs)
• gui/preferences/advanced/ExportProfileAction.java (modified) (1 diff)
• gui/preferences/advanced/PreferencesTable.java (modified) (4 diffs)
• gui/preferences/plugin/PluginPreference.java (modified) (3 diffs)
• gui/progress/swing/ProgressMonitorExecutor.java (modified) (2 diffs)
• gui/util/GuiHelper.java (modified) (4 diffs)
• tools/Logging.java (modified) (3 diffs)

trunk/src/org/openstreetmap/josm/data/preferences/JosmBaseDirectories.java

@@ -155 +155 @@
         return cacheDir;
     }
+
+    /**
+     * Clears any previously calculated values used for {@link #getPreferencesDirectory(boolean)},
+     * {@link #getCacheDirectory(boolean)} or {@link #getUserDataDirectory(boolean)}. Useful for tests.
+     * @since 14052
+     */
+    public void clearMemos() {
+        this.preferencesDir = null;
+        this.cacheDir = null;
+        this.userdataDir = null;
+    }
 }

trunk/src/org/openstreetmap/josm/gui/ConditionalOptionPaneUtil.java

@@ -265 +265 @@
      *
      */
-    static class MessagePanel extends JPanel {
+    public static class MessagePanel extends JPanel {
         private final JRadioButton cbShowPermanentDialog = new JRadioButton(NotShowAgain.PERMANENT.getLabel());
         private final JRadioButton cbShowSessionDialog = new JRadioButton(NotShowAgain.SESSION.getLabel());

trunk/src/org/openstreetmap/josm/gui/MainApplication.java

@@ -488 +488 @@
     @Override
     protected void shutdown() {
-        if (!GraphicsEnvironment.isHeadless()) {
-            try {
-                worker.shutdown();
-            } catch (SecurityException e) {
-                Logging.log(Logging.LEVEL_ERROR, "Unable to shutdown worker", e);
-            }
-            JCSCacheManager.shutdown();
-        }
+        try {
+            worker.shutdown();
+        } catch (SecurityException e) {
+            Logging.log(Logging.LEVEL_ERROR, "Unable to shutdown worker", e);
+        }
+        JCSCacheManager.shutdown();
+
         if (mainFrame != null) {
             mainFrame.storeState();
@@ -505 +504 @@
         layerManager.resetState();
         super.shutdown();
-        if (!GraphicsEnvironment.isHeadless()) {
-            try {
-                worker.shutdownNow();
-            } catch (SecurityException e) {
-                Logging.log(Logging.LEVEL_ERROR, "Unable to shutdown worker", e);
-            }
+
+        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);
         }
     }

trunk/src/org/openstreetmap/josm/gui/MapViewState.java

@@ -3 +3 @@
 
 import java.awt.Container;
-import java.awt.GraphicsEnvironment;
 import java.awt.Point;
 import java.awt.geom.AffineTransform;
@@ -146 +145 @@
 
     private static Point findTopLeftOnScreen(JComponent position) {
-        if (GraphicsEnvironment.isHeadless()) {
-            // in our imaginary universe the window is always (10, 10) from the top left of the screen
-            Point topLeftInWindow = findTopLeftInWindow(position);
-            return new Point(topLeftInWindow.x + 10, topLeftInWindow.y + 10);
-        } else {
-            try {
-                return position.getLocationOnScreen();
-            } catch (JosmRuntimeException | IllegalArgumentException | IllegalStateException e) {
-                throw BugReport.intercept(e).put("position", position).put("parent", position::getParent);
-            }
+        try {
+            return position.getLocationOnScreen();
+        } catch (JosmRuntimeException | IllegalArgumentException | IllegalStateException e) {
+            throw BugReport.intercept(e).put("position", position).put("parent", position::getParent);
         }
     }

trunk/src/org/openstreetmap/josm/gui/NavigatableComponent.java

@@ -3 +3 @@
 
 import java.awt.Cursor;
-import java.awt.GraphicsEnvironment;
 import java.awt.Point;
 import java.awt.Rectangle;
@@ -326 +325 @@
 
     protected boolean isVisibleOnScreen() {
-        return GraphicsEnvironment.isHeadless() || (
-            SwingUtilities.getWindowAncestor(this) != null && isShowing()
-        );
+        return SwingUtilities.getWindowAncestor(this) != null && isShowing();
     }
 

trunk/src/org/openstreetmap/josm/gui/io/AsynchronousUploadPrimitivesTask.java

@@ -4 +4 @@
 import static org.openstreetmap.josm.tools.I18n.tr;
 
-import java.awt.GraphicsEnvironment;
 import java.util.Optional;
 
@@ -86 +85 @@
         synchronized (AsynchronousUploadPrimitivesTask.class) {
             if (asynchronousUploadPrimitivesTask != null) {
-                if (!GraphicsEnvironment.isHeadless()) {
-                    GuiHelper.runInEDTAndWait(() ->
-                            JOptionPane.showMessageDialog(MainApplication.parent,
-                                    tr("A background upload is already in progress. " +
-                                            "Kindly wait for it to finish before uploading new changes")));
-                }
+                GuiHelper.runInEDTAndWait(() ->
+                        JOptionPane.showMessageDialog(MainApplication.parent,
+                                tr("A background upload is already in progress. " +
+                                        "Kindly wait for it to finish before uploading new changes")));
                 return Optional.empty();
             } else {

trunk/src/org/openstreetmap/josm/gui/preferences/PreferenceTabbedPane.java

@@ -26 +26 @@
 import javax.swing.JScrollPane;
 import javax.swing.JTabbedPane;
-import javax.swing.SwingUtilities;
 import javax.swing.event.ChangeEvent;
 import javax.swing.event.ChangeListener;
@@ -60 +59 @@
 import org.openstreetmap.josm.gui.preferences.validator.ValidatorTagCheckerRulesPreference;
 import org.openstreetmap.josm.gui.preferences.validator.ValidatorTestsPreference;
+import org.openstreetmap.josm.gui.util.GuiHelper;
 import org.openstreetmap.josm.plugins.PluginDownloadTask;
 import org.openstreetmap.josm.plugins.PluginHandler;
@@ -175 +175 @@
             }
 
-            Main.parent.repaint();
+            if (Main.parent != null) {
+                Main.parent.repaint();
+            }
         }
     }
@@ -421 +423 @@
                 // by the remaining "save preferences" activites run on the Swing EDT.
                 MainApplication.worker.submit(task);
-                MainApplication.worker.submit(() -> SwingUtilities.invokeLater(continuation));
+                MainApplication.worker.submit(() -> GuiHelper.runInEDT(continuation));
             } else {
                 // no need for asynchronous activities. Simply run the remaining "save preference"

trunk/src/org/openstreetmap/josm/gui/preferences/advanced/ExportProfileAction.java

@@ -56 +56 @@
         }
         if (keys.isEmpty()) {
-            if (!GraphicsEnvironment.isHeadless()) {
-                JOptionPane.showMessageDialog(Main.parent,
-                        tr("All the preferences of this group are default, nothing to save"), tr("Warning"), JOptionPane.WARNING_MESSAGE);
-            }
+            JOptionPane.showMessageDialog(Main.parent,
+                    tr("All the preferences of this group are default, nothing to save"), tr("Warning"), JOptionPane.WARNING_MESSAGE);
             return;
         }

trunk/src/org/openstreetmap/josm/gui/preferences/advanced/PreferencesTable.java

@@ -8 +8 @@
 import java.awt.Component;
 import java.awt.Font;
-import java.awt.GraphicsEnvironment;
 import java.awt.GridBagLayout;
 import java.awt.event.MouseAdapter;
@@ -96 +95 @@
     public boolean editPreference(final JComponent gui) {
         if (getSelectedRowCount() != 1) {
-            if (!GraphicsEnvironment.isHeadless()) {
-                JOptionPane.showMessageDialog(
-                        gui,
-                        tr("Please select the row to edit."),
-                        tr("Warning"),
-                        JOptionPane.WARNING_MESSAGE
-                        );
-            }
+            JOptionPane.showMessageDialog(
+                    gui,
+                    tr("Please select the row to edit."),
+                    tr("Warning"),
+                    JOptionPane.WARNING_MESSAGE
+                    );
             return false;
         }
@@ -197 +194 @@
         PrefEntry pe = null;
         boolean ok = false;
-        if (!GraphicsEnvironment.isHeadless() && askAddSetting(gui, p)) {
+        if (askAddSetting(gui, p)) {
             if (rbString.isSelected()) {
                 StringSetting sSetting = new StringSetting(null);
@@ -282 +279 @@
     public void resetPreferences(final JComponent gui) {
         if (getSelectedRowCount() == 0) {
-            if (!GraphicsEnvironment.isHeadless()) {
-                JOptionPane.showMessageDialog(
-                        gui,
-                        tr("Please select the row to delete."),
-                        tr("Warning"),
-                        JOptionPane.WARNING_MESSAGE
-                        );
-            }
+            JOptionPane.showMessageDialog(
+                    gui,
+                    tr("Please select the row to delete."),
+                    tr("Warning"),
+                    JOptionPane.WARNING_MESSAGE
+                    );
             return;
         }

trunk/src/org/openstreetmap/josm/gui/preferences/plugin/PluginPreference.java

@@ -7 +7 @@
 import java.awt.BorderLayout;
 import java.awt.Component;
-import java.awt.GraphicsEnvironment;
 import java.awt.GridBagConstraints;
 import java.awt.GridBagLayout;
@@ -159 +158 @@
         }
         sb.append("</html>");
-        if (!GraphicsEnvironment.isHeadless()) {
-            GuiHelper.runInEDTAndWait(() -> HelpAwareOptionPane.showOptionDialog(
-                    parent,
-                    sb.toString(),
-                    tr("Update plugins"),
-                    !failed.isEmpty() ? JOptionPane.WARNING_MESSAGE : JOptionPane.INFORMATION_MESSAGE,
-                            HelpUtil.ht("/Preferences/Plugins")
-                    ));
-        }
+        GuiHelper.runInEDTAndWait(() -> HelpAwareOptionPane.showOptionDialog(
+                parent,
+                sb.toString(),
+                tr("Update plugins"),
+                !failed.isEmpty() ? JOptionPane.WARNING_MESSAGE : JOptionPane.INFORMATION_MESSAGE,
+                        HelpUtil.ht("/Preferences/Plugins")
+                ));
     }
 
@@ -212 +209 @@
     }
 
+    private static Component addButton(JPanel pnl, JButton button, String buttonName) {
+        button.setName(buttonName);
+        return pnl.add(button);
+    }
+
     private JPanel buildActionPanel() {
         JPanel pnl = new JPanel(new GridLayout(1, 4));
 
-        pnl.add(new JButton(new DownloadAvailablePluginsAction()));
-        pnl.add(new JButton(new UpdateSelectedPluginsAction()));
-        ExpertToggleAction.addVisibilitySwitcher(pnl.add(new JButton(new SelectByListAction())));
-        ExpertToggleAction.addVisibilitySwitcher(pnl.add(new JButton(new ConfigureSitesAction())));
+        // assign some component names to these as we go to aid testing
+        addButton(pnl, new JButton(new DownloadAvailablePluginsAction()), "downloadListButton");
+        addButton(pnl, new JButton(new UpdateSelectedPluginsAction()), "updatePluginsButton");
+        ExpertToggleAction.addVisibilitySwitcher(addButton(pnl, new JButton(new SelectByListAction()), "loadFromListButton"));
+        ExpertToggleAction.addVisibilitySwitcher(addButton(pnl, new JButton(new ConfigureSitesAction()), "configureSitesButton"));
         return pnl;
     }

trunk/src/org/openstreetmap/josm/gui/progress/swing/ProgressMonitorExecutor.java

@@ -2 +2 @@
 package org.openstreetmap.josm.gui.progress.swing;
 
+import java.util.concurrent.CancellationException;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Future;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.ThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
 
+import org.openstreetmap.josm.tools.Logging;
 import org.openstreetmap.josm.tools.Utils;
 
@@ -37 +41 @@
     }
 
+    @Override
+    public void afterExecute(final Runnable r, Throwable t) {
+        // largely as proposed by JDK8 docs
+        super.afterExecute(r, t);
+        if (t == null && r instanceof Future<?>) {
+            try {
+                ((Future<?>) r).get();
+            } catch (CancellationException cancellationException) {
+                t = cancellationException;
+            } catch (ExecutionException executionException) {
+                t = executionException.getCause();
+            } catch (InterruptedException interruptedException) {
+                Thread.currentThread().interrupt(); // ignore/reset
+            }
+        }
+        if (t != null) {
+            Logging.error("Thread {0} raised {1}", Thread.currentThread().getName(), t);
+        }
+    }
 }

trunk/src/org/openstreetmap/josm/gui/util/GuiHelper.java

@@ -202 +202 @@
     }
 
+    private static void handleEDTException(Throwable t) {
+        Logging.logWithStackTrace(Logging.LEVEL_ERROR, t, "Exception raised in EDT");
+    }
+
     /**
      * Executes synchronously a runnable in
@@ -215 +219 @@
                 SwingUtilities.invokeAndWait(task);
             } catch (InterruptedException | InvocationTargetException e) {
-                Logging.error(e);
+                handleEDTException(e);
             }
         }
@@ -256 +260 @@
                 return callable.call();
             } catch (Exception e) { // NOPMD
-                Logging.error(e);
+                handleEDTException(e);
                 return null;
             }
@@ -265 +269 @@
                 return task.get();
             } catch (InterruptedException | ExecutionException e) {
-                Logging.error(e);
+                handleEDTException(e);
                 return null;
             }

trunk/src/org/openstreetmap/josm/tools/Logging.java

@@ -51 +51 @@
     private static final RememberWarningHandler WARNINGS = new RememberWarningHandler();
 
+    /**
+     * A {@link ConsoleHandler} with a couple of extra features, allowing it to be targeted at an
+     * an arbitrary {@link OutputStream} which it can be asked to reaquire the reference for on demand
+     * through {@link #reacquireOutputStream()}. It can also prevent a LogRecord's output if a
+     * specified {@code prioritizedHandler} would have outputted it.
+     * @since 14052
+     */
+    public static class ReacquiringConsoleHandler extends ConsoleHandler {
+        private final Supplier<OutputStream> outputStreamSupplier;
+        private final Handler prioritizedHandler;
+        private OutputStream outputStreamMemo;
+
+        /**
+        * Construct a new {@link ReacquiringConsoleHandler}.
+        * @param outputStreamSupplier A {@link Supplier} which will return the desired
+        *   {@link OutputStream} for this handler when called. Particularly useful if you happen to be
+        *   using a test framework which will switch out references to the stderr/stdout streams with
+        *   new dummy ones from time to time.
+        * @param prioritizedHandler If non-null, will suppress output of any log records which pass this
+        *   handler's {@code Handler#isLoggable(LogRecord)} method.
+        */
+        public ReacquiringConsoleHandler(
+            final Supplier<OutputStream> outputStreamSupplier,
+            final Handler prioritizedHandler
+        ) {
+            this.outputStreamSupplier = outputStreamSupplier;
+            this.prioritizedHandler = prioritizedHandler;
+
+            this.reacquireOutputStream();
+        }
+
+        /**
+         * Set output stream to one acquired from calling outputStreamSupplier
+         */
+        public void reacquireOutputStream() {
+            final OutputStream reacquiredStream = this.outputStreamSupplier.get();
+
+            // only bother calling setOutputStream if it's actually different, as setOutputStream
+            // has the nasty side effect of closing any previous output stream, which is certainly not
+            // what we would want were the new stream the same one
+            if (reacquiredStream != this.outputStreamMemo) {
+                this.setOutputStream(reacquiredStream);
+            }
+        }
+
+        @Override
+        public synchronized void setOutputStream(final OutputStream outputStream) {
+            // this wouldn't be necessary if StreamHandler made it possible to see what the current
+            // output stream is set to
+            this.outputStreamMemo = outputStream;
+            super.setOutputStream(outputStream);
+        }
+
+        @Override
+        public synchronized void publish(LogRecord record) {
+            if (this.prioritizedHandler == null || !this.prioritizedHandler.isLoggable(record)) {
+                super.publish(record);
+            }
+        }
+    }
+
     static {
         // We need to be sure java.locale.providers system property is initialized by JOSM, not by JRE
@@ -62 +123 @@
         Utils.updateSystemProperty("java.util.logging.SimpleFormatter.format", "%1$tF %1$tT.%1$tL %4$s: %5$s%6$s%n");
 
-        ConsoleHandler stderr = new ConsoleHandler();
+        ConsoleHandler stderr = new ReacquiringConsoleHandler(() -> System.err, null);
         LOGGER.addHandler(stderr);
         try {
@@ -70 +131 @@
         }
 
-        ConsoleHandler stdout = new ConsoleHandler() {
-            @Override
-            protected synchronized void setOutputStream(OutputStream out) {
-                // overwrite output stream.
-                super.setOutputStream(System.out);
-            }
-
-            @Override
-            public synchronized void publish(LogRecord record) {
-                if (!stderr.isLoggable(record)) {
-                    super.publish(record);
-                }
-            }
-        };
+        ConsoleHandler stdout = new ReacquiringConsoleHandler(() -> System.out, stderr);
         LOGGER.addHandler(stdout);
         try {