changeset 270:ca0636885cae

Improve error message on missing option. Reviewed-by: neugens Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2012-May/001177.html PR 969
author Roman Kennke <rkennke@redhat.com>
date Wed, 02 May 2012 16:53:03 +0200
parents f6f3c1cc9332
children c090011cb55e
files common/src/main/java/com/redhat/thermostat/cli/CommandLineArgumentsParser.java common/src/test/java/com/redhat/thermostat/cli/CommandLineArgumentsParserTest.java common/src/test/java/com/redhat/thermostat/cli/LauncherTest.java
diffstat 3 files changed, 50 insertions(+), 3 deletions(-) [+]
line wrap: on
line diff
--- a/common/src/main/java/com/redhat/thermostat/cli/CommandLineArgumentsParser.java	Wed May 02 16:51:42 2012 +0200
+++ b/common/src/main/java/com/redhat/thermostat/cli/CommandLineArgumentsParser.java	Wed May 02 16:53:03 2012 +0200
@@ -38,6 +38,7 @@
 
 import java.io.PrintWriter;
 import java.util.Collection;
+import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
 
@@ -45,6 +46,7 @@
 import org.apache.commons.cli.CommandLineParser;
 import org.apache.commons.cli.GnuParser;
 import org.apache.commons.cli.HelpFormatter;
+import org.apache.commons.cli.MissingOptionException;
 import org.apache.commons.cli.Option;
 import org.apache.commons.cli.Options;
 import org.apache.commons.cli.ParseException;
@@ -64,11 +66,33 @@
             CommandLine commandLine;
             commandLine = parser.parse(options, args);
             return new CommandLineArguments(commandLine);
+        } catch (MissingOptionException mae) {
+            String msg = createMissingOptionsMessage(mae);
+            throw new CommandLineArgumentParseException(msg.toString(), mae);
         } catch (ParseException e) {
             throw new CommandLineArgumentParseException(e.getMessage(), e);
         }
     }
 
+    private String createMissingOptionsMessage(MissingOptionException mae) {
+        @SuppressWarnings("unchecked")
+        List<String> missingOptions = mae.getMissingOptions();
+        StringBuilder msg = new StringBuilder();
+        if (missingOptions.size() == 1) {
+            msg.append("Missing required option: ");
+        } else {
+            msg.append("Missing required options: ");
+        }
+        for (Iterator<String> i = missingOptions.iterator(); i.hasNext();) {
+            String missingOption = i.next();
+            msg.append("--");
+            msg.append(missingOption);
+            if (i.hasNext()) {
+                msg.append(", ");
+            }
+        }
+        return msg.toString();
+    }
 
     private Options convertToCommonsCLIOptions(Collection<ArgumentSpec> args) {
         Options options = new Options();
--- a/common/src/test/java/com/redhat/thermostat/cli/CommandLineArgumentsParserTest.java	Wed May 02 16:51:42 2012 +0200
+++ b/common/src/test/java/com/redhat/thermostat/cli/CommandLineArgumentsParserTest.java	Wed May 02 16:53:03 2012 +0200
@@ -38,6 +38,7 @@
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -94,9 +95,31 @@
         parser.parse(new String[] { "--test1", "--no-match" });
     }
 
-    @Test(expected=CommandLineArgumentParseException.class)
+    @Test
     public void testMissingRequiredArgument() throws CommandLineArgumentParseException {
-        parser.parse(new String[] { "--test2" });
+        try {
+          parser.parse(new String[] { "--test2" });
+          fail();
+        } catch (CommandLineArgumentParseException ex) {
+            String msg = ex.getMessage();
+            assertEquals("Missing required option: --test1", msg);
+        }
+    }
+
+    @Test
+    public void testMissingRequiredArguments() throws CommandLineArgumentParseException {
+        ArgumentSpec arg4 = mock(ArgumentSpec.class);
+        when(arg4.getName()).thenReturn("test4");
+        when(arg4.isRequired()).thenReturn(true);
+        parser.addArguments(Arrays.asList(arg4));
+
+        try {
+          parser.parse(new String[] { "--test2" });
+          fail();
+        } catch (CommandLineArgumentParseException ex) {
+            String msg = ex.getMessage();
+            assertEquals("Missing required options: --test1, --test4", msg);
+        }
     }
 
     @Test
--- a/common/src/test/java/com/redhat/thermostat/cli/LauncherTest.java	Wed May 02 16:51:42 2012 +0200
+++ b/common/src/test/java/com/redhat/thermostat/cli/LauncherTest.java	Wed May 02 16:53:03 2012 +0200
@@ -220,6 +220,6 @@
     @Test
     public void verifyStorageCommandRequiresDbUrl() {
         new Launcher().run(new String[] { "test3" });
-        assertEquals("Missing required option: dbUrl\n", ctxFactory.getError());
+        assertEquals("Missing required option: --dbUrl\n", ctxFactory.getError());
     }
 }