Thanks Chris,
thanks,
Paru.
On 2/16/18, 10:59 AM, Chris Plummer wrote:
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
<http://cr.openjdk.java.net/%7Epsomashe/4916621/webrev.01/>.01
<http://cr.openjdk.java.net/%7Epsomashe/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
<mailto: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 <https://bugs.openjdk.java.net/browse/JDK-4916621>
Webrev : http://cr.openjdk.java.net/~psomashe/4916621/webrev/
<http://cr.openjdk.java.net/%7Epsomashe/4916621/webrev/>
The updated tests ran successfully with Mach5.
thanks,
Paru.
