RFR 6946101: ShellScaffold.sh based tests can issue "write error: Broken pipe" messages

[Pavel Punegov]

New wevrev, thanks to Igor I.:
  http://cr.openjdk.java.net/~iignatyev/ppunegov/6946101/webrev.01/

Fixed typos/grammar
Added next string to catch the situation when jdb exited with 
input stream closed prematurely (break in a while loop before the check) :
   996     # jdb exited because its input stream closed prematurely
   997     jdbFailIfPresent "Input stream closed"

It could happen if dojdbCmds() subshell was killed or exited prematurely.
Without this check test fails with no complains about Input stream.


On Fri, 28 of Feb 2014 09:31:29 Staffan Larsen wrote:
> 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
Fixed

> Just a note that this should be pushed through jdk9/dev and not jdk9/hs-comp
> (where the webrev was made).
Igor made a webrev based on jdk9/dev and will sponsor me.


On 27 feb 2014, at 23:47, Daniel D. Daugherty <daniel.daugherty at 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'
>> 
Fixed

>>    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 
Yep

>>    - 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
The test  JdbMethodExitTest.sh  doesn't have allowExit set for the last
'cmd cont'.  This 'cont' should run jdb to breakpoint set with bkpt();
See java file inside the test:
  183        // test trace method exit
  184        traceExit1();
  185        bkpt();
  186 
  187   }

>>    - 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
My testing (see in the bug comments)  shows that old logic can only catch the  
message absence with  failIfNotPresent() functions. If I remove these checks 
tests will silently pass even though we have sent 'cmd exit'  inside the test.

-- 
Thanks,
Pavel Punegov