RFR JDK-8211292: [TEST] convert com/sun/jdi/DeferredStepTest.sh test

[serguei.spitsyn at oracle.com]

Hi Alex,

It looks good to me.
Could you, please, also remove the line? :

  156             //

No need in new webrev.

Thanks,
Serguei


On 10/4/18 4:11 PM, Alex Menkov wrote:
> Hi Serguei,
>
> Updated webrev:
> http://cr.openjdk.java.net/~amenkov/sh2java/DeferredStep_final/webrev.02/
>
> Fixed all issues except
>  140                 // the 1st stop in the thread
>  141                 break;
> In this case the comment is an explanation why we reach the block, not 
> an explanation for the "break" statement.
>
> --alex
>
> On 10/04/2018 13:56, serguei.spitsyn at oracle.com wrote:
>> Hi Alex,
>>
>> Several minor suggestions.
>>
>> 77 new Thread(aRP, "jj1").start();
>> 78 new Thread(asRP, "jj2").start(); What mean aRP and asRP? In fact, 
>> it is confusing. Can they be renamed to something like obj1 and obj2.
>>
>> 79 // new Thread(aRP, "jj3").start();
>> 80 // new Thread(asRP, "jj4").start();
>>
>>   These lines can be removed.
>>
>> 94 // line of the last stop
>> 95 int lastLine = -1;
>> 96 // min line (-1 means "not known yet")
>> 97 int minLine = -1;
>> 98 // max line (-1 means "not known yet")
>> 99 int maxLine = -1; ... 140 // the 1st stop in the thread
>> 141 break;
>>
>>    I'd suggest the refactor above as below:
>>
>> int lastLine = -1;  // line of the last stop
>> int minLine = -1;  // min line (-1 means "not known yet")
>> int maxLine = -1;// max line (-1 means "not known yet")
>>   ...
>> break;  // the 1st stop in the thread
>>
>> 116 private void next() {
>> 117 List<String> reply = jdb.command(JdbCommand.next());
>> 118 /* each "next" produces something like ("Breakpoint hit" line 
>> only if the line has BP)
>> 119 Step completed:
>> 120 Breakpoint hit: "thread=jj2", DeferredStepTestTarg$jj2.run(), 
>> line=74 bci=12
>> 121 74 ++count2; // @2 breakpoint
>> 122 <empty line>
>> 123 jj2[1]
>> 124 */ It would better to have it in this style: 118 /* * Each "next" 
>> produces something like ("Breakpoint hit" line only if the line has BP).
>> 119 * Step completed:
>> 120 * Breakpoint hit: "thread=jj2", DeferredStepTestTarg$jj2.run(), 
>> line=74 bci=12
>> 121 * 74 ++count2; // @2 breakpoint
>> 122 * <empty line>
>> 123 * jj2[1]
>> 124 */
>>
>>
>> Otherwise, it looks Okay to me.
>>
>>
>> Thanks,
>> Serguei
>>
>> On 10/3/18 5:49 PM, Alex Menkov wrote:
>>> Hi all,
>>>
>>> please review a fix for
>>> https://bugs.openjdk.java.net/browse/JDK-8211292
>>> webrev:
>>> http://cr.openjdk.java.net/~amenkov/sh2java/DeferredStep_final/webrev/
>>>
>>> The fix converts manual shell test to automatic java (as Java allows 
>>> to parse jdb replies much easier).
>>> This is the last sub-task for the "shell to java conversion" task, 
>>> so the fix also removes shared shell scripts.
>>>
>>> --alex
>>