Still good. Thanks, /Staffan
On 28 feb 2014, at 15:36, Pavel Punegov <pavel.pune...@oracle.com> wrote: > 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.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' >>> > 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
