RFR JDK-8210243: [TEST] rewrite com/sun/jdi shell tests to java version - step3

Alex Menkov alexey.menkov at oracle.com
Fri Aug 31 17:26:15 UTC 2018


Hi Serguei,

On 08/30/2018 23:12, serguei.spitsyn at oracle.com wrote:
> Hi Alex,
> 
> It looks good in general but not sure I understand all the changes.
> Could you, please, tell a little bit more about the refactoring in the 
> Jdb.java and JdbTest.java?
> I understand that you moved some code from Jdb.java to JdbTest.java.
> But it is hard to track all of these move details.
> What was the main refactoring logic?

Before the fix Jdb class contained logic to launch debuggee, parse 
listening port for debugger and run jdb connecting to the port.
Now it contains only jdb-related code.
Debuggee-related code is moved to JdbTest (including LaunchOptions inner 
class which contains info about debuggee main class and options).
I thought about moving debuggee-related code to dedicated class 
(Debuggee), but it's quite small amount of code (main part is 
implemented in jdk.test.lib.process.ProcessTools) and I don't see any 
chance the class can be reused.

> 
> Some comments on this update:
> 
> http://cr.openjdk.java.net/~amenkov/sh2java/step3/webrev.01/test/jdk/com/sun/jdi/NullLocalVariable.java.frames.html
> 
> 30 * @library /test/lib
> 31 * @compile -g JdbExprTest.java
> 32 * @run main/othervm JdbExprTest
> 
>    Why the class 'JdbExprTest' is used above?
>    Should it be the 'NullLocalVariable' instead?

Good catch!
Fixed.
updated webrev:
cr.openjdk.java.net/~amenkov/sh2java/step3/webrev.02/

--alex

> 
> Thanks,
> Serguei
> 
> 
> On 8/30/18 16:27, Alex Menkov wrote:
>> Hi all,
>>
>> Please review a fix for
>> https://bugs.openjdk.java.net/browse/JDK-8210243
>> webrev:
>> http://cr.openjdk.java.net/~amenkov/sh2java/step3/webrev.01/
>>
>> The fix converts the following tests:
>> - test/jdk/com/sun/jdi/JdbArgTest.sh
>> - test/jdk/com/sun/jdi/JdbLockTest.sh
>> - test/jdk/com/sun/jdi/JdbMissStep.sh
>> - test/jdk/com/sun/jdi/JdbVarargsTest.sh
>> - test/jdk/com/sun/jdi/MixedSuspendTest.sh
>> - test/jdk/com/sun/jdi/NotAField.sh
>> - test/jdk/com/sun/jdi/NullLocalVariable.sh
>>
>> JdbArgTest requires to run only jdb (without debuggee), so Jdb class 
>> was decoupled - it now contains only jdb logic, debuggee logic was 
>> moved to JdbTest.
>>
>> --alex
> 


More information about the serviceability-dev mailing list