Re: [patch] print-threads.exp: Extend timeout for slower tests.

[Doug Evans <dje@google.com>]

On Mon, Nov 7, 2011 at 11:56 PM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> On Mon, 07 Nov 2011 17:22:02 +0100, Joel Brobecker wrote:
>> > >> +# Record the old timeout, we need to extend it for slower tests.
>> > >> +set oldtimeout $timeout
>> > >
>> > > You can use global $gdb_test_timeout nowadays.
>> >
>> > Thanks, I didn't know about gdb_test_timeout.
>> > Still, I like the patch as is, one less global the code has to care about.
>>
>> Same here.
>
> I did not agree with Doug.  I think there was just some misunderstanding.
>
> I was proposing the attached patch.
>
> The very last line is sure redundant but I think it should be there for the
> possibility of future test additions.
>
> Found in Doug's original patch:
>        set $timeout $oldtimeout
> should have been:
>        set timeout $oldtimeout

That's just a typo (obviously).

>
> Thanks,
> Jan
>
>
> gdb/testsuite/
> 2011-11-08  Doug Evans  <dje@google.com>
>            Jan Kratochvil  <jan.kratochvil@redhat.com>
>
>        * gdb.threads/print-threads.exp: Extend timeout for slower tests.
>
> --- a/gdb/testsuite/gdb.threads/print-threads.exp
> +++ b/gdb/testsuite/gdb.threads/print-threads.exp
> @@ -106,12 +106,18 @@ test_all_threads "fast" 0
>  runto_main
>  gdb_test "break thread_function" "Breakpoint \[0-9\]+ at 0x\[0-9a-f\]+: file .*print-threads.c, line \[0-9\]*\\." "break thread_function (2)"
>  gdb_test_no_output "set var slow = 1"
> +# Extend the timeout for slower tests.
> +set timeout [expr $gdb_test_timeout + 120]
>  test_all_threads "slow" 0
> +set timeout $gdb_test_timeout
>
>  runto_main
>  gdb_test "break thread_function" "Breakpoint \[0-9\]+ at 0x\[0-9a-f\]+: file .*print-threads.c, line \[0-9\]*\\." "break thread_function (3)"
>  gdb_test_no_output "set var slow = 1" "set var slow = 1 (2)"
>  gdb_breakpoint "kill"
> +# Extend the timeout for slower tests.
> +set timeout [expr $gdb_test_timeout + 120]
>  test_all_threads "slow with kill breakpoint" 1
> +set timeout $gdb_test_timeout
>
>  return 0
>

I disagree.
There are no existing uses of gdb_test_timeout outside of lib/gdb.exp,
and regardless, I prefer my patch.  "timeout" is a standard global variable
.exp writers know about.  Adding gdb_test_timeout is one more global,
one more bit of context .exp writers are expected to know about.  I
think it's fine as a mechanism to ensure a stable and consistent value
at the start of the test, but that his hidden by lib/gdb.exp routines.
 Once I'm in a testsuite .exp file, I'd rather not have to involve it
in my hacking.