|
Hi Paru,
Thanks for the explanation. Your changes look fine.
Chris
On 2/16/18 10:38 AM, Paru Somashekar wrote:
Hi Chris,
Firstly, when I moved the code from runTests() method to the
main() method of the debugger, I only moved that part where the
argument was saved - not the call to connect. Hence connect() was
never called twice. Connect() gets called when we do a
startTo(..). Hence the saving of the argument did no harm and
produced no errors.
thanks,
Paru.
On 2/16/18, 10:14 AM, Chris Plummer wrote:
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.
|
