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

Alex Menkov alexey.menkov at oracle.com
Thu Oct 4 23:11:00 UTC 2018


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
> 


More information about the serviceability-dev mailing list