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

[Paru Somashekar]

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.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/5fda913d/attachment.html>