Re: RFR: JDK-8209604: [TEST] rewrite com/sun/jdi shell tests to java version - step2

[serguei.spit...@oracle.com]

Hi Alex, It looks good in general. Some minor comments below. http://cr.openjdk.java.net/~amenkov/sh2java/step2/webrev/test/jdk/com/sun/jdi/lib/jdb/JdbCommand.java.frames.html 132 private static final String ls = System.getProperty("line.separator"); Minor: Replace variable name "ls" with "LINE_SEP" (in upper case as it is a static final). http://cr.openjdk.java.net/~amenkov/sh2java/step2/webrev/test/jdk/com/sun/jdi/EvalArgs.java.frames.html   Just wanted to double-check that this check:     262 jdbFailIfPresent "Arguments match no method"   works in each call of the verifyNoArgumentsMatchMethod()   instead of just once as it was originally. 214 private void verifyNoArgumentsMatchMethod(String expr) { 215 List<String> reply = jdb.command(JdbCommand.eval(expr.replace("$classname", DEBUGGEE_CLASS))); 216 new OutputAnalyzer(reply.stream().collect(Collectors.joining(lineSeparator))) 217 .shouldNotContain("Arguments match no method"); 218 }  Minor: It feels like the method name need a tweak.         Something like: verifyNoArgumentsMatchNoMethod 292 // These overload calls should fail because ther Minor: A suggestion is to fix a typo at the end of comment. http://cr.openjdk.java.net/~amenkov/sh2java/step2/webrev/test/jdk/com/sun/jdi/EvalArraysAsList.java.frames.html   The comment was not converted: 31 # The test checks if evaluation of the _expression_ java.util.Arrays.asList(null, "a") 32 # works normally and does not throw an IllegalArgumentException. http://cr.openjdk.java.net/~amenkov/sh2java/step2/webrev/test/jdk/com/sun/jdi/EvalInterfaceStatic.java.frames.html   The comment was not converted: 33 # The test exercises the ability to invoke static methods on interfaces. 34 # Static interface methods are a new feature added in JDK8. 35 # 36 # The test makes sure that it is, at all, possible to invoke an interface 37 # static method and that the static methods are not inherited by extending 38 # interfaces. Thanks, Serguei On 8/16/18 14:13, Alex Menkov wrote: Hi all, Please review next chunk of shell->java test conversion. jira: https://bugs.openjdk.java.net/browse/JDK-8209604 webrev: http://cr.openjdk.java.net/~amenkov/sh2java/step2/webrev/ The fix contains some changes in library classes: Jdb.java - timeouts are updated (as per Dan note in one of previous review, timeouts should respect timeout factor, Utils.adjustTimeout implements the functionality); JdbCommand.java - new jdb commands (required by tests) are added. --alex