8153716: JShell tool: should warn when failed to launch editor
authorrfield
Sat, 09 Apr 2016 11:03:48 -0700
changeset 37007 6023a9a9d58a
parent 37006 73ca1e844343
child 37008 55c73d04e57c
8153716: JShell tool: should warn when failed to launch editor Summary: Catch launch exceptions. Split ToolBasicTest into two to provide place for regression test. Reviewed-by: rfield Contributed-by: kubota.yuji@gmail.com
langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellTool.java
langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/resources/l10n.properties
langtools/test/jdk/jshell/ReplToolTesting.java
langtools/test/jdk/jshell/ToolBasicTest.java
langtools/test/jdk/jshell/ToolSimpleTest.java
--- a/langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellTool.java	Fri Apr 08 13:26:38 2016 -0700
+++ b/langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellTool.java	Sat Apr 09 11:03:48 2016 -0700
@@ -1402,7 +1402,13 @@
         Consumer<String> saveHandler = new SaveHandler(src, srcSet);
         Consumer<String> errorHandler = s -> hard("Edit Error: %s", s);
         if (editor == null) {
-            EditPad.edit(errorHandler, src, saveHandler);
+            try {
+                EditPad.edit(errorHandler, src, saveHandler);
+            } catch (RuntimeException ex) {
+                errormsg("jshell.err.cant.launch.editor", ex);
+                fluffmsg("jshell.msg.try.set.editor");
+                return false;
+            }
         } else {
             ExternalEditor.edit(editor, errorHandler, src, saveHandler, input);
         }
--- a/langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/resources/l10n.properties	Fri Apr 08 13:26:38 2016 -0700
+++ b/langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/resources/l10n.properties	Sat Apr 09 11:03:48 2016 -0700
@@ -53,6 +53,8 @@
 jshell.err.command.ambiguous = Command: ''{0}'' is ambiguous: {1}
 jshell.err.set.editor.arg = The ''/set editor'' command requires a path argument
 jshell.msg.set.editor.set = Editor set to: {0}
+jshell.err.cant.launch.editor = Cannot launch editor -- unexpected exception: {0}
+jshell.msg.try.set.editor = Try /set editor to use external editor.
 
 jshell.msg.try.list.without.args = Try ''/list'' without arguments.
 jshell.msg.no.active = There are no active definitions.
--- a/langtools/test/jdk/jshell/ReplToolTesting.java	Fri Apr 08 13:26:38 2016 -0700
+++ b/langtools/test/jdk/jshell/ReplToolTesting.java	Sat Apr 09 11:03:48 2016 -0700
@@ -438,7 +438,7 @@
             }
             setCommandInput(cmd + "\n");
         } else {
-            assertOutput(getCommandOutput(), out, "command output: " + cmd);
+            assertOutput(getCommandOutput().trim(), out==null? out : out.trim(), "command output: " + cmd);
             assertOutput(getCommandErrorOutput(), err, "command error: " + cmd);
             assertOutput(getUserOutput(), print, "user output: " + cmd);
             assertOutput(getUserErrorOutput(), usererr, "user error: " + cmd);
--- a/langtools/test/jdk/jshell/ToolBasicTest.java	Fri Apr 08 13:26:38 2016 -0700
+++ b/langtools/test/jdk/jshell/ToolBasicTest.java	Sat Apr 09 11:03:48 2016 -0700
@@ -62,34 +62,6 @@
 @Test
 public class ToolBasicTest extends ReplToolTesting {
 
-    public void defineVar() {
-        test(
-                (a) -> assertCommand(a, "int x = 72", "|  Added variable x of type int with initial value 72\n"),
-                (a) -> assertCommand(a, "x", "|  Variable x of type int has value 72\n"),
-                (a) -> assertCommand(a, "/vars", "|    int x = 72\n")
-        );
-    }
-
-    public void defineUnresolvedVar() {
-        test(
-                (a) -> assertCommand(a, "undefined x",
-                        "|  Added variable x, however, it cannot be referenced until class undefined is declared\n"),
-                (a) -> assertCommand(a, "/vars", "|    undefined x = (not-active)\n")
-        );
-    }
-
-    public void testUnresolved() {
-        test(
-                (a) -> assertCommand(a, "int f() { return g() + x + new A().a; }",
-                        "|  Added method f(), however, it cannot be invoked until method g(), variable x, and class A are declared\n"),
-                (a) -> assertCommand(a, "f()",
-                        "|  Attempted to call f which cannot be invoked until method g(), variable x, and class A are declared\n"),
-                (a) -> assertCommand(a, "int g() { return x; }",
-                        "|  Added method g(), however, it cannot be invoked until variable x is declared\n"),
-                (a) -> assertCommand(a, "g()", "|  Attempted to call g which cannot be invoked until variable x is declared\n")
-        );
-    }
-
     public void elideStartUpFromList() {
         test(
                 (a) -> assertCommandOutputContains(a, "123", "type int"),
@@ -297,47 +269,6 @@
         );
     }
 
-    public void testDebug() {
-        test(
-                (a) -> assertCommand(a, "/deb", "|  Debugging on\n"),
-                (a) -> assertCommand(a, "/debug", "|  Debugging off\n"),
-                (a) -> assertCommand(a, "/debug", "|  Debugging on\n"),
-                (a) -> assertCommand(a, "/deb", "|  Debugging off\n")
-        );
-    }
-
-    public void testHelpLength() {
-        Consumer<String> testOutput = (s) -> {
-            List<String> ss = Stream.of(s.split("\n"))
-                    .filter(l -> !l.isEmpty())
-                    .collect(Collectors.toList());
-            assertTrue(ss.size() >= 10, "Help does not print enough lines:\n" + s);
-        };
-        test(
-                (a) -> assertCommandCheckOutput(a, "/?", testOutput),
-                (a) -> assertCommandCheckOutput(a, "/help", testOutput),
-                (a) -> assertCommandCheckOutput(a, "/help /list", testOutput)
-        );
-    }
-
-    public void testHelp() {
-        test(
-                (a) -> assertHelp(a, "/?", "/list", "/help", "/exit", "intro"),
-                (a) -> assertHelp(a, "/help", "/list", "/help", "/exit", "intro"),
-                (a) -> assertHelp(a, "/help short", "shortcuts", "<tab>"),
-                (a) -> assertHelp(a, "/? /li", "/list all", "snippets"),
-                (a) -> assertHelp(a, "/help /help", "/help <command>")
-        );
-    }
-
-    private void assertHelp(boolean a, String command, String... find) {
-        assertCommandCheckOutput(a, command, s -> {
-            for (String f : find) {
-                assertTrue(s.contains(f), "Expected output of " + command + " to contain: " + f);
-            }
-        });
-    }
-
     public void oneLineOfError() {
         test(
                 (a) -> assertCommand(a, "12+", null),
@@ -678,39 +609,6 @@
         );
     }
 
-    // Check that each line of output contains the corresponding string from the list
-    private void checkLineToList(String in, List<String> match) {
-        String[] res = in.trim().split("\n");
-        assertEquals(res.length, match.size(), "Got: " + Arrays.asList(res));
-        for (int i = 0; i < match.size(); ++i) {
-            assertTrue(res[i].contains(match.get(i)));
-        }
-    }
-
-    public void testListArgs() {
-        String arg = "qqqq";
-        List<String> startVarList = new ArrayList<>(START_UP);
-        startVarList.add("int aardvark");
-        test(
-                a -> assertCommandCheckOutput(a, "/list all",
-                        s -> checkLineToList(s, START_UP)),
-                a -> assertCommandCheckOutput(a, "/list " + arg,
-                        s -> assertEquals(s, "|  No definition or id named " + arg +
-                                " found.  There are no active definitions.\n")),
-                a -> assertVariable(a, "int", "aardvark"),
-                a -> assertCommandOutputContains(a, "/list aardvark", "aardvark"),
-                a -> assertCommandCheckOutput(a, "/list start",
-                        s -> checkLineToList(s, START_UP)),
-                a -> assertCommandCheckOutput(a, "/list all",
-                        s -> checkLineToList(s, startVarList)),
-                a -> assertCommandCheckOutput(a, "/list printf",
-                        s -> assertTrue(s.contains("void printf"))),
-                a -> assertCommandCheckOutput(a, "/list " + arg,
-                        s -> assertEquals(s, "|  No definition or id named " + arg +
-                                " found.  Try /list without arguments.\n"))
-        );
-    }
-
     public void testFeedbackNegative() {
         test(a -> assertCommandCheckOutput(a, "/set feedback aaaa",
                 assertStartsWith("|  Does not match any current feedback mode")));
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/jdk/jshell/ToolSimpleTest.java	Sat Apr 09 11:03:48 2016 -0700
@@ -0,0 +1,162 @@
+/*
+ * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8153716
+ * @summary Simple jshell tool tests
+ * @modules jdk.compiler/com.sun.tools.javac.api
+ *          jdk.compiler/com.sun.tools.javac.main
+ *          jdk.jdeps/com.sun.tools.javap
+ *          jdk.jshell/jdk.internal.jshell.tool
+ * @build KullaTesting TestingInputStream
+ * @run testng ToolSimpleTest
+ */
+import java.util.Arrays;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.function.Consumer;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertTrue;
+
+@Test
+public class ToolSimpleTest extends ReplToolTesting {
+
+    public void defineVar() {
+        test(
+                (a) -> assertCommand(a, "int x = 72", "|  Added variable x of type int with initial value 72"),
+                (a) -> assertCommand(a, "x", "|  Variable x of type int has value 72"),
+                (a) -> assertCommand(a, "/vars", "|    int x = 72")
+        );
+    }
+
+    @Test(enabled = false) // TODO 8153897
+    public void defineUnresolvedVar() {
+        test(
+                (a) -> assertCommand(a, "undefined x",
+                        "|  Added variable x, however, it cannot be referenced until class undefined is declared"),
+                (a) -> assertCommand(a, "/vars", "|    undefined x = (not-active)")
+        );
+    }
+
+    public void testUnresolved() {
+        test(
+                (a) -> assertCommand(a, "int f() { return g() + x + new A().a; }",
+                        "|  Added method f(), however, it cannot be invoked until method g(), variable x, and class A are declared"),
+                (a) -> assertCommand(a, "f()",
+                        "|  Attempted to call method f() which cannot be invoked until method g(), variable x, and class A are declared"),
+                (a) -> assertCommandOutputStartsWith(a, "int g() { return x; }",
+                        "|  Added method g(), however, it cannot be invoked until variable x is declared"),
+                (a) -> assertCommand(a, "g()", "|  Attempted to call method g() which cannot be invoked until variable x is declared")
+        );
+    }
+
+    public void testDebug() {
+        test(
+                (a) -> assertCommand(a, "/deb", "|  Debugging on"),
+                (a) -> assertCommand(a, "/debug", "|  Debugging off"),
+                (a) -> assertCommand(a, "/debug", "|  Debugging on"),
+                (a) -> assertCommand(a, "/deb", "|  Debugging off")
+        );
+    }
+
+    public void testHelpLength() {
+        Consumer<String> testOutput = (s) -> {
+            List<String> ss = Stream.of(s.split("\n"))
+                    .filter(l -> !l.isEmpty())
+                    .collect(Collectors.toList());
+            assertTrue(ss.size() >= 10, "Help does not print enough lines:" + s);
+        };
+        test(
+                (a) -> assertCommandCheckOutput(a, "/?", testOutput),
+                (a) -> assertCommandCheckOutput(a, "/help", testOutput),
+                (a) -> assertCommandCheckOutput(a, "/help /list", testOutput)
+        );
+    }
+
+    public void testHelp() {
+        test(
+                (a) -> assertHelp(a, "/?", "/list", "/help", "/exit", "intro"),
+                (a) -> assertHelp(a, "/help", "/list", "/help", "/exit", "intro"),
+                (a) -> assertHelp(a, "/help short", "shortcuts", "<tab>"),
+                (a) -> assertHelp(a, "/? /li", "/list all", "snippets"),
+                (a) -> assertHelp(a, "/help /help", "/help <command>")
+        );
+    }
+
+    private void assertHelp(boolean a, String command, String... find) {
+        assertCommandCheckOutput(a, command, s -> {
+            for (String f : find) {
+                assertTrue(s.contains(f), "Expected output of " + command + " to contain: " + f);
+            }
+        });
+    }
+
+    // Check that each line of output contains the corresponding string from the list
+    private void checkLineToList(String in, List<String> match) {
+        String[] res = in.trim().split("\n");
+        assertEquals(res.length, match.size(), "Got: " + Arrays.asList(res));
+        for (int i = 0; i < match.size(); ++i) {
+            assertTrue(res[i].contains(match.get(i)));
+        }
+    }
+
+    public void testListArgs() {
+        String arg = "qqqq";
+        List<String> startVarList = new ArrayList<>(START_UP);
+        startVarList.add("int aardvark");
+        test(
+                a -> assertCommandCheckOutput(a, "/list all",
+                        s -> checkLineToList(s, START_UP)),
+                a -> assertCommandOutputStartsWith(a, "/list " + arg,
+                        "|  No definition or id found named: " + arg),
+                a -> assertVariable(a, "int", "aardvark"),
+                a -> assertCommandOutputContains(a, "/list aardvark", "aardvark"),
+                a -> assertCommandCheckOutput(a, "/list start",
+                        s -> checkLineToList(s, START_UP)),
+                a -> assertCommandCheckOutput(a, "/list all",
+                        s -> checkLineToList(s, startVarList)),
+                a -> assertCommandCheckOutput(a, "/list printf",
+                        s -> assertTrue(s.contains("void printf"))),
+                a -> assertCommandOutputStartsWith(a, "/list " + arg,
+                        "|  No definition or id found named: " + arg)
+        );
+    }
+
+    public void testHeadlessEditPad() {
+        String prevHeadless = System.getProperty("java.awt.headless");
+        try {
+            System.setProperty("java.awt.headless", "true");
+            test(
+                (a) -> assertCommandOutputStartsWith(a, "/edit printf", "|  Cannot launch editor -- unexpected exception:")
+            );
+        } finally {
+            System.setProperty("java.awt.headless", prevHeadless==null? "false" : prevHeadless);
+        }
+    }
+}