Very nice change - looks good!
test/com/sun/jdi/ShellScaffold.sh
line 1000: # mydojdbCmds() didn't finished because it waits for JDB
message
nit: finished -> finish
Just a note that this should be pushed through jdk9/dev and not jdk9/hs-comp
(where the webrev was made).
Thanks,
/Staffan
On 27 feb 2014, at 23:47, Daniel D. Daugherty <daniel.daughe...@oracle.com>
wrote:
> On 2/27/14 9:20 AM, Pavel Punegov wrote:
>> Please review the fix for:
>> https://bugs.openjdk.java.net/browse/JDK-6946101
>>
>> webrev:
>> http://cr.openjdk.java.net/~iignatyev/ppunegov/6946101/webrev.00/
>
> test/com/sun/jdi/ShellScaffold.sh
> line 531: # allows JDB to exit"
> stray double-quote at end of comment
>
> line 563: dofail "It's not allowed to send quit and exit commands from the
> test"
> 'and' should be 'or'
>
> line 819: # Kill debugger, it could be hang
> Typo: 'hang' -> 'hung'
>
> test/com/sun/jdi/ArrayLengthDumpTest.sh
> test/com/sun/jdi/CatchAllTest.sh
> test/com/sun/jdi/CatchCaughtTest.sh
> test/com/sun/jdi/CatchPatternTest.sh
> test/com/sun/jdi/CommandCommentDelimiter.sh
> test/com/sun/jdi/DeferredStepTest.sh
> test/com/sun/jdi/DeoptimizeWalk.sh
> test/com/sun/jdi/EvalArgs.sh
> test/com/sun/jdi/GetLocalVariables3Test.sh
> test/com/sun/jdi/GetLocalVariables4Test.sh
> test/com/sun/jdi/JdbExprTest.sh
> test/com/sun/jdi/JdbLockTest.sh
> test/com/sun/jdi/JdbMethodExitTest.sh
> test/com/sun/jdi/JdbMissStep.sh
> test/com/sun/jdi/MixedSuspendTest.sh
> test/com/sun/jdi/NotAField.sh
> test/com/sun/jdi/NullLocalVariable.sh
> test/com/sun/jdi/Redefine-g.sh
> test/com/sun/jdi/RedefineAnnotation.sh
> test/com/sun/jdi/RedefineChangeClassOrder.sh
> test/com/sun/jdi/RedefineClasses.sh
> test/com/sun/jdi/RedefineException.sh
> test/com/sun/jdi/RedefineFinal.sh
> test/com/sun/jdi/RedefineImplementor.sh
> test/com/sun/jdi/RedefineIntConstantToLong.sh
> test/com/sun/jdi/RedefineMulti.sh
> test/com/sun/jdi/RedefinePop.sh
> test/com/sun/jdi/RedefineTTYLineNumber.sh
> test/com/sun/jdi/StringConvertTest.sh
> test/com/sun/jdi/WatchFramePop.sh
> I _think_ I understand the new test driver style:
>
> - get rid of all explicit 'cmd quit' usages because mydojdbCmds()
> now wraps the test's dojdbCmds with a 'quit' cmd
> - any test that previously ended with a 'cmd cont' is presumed to
> be OK of that 'cmd cont' caused jdb to execute off the end of
> main(); sounds reasonable to me
> - perfect example of the new logic to catch an errant run off the
> end is test/com/sun/jdi/WatchFramePop.sh
> - the last jdb cmd is 'next'
> - and jdb is NOT supposed to run off the end
> - the new logic should catch this nicely; I _think_ the old
> logic would only catch a run off the end if someone manually
> checked the test result
>
>
> Thumbs up!
>
> Dan
>
>
>>
>>
>> This change fixes two issues with the tests:
>> 1. Fix incorrect 'quit' command sending to JDB when JDB process was
>> finished.
>> 2. Improve JDB unexpected exit detection and process synchronization.
>>
>> Description of fix:
>> 1. Add allowExit parameter to cmd() to show that the given command can
>> finish
>> JDB. E.g., 'cont' command make JDB execute debuggee to the end.
>> If allowExit wasn't set for a command then assume that it can't finish
>> execution, and fail the test if it did.
>>
>> 2. Make test fail if it tries to send 'quit' or 'exit' commands. This makes
>> it impossible to send quit/exit from test by mistake. Scaffold will
>> finish JDB by itself if JDB didn't finish before be a command with
>> allowExit
>> set. Add dofinish() function to be the only method that may exit JDB.
>>
>> 3. Add proper synchronization into waitForFinish(). On all systems except
>> SunOS use wait (from bash). On Solaris find the shell subprocess and wait
>> for
>> its finish. It replaces wait used on all other systems, because it
>> doesn't work on sh/ksh as in bash.
>>
>> 4. Fix tests: add allowExit to tests where it's needed.
>>
>
