Re: RFR 4916621: update those still using JDIScaffold to use TestScaffold instead

[Chris Plummer]

Hi Paru, So it looks like the code I pointed out that you removed was actually all redundant since it is covered by startTo(). Is that correct? If so, why did the test not produce any errors? What happens when connect() is called twice on the same debuggee? thanks, Chris On 2/15/18 11:25 PM, Paru Somashekar wrote: Hi Chris, Thanks for the review. I have updated the patch that has the change you suggested i.e. we just need startTo("TestVars", "hi", "()V"); in the runTests method and the code to save the value of args is not needed at all. Also, I realized that there was one more class in the list ( ModificationWatchpoints.java) which needed an update to its @run build JDIScaffold comment to the class. So I included that change as well in the review. Note : this class has already been updated to extend TestScaffold. http://cr.openjdk.java.net/~psomashe/4916621/webrev.01 thanks, Paru. On 2/15/18, 4:22 PM, Chris Plummer wrote: Hi Paru, I think instead of moving the following code, which did to avoid having to save the value of args, can instead replaced with a call to TestScaffold.startup("TestVars"):  147         List argList = new ArrayList(Arrays.asList(args));  148         argList.add("TestVars");  149         System.out.println("run args: " + argList);  150         connect((String[])argList.toArray(args));  151         waitForVMStart(); Other than that it looks good. thanks, Chris On 2/15/18 3:46 PM, serguei.spit...@oracle.com wrote: Hi Paru, This looks good. We need one more reviewer. Thanks, Serguei On 2/15/18 15:22, Paru Somashekar wrote: Hi, Please review fix for JDK-4916621 These are the remaining tests from the list that needed to be updated. Bug : JDK-4916621 Webrev : http://cr.openjdk.java.net/~psomashe/4916621/webrev/ The updated tests ran successfully with Mach5. thanks, Paru.