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

[Paru Somashekar]

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.spitsyn at oracle.com 
>>> <mailto:serguei.spitsyn at 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.
>>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20180216/882f88ad/attachment.html>