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

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

[Simon Marchi]

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

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

[Tom Tromey]

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

Tom

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

[Simon Marchi]

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

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

[Tom Tromey]

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

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

[Simon Marchi]

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

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

[Tom de Vries]

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

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

[Simon Marchi]

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

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

[Tom de Vries]

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

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

[Pedro Alves]

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