Changeset 11986 in josm

Timestamp:

2017-04-23T19:14:12+02:00 (7 years ago)

Author:

Don-vip

Message:

add unit test for Main.postConstructorProcessCmdLine

Location:

trunk

Files:

• src/org/openstreetmap/josm/Main.java (modified) (10 diffs)
• src/org/openstreetmap/josm/actions/OpenFileAction.java (modified) (3 diffs)
• src/org/openstreetmap/josm/actions/OpenLocationAction.java (modified) (4 diffs)
• src/org/openstreetmap/josm/actions/downloadtasks/DownloadOsmTask.java (modified) (1 diff)
• src/org/openstreetmap/josm/actions/search/SearchAction.java (modified) (2 diffs)
• src/org/openstreetmap/josm/gui/ProgramArguments.java (modified) (1 diff)
• src/org/openstreetmap/josm/gui/datatransfer/importers/OsmLinkPaster.java (modified) (1 diff)
• test/unit/org/openstreetmap/josm/MainTest.java (modified) (5 diffs)

trunk/src/org/openstreetmap/josm/Main.java

@@ -18 +18 @@
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.EnumSet;
 import java.util.HashMap;
@@ -826 +827 @@
     }
 
+    /**
+     * Handle command line instructions after GUI has been initialized.
+     * @param args program arguments
+     */
     protected static void postConstructorProcessCmdLine(ProgramArguments args) {
+        List<Future<?>> tasks = new ArrayList<>();
         List<File> fileList = new ArrayList<>();
         for (String s : args.get(Option.DOWNLOAD)) {
-            DownloadParamType.paramType(s).download(s, fileList);
+            tasks.addAll(DownloadParamType.paramType(s).download(s, fileList));
         }
         if (!fileList.isEmpty()) {
-            OpenFileAction.openFiles(fileList, true);
+            tasks.add(OpenFileAction.openFiles(fileList, true));
         }
         for (String s : args.get(Option.DOWNLOADGPS)) {
-            DownloadParamType.paramType(s).downloadGps(s);
+            tasks.addAll(DownloadParamType.paramType(s).downloadGps(s));
+        }
+        // Make sure all download tasks complete before handling selection arguments
+        for (Future<?> task : tasks) {
+            try {
+                task.get();
+            } catch (InterruptedException | ExecutionException e) {
+                error(e);
+            }
         }
         for (String s : args.get(Option.SELECTION)) {
@@ -868 +882 @@
     }
 
+    /**
+     * Shutdown JOSM.
+     */
     protected void shutdown() {
         if (!GraphicsEnvironment.isHeadless()) {
@@ -897 +914 @@
         httpUrl {
             @Override
-            void download(String s, Collection<File> fileList) {
-                new OpenLocationAction().openUrl(false, s);
+            List<Future<?>> download(String s, Collection<File> fileList) {
+                return new OpenLocationAction().openUrl(false, s);
             }
 
             @Override
-            void downloadGps(String s) {
+            List<Future<?>> downloadGps(String s) {
                 final Bounds b = OsmUrlToBounds.parse(s);
                 if (b == null) {
@@ -911 +928 @@
                             JOptionPane.WARNING_MESSAGE
                     );
-                    return;
+                    return Collections.emptyList();
                 }
-                downloadFromParamBounds(true, b);
+                return downloadFromParamBounds(true, b);
             }
         }, fileUrl {
             @Override
-            void download(String s, Collection<File> fileList) {
+            List<Future<?>> download(String s, Collection<File> fileList) {
                 File f = null;
                 try {
@@ -933 +950 @@
                     fileList.add(f);
                 }
+                return Collections.emptyList();
             }
         }, bounds {
@@ -940 +958 @@
              * @param rawGps Flag to download raw GPS tracks
              * @param s The bounds parameter
+             * @return the complete download task (including post-download handler), or {@code null}
              */
-            private void downloadFromParamBounds(final boolean rawGps, String s) {
+            private List<Future<?>> downloadFromParamBounds(final boolean rawGps, String s) {
                 final StringTokenizer st = new StringTokenizer(s, ",");
                 if (st.countTokens() == 4) {
-                    Bounds b = new Bounds(
+                    return Main.downloadFromParamBounds(rawGps, new Bounds(
                             new LatLon(Double.parseDouble(st.nextToken()), Double.parseDouble(st.nextToken())),
                             new LatLon(Double.parseDouble(st.nextToken()), Double.parseDouble(st.nextToken()))
-                    );
-                    Main.downloadFromParamBounds(rawGps, b);
+                    ));
                 }
+                return Collections.emptyList();
             }
 
             @Override
-            void download(String param, Collection<File> fileList) {
-                downloadFromParamBounds(false, param);
+            List<Future<?>> download(String param, Collection<File> fileList) {
+                return downloadFromParamBounds(false, param);
             }
 
             @Override
-            void downloadGps(String param) {
-                downloadFromParamBounds(true, param);
+            List<Future<?>> downloadGps(String param) {
+                return downloadFromParamBounds(true, param);
             }
         }, fileName {
             @Override
-            void download(String s, Collection<File> fileList) {
+            List<Future<?>> download(String s, Collection<File> fileList) {
                 fileList.add(new File(s));
+                return Collections.emptyList();
             }
         };
@@ -972 +992 @@
          * @param param represents the object to be downloaded
          * @param fileList files which shall be opened, should be added to this collection
+         * @return the download task, or {@code null}
          */
-        abstract void download(String param, Collection<File> fileList);
+        abstract List<Future<?>> download(String param, Collection<File> fileList);
 
         /**
          * Performs the GPS download
          * @param param represents the object to be downloaded
+         * @return the download task, or {@code null}
          */
-        void downloadGps(String param) {
+        List<Future<?>> downloadGps(String param) {
             JOptionPane.showMessageDialog(
                     Main.parent,
@@ -986 +1008 @@
                     JOptionPane.WARNING_MESSAGE
             );
+            return Collections.emptyList();
         }
 
@@ -1008 +1031 @@
      * @param rawGps Flag to download raw GPS tracks
      * @param b The bounds value
-     */
-    private static void downloadFromParamBounds(final boolean rawGps, Bounds b) {
+     * @return the complete download task (including post-download handler)
+     */
+    private static List<Future<?>> downloadFromParamBounds(final boolean rawGps, Bounds b) {
         DownloadTask task = rawGps ? new DownloadGpsTask() : new DownloadOsmTask();
         // asynchronously launch the download task ...
         Future<?> future = task.download(true, b, null);
         // ... and the continuation when the download is finished (this will wait for the download to finish)
-        Main.worker.execute(new PostDownloadHandler(task, future));
+        return Collections.singletonList(Main.worker.submit(new PostDownloadHandler(task, future)));
     }
 

trunk/src/org/openstreetmap/josm/actions/OpenFileAction.java

@@ -22 +22 @@
 import java.util.List;
 import java.util.Set;
+import java.util.concurrent.Future;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -84 +85 @@
      * Filenames will not be saved in history.
      * @param fileList A list of files
-     */
-    public static void openFiles(List<File> fileList) {
-        openFiles(fileList, false);
+     * @return the future task
+     * @since 11986 (return task)
+     */
+    public static Future<?> openFiles(List<File> fileList) {
+        return openFiles(fileList, false);
     }
 
@@ -93 +96 @@
      * @param fileList A list of files
      * @param recordHistory {@code true} to save filename in history (default: false)
-     */
-    public static void openFiles(List<File> fileList, boolean recordHistory) {
+     * @return the future task
+     * @since 11986 (return task)
+     */
+    public static Future<?> openFiles(List<File> fileList, boolean recordHistory) {
         OpenFileTask task = new OpenFileTask(fileList, null);
         task.setRecordHistory(recordHistory);
-        Main.worker.submit(task);
+        return Main.worker.submit(task);
     }
 

trunk/src/org/openstreetmap/josm/actions/OpenLocationAction.java

@@ -57 +57 @@
      */
     private static final BooleanProperty USE_NEW_LAYER = new BooleanProperty("download.newlayer", false);
+    /**
+     * the list of download tasks
+     */
     protected final transient List<Class<? extends DownloadTask>> downloadTasks;
 
@@ -200 +203 @@
      * @param newLayer true if the URL needs to be opened in a new layer, false otherwise
      * @param url The URL to open
-     */
-    public void openUrl(boolean newLayer, String url) {
-        realOpenUrl(newLayer, url);
+     * @return the list of tasks that have been started successfully (can be empty).
+     * @since 11986 (return type)
+     */
+    public List<Future<?>> openUrl(boolean newLayer, String url) {
+        return realOpenUrl(newLayer, url);
     }
 
@@ -208 +213 @@
      * Open the given URL. This class checks the {@link #USE_NEW_LAYER} preference to check if a new layer should be used.
      * @param url The URL to open
-     * @return <code>true</code> if at least one task was started successfully.
-     * @since 11279
-     */
-    public boolean openUrl(String url) {
+     * @return the list of tasks that have been started successfully (can be empty).
+     * @since 11986 (return type)
+     */
+    public List<Future<?>> openUrl(String url) {
         return realOpenUrl(USE_NEW_LAYER.get(), url);
     }
 
-    private boolean realOpenUrl(boolean newLayer, String url) {
+    private List<Future<?>> realOpenUrl(boolean newLayer, String url) {
         Collection<DownloadTask> tasks = findDownloadTasks(url, false);
 
@@ -222 +227 @@
         } else if (tasks.isEmpty()) {
             warnNoSuitableTasks(url);
-            return false;
+            return Collections.emptyList();
         }
 
         PleaseWaitProgressMonitor monitor = new PleaseWaitProgressMonitor(tr("Download Data"));
 
-        boolean hadAnySuccess = false;
+        List<Future<?>> result = new ArrayList<>();
         for (final DownloadTask task : tasks) {
             try {
-                Future<?> future = task.loadUrl(newLayer, url, monitor);
-                Main.worker.submit(new PostDownloadHandler(task, future));
-                hadAnySuccess = true;
+                result.add(Main.worker.submit(new PostDownloadHandler(task, task.loadUrl(newLayer, url, monitor))));
             } catch (IllegalArgumentException e) {
                 Main.error(e);
             }
         }
-        return hadAnySuccess;
+        return result;
     }
 

trunk/src/org/openstreetmap/josm/actions/downloadtasks/DownloadOsmTask.java

@@ -267 +267 @@
                 //
                 final OsmDataLayer layer = createNewLayer(newLayerName);
-                if (Main.main != null)
-                    Main.getLayerManager().addLayer(layer, zoomAfterDownload);
+                Main.getLayerManager().addLayer(layer, zoomAfterDownload);
                 return layer;
             }

trunk/src/org/openstreetmap/josm/actions/search/SearchAction.java

@@ -10 +10 @@
 import java.awt.Dimension;
 import java.awt.FlowLayout;
+import java.awt.GraphicsEnvironment;
 import java.awt.GridBagLayout;
 import java.awt.event.ActionEvent;
@@ -596 +597 @@
                     msg = null;
                 }
-                Main.map.statusLine.setHelpText(msg);
-                JOptionPane.showMessageDialog(
-                        Main.parent,
-                        msg,
-                        tr("Warning"),
-                        JOptionPane.WARNING_MESSAGE
-                );
-            } else {
+                if (Main.map != null) {
+                    Main.map.statusLine.setHelpText(msg);
+                }
+                if (!GraphicsEnvironment.isHeadless()) {
+                    JOptionPane.showMessageDialog(
+                            Main.parent,
+                            msg,
+                            tr("Warning"),
+                            JOptionPane.WARNING_MESSAGE
+                    );
+                }
+            } else if (Main.map != null) {
                 Main.map.statusLine.setHelpText(tr("Found {0} matches", foundMatches));
             }

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

@@ -136 +136 @@
                 addOption(opt, g.getOptarg());
             } else
-                throw new IllegalArgumentException("Invalid option: "+c);
+                throw new IllegalArgumentException("Invalid option: "+ (char) c);
         }
         // positional arguments are a shortcut for the --download ... option

trunk/src/org/openstreetmap/josm/gui/datatransfer/importers/OsmLinkPaster.java

@@ -51 +51 @@
 
         String transferData = (String) support.getTransferable().getTransferData(df);
-        if (new NoWarnOpenLocationAction().openUrl(transferData)) {
+        if (!new NoWarnOpenLocationAction().openUrl(transferData).isEmpty()) {
             return true;
         }

trunk/test/unit/org/openstreetmap/josm/MainTest.java

@@ -3 +3 @@
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
@@ -8 +9 @@
 
 import java.util.Collection;
+import java.util.List;
 
 import javax.swing.UIManager;
@@ -14 +16 @@
 import org.junit.Test;
 import org.openstreetmap.josm.Main.DownloadParamType;
+import org.openstreetmap.josm.data.osm.DataSet;
+import org.openstreetmap.josm.gui.ProgramArguments;
+import org.openstreetmap.josm.gui.layer.GpxLayer;
 import org.openstreetmap.josm.testutils.JOSMTestRules;
 
@@ -28 +33 @@
     @Rule
     @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD")
-    public JOSMTestRules test = new JOSMTestRules().platform();
+    public JOSMTestRules test = new JOSMTestRules().platform().devAPI();
 
     /**
@@ -81 +86 @@
         assertNotNull(Main.toolbar);
     }
+
+    /**
+     * Unit test of {@link Main#postConstructorProcessCmdLine} - empty case.
+     */
+    @Test
+    public void testPostConstructorProcessCmdLineEmpty() {
+        // Check the method accepts no arguments
+        Main.postConstructorProcessCmdLine(new ProgramArguments(new String[0]));
+    }
+
+    /**
+     * Unit test of {@link Main#postConstructorProcessCmdLine} - nominal cases.
+     * This test assumes the DEV API contains nodes around 0,0 and GPX tracks around London
+     */
+    @Test
+    public void testPostConstructorProcessCmdLineNominal() {
+        assertNull(Main.getLayerManager().getEditDataSet());
+        Main.postConstructorProcessCmdLine(new ProgramArguments(new String[]{
+                "--download=0.01,0.01,0.05,0.05",
+                "--downloadgps=51.35,-0.4,51.60,0.2",
+                "--selection=type: node"}));
+        DataSet ds = Main.getLayerManager().getEditDataSet();
+        assertNotNull(ds);
+        assertFalse(ds.getSelected().isEmpty());
+        Main.getLayerManager().removeLayer(Main.getLayerManager().getEditLayer());
+        List<GpxLayer> gpxLayers = Main.getLayerManager().getLayersOfType(GpxLayer.class);
+        assertEquals(1, gpxLayers.size());
+        Main.getLayerManager().removeLayer(gpxLayers.iterator().next());
+    }
+
+    /**
+     * Unit test of {@link DownloadParamType} enum.
+     */
+    @Test
+    public void testEnumDownloadParamType() {
+        TestUtils.superficialEnumCodeCoverage(DownloadParamType.class);
+    }
 }