Increase timeout in gdb.mi/list-thread-groups-available.exp
Commit Message
On 01-08-19 21:16, Simon Marchi wrote:
> On 2019-08-01 2:18 p.m., Tom Tromey wrote:
>>>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:
>>
>> Simon> PR gdb/24863
>> Simon> * gdb.mi/list-thread-groups-available.exp: Increase timeout for
>> Simon> -list-thread-groups --available test.
>>
>> Simon> +save_vars { timeout } {
>> Simon> + # Increase the timeout: when running with `make check-read1`, this can take
>> Simon> + # a bit of time, as there is a lot of output generated, hence a lot of read
>> Simon> + # syscalls.
>> Simon> + set timeout [expr $timeout * 10]
>>
>> Maybe this should use with_timeout_factor.
>
> Ah, totally, I forgot about its existence. Here's a version using that.
>
> From d55b3eb5acd69b87495c9eeb57f96e4228911dbc Mon Sep 17 00:00:00 2001
> From: Simon Marchi <simon.marchi@efficios.com>
> Date: Thu, 1 Aug 2019 10:28:52 -0400
> Subject: [PATCH] Increase timeout in gdb.mi/list-thread-groups-available.exp
>
> Running
>
> make check-read1 TESTS="gdb.mi/list-thread-groups-available.exp"
>
> on my machine results in timeout failures. Running it while having
> `tail -F testsuite/gdb.log` on the side shows that the test is never
> really blocked, it is just slow at consuming the large output generated
> by `-list-thread-groups --available` (which lists all the processes on
> the system).
>
> If I increase the timeout to a large value, the test passes in ~30
> seconds (compared to under 1 second normally).
>
> Increase the timeout for the particular mi_gdb_test that is long to
> execute under read1. The new timeout value is a bit arbitrary. The
> default timeout is 10 seconds, so I set the new timeout to be
> "old-timeout * 10", so 100 seconds in the typical case.
>
> gdb/testsuite/ChangeLog:
>
> PR gdb/24863
> * gdb.mi/list-thread-groups-available.exp: Increase timeout for
> -list-thread-groups --available test.
> ---
Hi,
for me, both tests fail with a timeout. And if we're increasing the
timeout, how about we only do that if check-read1 is used?
Thanks,
- Tom
Comments
>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
Tom> for me, both tests fail with a timeout. And if we're increasing the
Tom> timeout, how about we only do that if check-read1 is used?
I'm reluctant to make the test suite more sensitive to the environment
it's running it. Is the reason to do this that the test can time out
normally, and so we'd like to avoid lengthy timeouts? If that's the
case, can the test be fixed somehow instead?
I guess my mental model here is that a timeout should not matter unless
a test is flaky. But maybe that's naive? I don't know :-)
Tom> +proc with_timeout_factor { factor body {body_uplevel 1}} {
I think body_uplevel shouldn't be needed.
Tom> + return [with_timeout_factor $factor $body 2]
... since this can just do
return [uplevel [list with_timeout_factor $factor $body]]
thanks,
Tom
On 2019-08-02 9:43 a.m., Tom Tromey wrote:
> I'm reluctant to make the test suite more sensitive to the environment
> it's running it. Is the reason to do this that the test can time out
> normally, and so we'd like to avoid lengthy timeouts? If that's the
> case, can the test be fixed somehow instead?
>
> I guess my mental model here is that a timeout should not matter unless
> a test is flaky. But maybe that's naive? I don't know :-)
No, the test is not expected to time out under normal circumstances. The advantage
of having with_read1_timeout_factor is just that if we happen to break this test
and make it time out, we'll have to wait 10 seconds instead of 100 when running without
read1.
Given that the probability of breaking this test is very small, I don't have
a strong opinion on the matter, it doesn't affect the correctness of the testsuite.
Simon
On 02-08-19 15:43, Tom Tromey wrote:
> Tom> +proc with_timeout_factor { factor body {body_uplevel 1}} {
>
> I think body_uplevel shouldn't be needed.
>
> Tom> + return [with_timeout_factor $factor $body 2]
>
> ... since this can just do
>
> return [uplevel [list with_timeout_factor $factor $body]]
Ack, I've used that in the committed version (
https://sourceware.org/ml/gdb-patches/2019-08/msg00107.html ).
Thanks,
- Tom
On 02-08-19 16:46, Simon Marchi wrote:
> On 2019-08-02 9:43 a.m., Tom Tromey wrote:
>> I'm reluctant to make the test suite more sensitive to the environment
>> it's running it. Is the reason to do this that the test can time out
>> normally, and so we'd like to avoid lengthy timeouts? If that's the
>> case, can the test be fixed somehow instead?
>>
>> I guess my mental model here is that a timeout should not matter unless
>> a test is flaky. But maybe that's naive? I don't know :-)
>
> No, the test is not expected to time out under normal circumstances. The advantage
> of having with_read1_timeout_factor is just that if we happen to break this test
> and make it time out, we'll have to wait 10 seconds instead of 100 when running without
> read1.
>
> Given that the probability of breaking this test is very small, I don't have
> a strong opinion on the matter, it doesn't affect the correctness of the testsuite.
I went ahead and committed a patch introducing with_read1_timeout_factor
( https://sourceware.org/ml/gdb-patches/2019-08/msg00107.html ).
Simon, can you commit your patch, fixing both mi_gdb_tests?
Thanks,
- Tom
On 2019-08-05 9:58 a.m., Tom de Vries wrote:
> I went ahead and committed a patch introducing with_read1_timeout_factor
> ( https://sourceware.org/ml/gdb-patches/2019-08/msg00107.html ).
>
> Simon, can you commit your patch, fixing both mi_gdb_tests?
>
> Thanks,
> - Tom
Thanks, I changed my patch to use with_read1_timeout_factor and pushed it.
Simon
[gdb/testsuite] Increase timeout in gdb.mi/list-thread-groups-available.exp
Running
make check-read1 TESTS="gdb.mi/list-thread-groups-available.exp"
on my machine results in timeout failures. Running it while having
`tail -F testsuite/gdb.log` on the side shows that the test is never
really blocked, it is just slow at consuming the large output generated
by `-list-thread-groups --available` (which lists all the processes on
the system).
If I increase the timeout to a large value, the test passes in ~30
seconds (compared to under 1 second normally).
Increase the timeout for the particular mi_gdb_test that is long to
execute under read1. The new timeout value is a bit arbitrary. The
default timeout is 10 seconds, so I set the new timeout to be
"old-timeout * 10", so 100 seconds in the typical case.
gdb/testsuite/ChangeLog:
PR gdb/24863
* lib/gdb.exp (with_timeout_factor): Add and handle body_uplevel
parameter.
(with_read1_timeout_factor): New proc.
* gdb.mi/list-thread-groups-available.exp: Increase timeout for
-list-thread-groups --available tests.
---
.../gdb.mi/list-thread-groups-available.exp | 20 ++++++++++++--------
gdb/testsuite/lib/gdb.exp | 20 ++++++++++++++++----
2 files changed, 28 insertions(+), 12 deletions(-)
@@ -54,10 +54,12 @@ set cores_re "cores=\\\[(\"$decimal\"(,\"$decimal\")*)?\\\]"
# List all available processes.
set process_entry_re "{${id_re},${type_re}(,$description_re)?(,$user_re)?(,$cores_re)?}"
-mi_gdb_test \
- "-list-thread-groups --available" \
- "\\^done,groups=\\\[${process_entry_re}(,$process_entry_re)*\\\]" \
- "list available thread groups"
+with_read1_timeout_factor 10 {
+ mi_gdb_test \
+ "-list-thread-groups --available" \
+ "\\^done,groups=\\\[${process_entry_re}(,$process_entry_re)*\\\]" \
+ "list available thread groups"
+}
# List specific processes, make sure there are two entries.
set spawn_id_1 [remote_spawn target $binfile]
@@ -79,10 +81,12 @@ set process_entry_re_2 "{${id_re_2},${type_re}(,$description_re)?(,$user_re)?(,$
set process_list_re "(${process_entry_re_1},${process_entry_re_2}|${process_entry_re_2},${process_entry_re_1})"
-mi_gdb_test \
- "-list-thread-groups --available i${pid_1} i${pid_2}" \
- "\\^done,groups=\\\[${process_list_re}\\\]" \
- "list available thread groups with filter"
+with_read1_timeout_factor 10 {
+ mi_gdb_test \
+ "-list-thread-groups --available i${pid_1} i${pid_2}" \
+ "\\^done,groups=\\\[${process_list_re}\\\]" \
+ "list available thread groups with filter"
+}
kill_wait_spawned_process $spawn_id_1
kill_wait_spawned_process $spawn_id_2
@@ -2338,16 +2338,16 @@ proc get_largest_timeout {} {
return $tmt
}
-# Run tests in BODY with timeout increased by factor of FACTOR. When
-# BODY is finished, restore timeout.
+# Run tests in BODY with BODY_UPLEVEL and with timeout increased by factor of
+# FACTOR. When BODY is finished, restore timeout.
-proc with_timeout_factor { factor body } {
+proc with_timeout_factor { factor body {body_uplevel 1}} {
global timeout
set savedtimeout $timeout
set timeout [expr [get_largest_timeout] * $factor]
- set code [catch {uplevel 1 $body} result]
+ set code [catch {uplevel $body_uplevel $body} result]
set timeout $savedtimeout
if {$code == 1} {
@@ -2358,6 +2358,18 @@ proc with_timeout_factor { factor body } {
}
}
+# Run BODY with timeout factor FACTOR if check-read1 is used.
+
+proc with_read1_timeout_factor { factor body } {
+ if { [info exists ::env(READ1)] == 1 && $::env(READ1) == 1 } {
+ # Use timeout factor
+ } else {
+ # Reset timeout factor
+ set factor 1
+ }
+ return [with_timeout_factor $factor $body 2]
+}
+
# Return 1 if _Complex types are supported, otherwise, return 0.
gdb_caching_proc support_complex_tests {