[PATCH] Increase timeout in gdb.mi/list-thread-groups-available.exp

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Increase timeout in gdb.mi/list-thread-groups-available.exp

Simon Marchi-5
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.
---
 .../gdb.mi/list-thread-groups-available.exp      | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/gdb/testsuite/gdb.mi/list-thread-groups-available.exp b/gdb/testsuite/gdb.mi/list-thread-groups-available.exp
index 792c3bac8962..c8486ce1f3d1 100644
--- a/gdb/testsuite/gdb.mi/list-thread-groups-available.exp
+++ b/gdb/testsuite/gdb.mi/list-thread-groups-available.exp
@@ -54,10 +54,18 @@ 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"
+save_vars { timeout } {
+    # Increase the timeout: when running with `make check-read1`, this can take
+    # a bit of time, as there is a lot of output generated, hence a lot of read
+    # syscalls.
+    set timeout [expr $timeout * 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]
--
2.22.0

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Increase timeout in gdb.mi/list-thread-groups-available.exp

Tom Tromey-2
>>>>> "Simon" == Simon Marchi <[hidden email]> 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.

Tom
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Increase timeout in gdb.mi/list-thread-groups-available.exp

Simon Marchi-5
On 2019-08-01 2:18 p.m., Tom Tromey wrote:

>>>>>> "Simon" == Simon Marchi <[hidden email]> 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 <[hidden email]>
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.
---
 .../gdb.mi/list-thread-groups-available.exp         | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/gdb/testsuite/gdb.mi/list-thread-groups-available.exp b/gdb/testsuite/gdb.mi/list-thread-groups-available.exp
index 792c3bac8962..2fbf282c661e 100644
--- a/gdb/testsuite/gdb.mi/list-thread-groups-available.exp
+++ b/gdb/testsuite/gdb.mi/list-thread-groups-available.exp
@@ -54,10 +54,15 @@ 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"
+# Increase the timeout: when running with `make check-read1`, this can take
+# a bit of time, as there is a lot of output generated, hence a lot of read
+# syscalls.
+with_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]
--
2.22.0


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Increase timeout in gdb.mi/list-thread-groups-available.exp

Tom de Vries
On 01-08-19 21:16, Simon Marchi wrote:

> On 2019-08-01 2:18 p.m., Tom Tromey wrote:
>>>>>>> "Simon" == Simon Marchi <[hidden email]> 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 <[hidden email]>
> 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


[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(-)

diff --git a/gdb/testsuite/gdb.mi/list-thread-groups-available.exp b/gdb/testsuite/gdb.mi/list-thread-groups-available.exp
index 792c3bac89..79b8d4d327 100644
--- a/gdb/testsuite/gdb.mi/list-thread-groups-available.exp
+++ b/gdb/testsuite/gdb.mi/list-thread-groups-available.exp
@@ -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
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 9ca34d8b15..b645100fc0 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -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 {
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Increase timeout in gdb.mi/list-thread-groups-available.exp

Tom Tromey-2
>>>>> "Tom" == Tom de Vries <[hidden email]> 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
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Increase timeout in gdb.mi/list-thread-groups-available.exp

Simon Marchi-4
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
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Increase timeout in gdb.mi/list-thread-groups-available.exp

Tom de Vries
In reply to this post by Tom Tromey-2
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
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Increase timeout in gdb.mi/list-thread-groups-available.exp

Tom de Vries
In reply to this post by Simon Marchi-4
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
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Increase timeout in gdb.mi/list-thread-groups-available.exp

Simon Marchi-4
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
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Increase timeout in gdb.mi/list-thread-groups-available.exp

Pedro Alves-7
In reply to this post by Simon Marchi-5
On 8/1/19 3:38 PM, Simon Marchi wrote:

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

A better approach to fix this is match a line at a time using gdb_test_multiple
and exp_continue.

  -re "$process_entry_re" {
     exp_continue
  }

No need to bump the timeout that way, since by default exp_continue resets
the timeout timer.

Thanks,
Pedro Alves