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.
