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

serguei.spitsyn at oracle.com serguei.spitsyn at oracle.com
Thu Oct 4 20:56:57 UTC 2018


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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20181004/77d76a18/attachment-0001.html>


More information about the serviceability-dev mailing list