public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] gdb/testsuite: modernize gdb.base/maint.exp
@ 2022-05-09 18:04 Bruno Larsen
  2022-05-23 14:14 ` [PING] " Bruno Larsen
                   ` (3 more replies)
  0 siblings, 4 replies; 25+ messages in thread
From: Bruno Larsen @ 2022-05-09 18:04 UTC (permalink / raw)
  To: gdb-patches

gdb.base/maint.exp was using several gdb_expect statements, probably
because this test case predates the existance of gdb_test_multiple. This
commit updates the test case to use gdb_test_multiple, making it more
resilient to internal errors and such.

The only gdb_expect left in the testcase is one that specifically looks
for an internal error being triggered as a PASS.
---
 gdb/testsuite/gdb.base/maint.exp | 109 ++++++++-----------------------
 1 file changed, 29 insertions(+), 80 deletions(-)

diff --git a/gdb/testsuite/gdb.base/maint.exp b/gdb/testsuite/gdb.base/maint.exp
index 45ccdc6584e..2efdda9aed7 100644
--- a/gdb/testsuite/gdb.base/maint.exp
+++ b/gdb/testsuite/gdb.base/maint.exp
@@ -155,18 +155,10 @@ set have_psyms [expr ! ( $have_gdb_index || $readnow_p )]
 # guo: on linux this command output is huge.  for some reason splitting up
 # the regexp checks works.
 #
-send_gdb "maint check-psymtabs\n"
-gdb_expect  {
-    -re "^maint check-psymtabs" {
-	gdb_expect {
-	    -re "$gdb_prompt $" {
-		pass "maint check-psymtabs"
-	    }
-	    timeout { fail "(timeout) maint check-psymtabs" }
-	}
+gdb_test_multiple "maint check-psymtabs" "maint check-psymtabs" {
+    -re -wrap "^maint check-psymtabs.*" {
+	pass "maint check-psymtabs"
     }
-    -re ".*$gdb_prompt $"     { fail "maint check-psymtabs" }
-    timeout         { fail "(timeout) maint check-psymtabs" }
 }
 
 # This command does not produce any output unless there is some problem
@@ -270,62 +262,38 @@ if { $have_psyms } {
 		       "maint print psymbols -pc" \
 		       "maint print psymbols -pc main $psymbols_output"]
     foreach { test_name command } $test_list {
-	send_gdb "$command\n"
-	    gdb_expect  {
-		-re "^maint print psymbols \[^\n\]*\r\n$gdb_prompt $" {
-		    send_gdb "shell ls $psymbols_output\n"
-		    gdb_expect {
-			-re "$psymbols_output_re\r\n$gdb_prompt $" {
-			    # We want this grep to be as specific as possible,
-			    # so it's less likely to match symbol file names in
-			    # psymbols_output.  Yes, this actually happened;
-			    # poor expect got tons of output, and timed out
-			    # trying to match it.   --- Jim Blandy <jimb@cygnus.com>
-			    send_gdb "shell grep 'main.*function' $psymbols_output\n"
-			    gdb_expect {
-				-re ".main., function, $hex.*$gdb_prompt $" {
-				    pass "$test_name 1"
-				}
-				-re ".*main.  .., function, $hex.*$gdb_prompt $" {
-				    pass "$test_name 2"
-				}
-				-re ".*$gdb_prompt $" { fail "$test_name" }
-				timeout { fail "$test_name (timeout)" }
-			    }
+	gdb_test_multiple "$command" "$test_name" {
+	    -re  -wrap "^maint print psymbols \[^\n\]*" {
+		gdb_test_multiple "shell grep 'main.*function' $psymbols_output"\
+		    "$test_name internal"{
+			-re -wrap ".main., function, $hex.*" {
+			    gdb_test "shell rm -f $psymbols_output" ".*" \
+				"${test_name}: shell rm -f psymbols_output"
+			    pass "$test_name 1"
+			}
+			-re -wrap ".*main.  .., function, $hex.*" {
 			    gdb_test "shell rm -f $psymbols_output" ".*" \
 				"${test_name}: shell rm -f psymbols_output"
+			    pass "$test_name 2"
 			}
-			-re ".*$gdb_prompt $" { fail "$test_name" }
-			timeout { fail "$test_name (timeout)" }
-		    }
 		}
-		-re ".*$gdb_prompt $" { fail "$test_name" }
-		timeout { fail "$test_name (timeout)" }
 	    }
+	}
     }
 }
 
 
 set msymbols_output [standard_output_file msymbols_output]
 set msymbols_output_re [string_to_regexp $msymbols_output]
-send_gdb "maint print msymbols -objfile ${binfile} $msymbols_output\n"
-gdb_expect  {
-    -re "^maint print msymbols \[^\n\]*\r\n$gdb_prompt $" {
-	send_gdb "shell ls $msymbols_output\n"
-	gdb_expect {
-	    -re "$msymbols_output_re\r\n$gdb_prompt $" {
-		gdb_test "shell grep factorial $msymbols_output" \
-		    "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
-		    "maint print msymbols, absolute pathname"
-		gdb_test "shell rm -f $msymbols_output" ".*" \
-		    "shell rm -f msymbols_output"
-	    }
-	    -re ".*$gdb_prompt $" { fail "maint print msymbols" }
-	    timeout { fail "maint print msymbols (timeout)" }
-	}
+gdb_test_multiple "maint print msymbols -objfile ${binfile} $msymbols_output" \
+"printing msymbols" {
+    -re -wrap "^maint print msymbols \[^\n\]*" {
+	gdb_test "shell grep factorial $msymbols_output" \
+	    "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
+	    "maint print msymbols, absolute pathname"
+	gdb_test "shell rm -f $msymbols_output" ".*" \
+	    "shell rm -f msymbols_output"
     }
-    -re ".*$gdb_prompt $" { fail "maint print msymbols" }
-    timeout { fail "maint print msymbols (timeout)" }
 }
 
 # Check that maint print msymbols allows relative pathnames
@@ -363,31 +331,12 @@ set test_list [list \
 		   "maint print symbols -pc" \
 		   "maint print symbols -pc main $symbols_output"]
 foreach { test_name command } $test_list {
-    send_gdb "$command\n"
-    gdb_expect {
-	-re "^maint print symbols \[^\n\]*\r\n$gdb_prompt $" {
-	    send_gdb "shell ls $symbols_output\n"
-	    gdb_expect {
-		-re "$symbols_output_re\r\n$gdb_prompt $" {
-		    # See comments for `maint print psymbols'.
-		    send_gdb "shell grep 'main(.*block' $symbols_output\n"
-		    gdb_expect {
-			-re "int main\\(int, char \\*\\*, char \\*\\*\\); block.*$gdb_prompt $" {
-			    pass "$test_name"
-			}
-			-re ".*$gdb_prompt $" { fail "$test_name" }
-			timeout { fail "$test_name (timeout)" }
-		    }
-		    gdb_test "shell rm -f $symbols_output" ".*" \
-			"$test_name: shell rm -f symbols_output"
-		}
-		-re ".*$gdb_prompt $" { fail "$test_name" }
-		timeout { fail "$test_name (timeout)" }
-	    }
-	}
-	-re ".*$gdb_prompt $" { fail "$test_name" }
-	timeout { fail "$test_name (timeout)" }
-    }
+    gdb_test_no_output "$command" "$test_name generate"
+    gdb_test "shell grep 'main(.*block' $symbols_output"\
+	"int main\\(int, char \\*\\*, char \\*\\*\\); block.*"\
+	"$test_name read"
+    gdb_test "shell rm -f $symbols_output" ".*" \
+	"$test_name: shell rm -f symbols_output"
 }
 
 set msg "maint print type"
-- 
2.31.1


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PING] [PATCH] gdb/testsuite: modernize gdb.base/maint.exp
  2022-05-09 18:04 [PATCH] gdb/testsuite: modernize gdb.base/maint.exp Bruno Larsen
@ 2022-05-23 14:14 ` Bruno Larsen
  2022-05-30 11:14   ` [PINGv2] " Bruno Larsen
  2022-06-21 15:52 ` Andrew Burgess
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 25+ messages in thread
From: Bruno Larsen @ 2022-05-23 14:14 UTC (permalink / raw)
  To: gdb-patches

ping

Cheers!
Bruno Larsen

On 5/9/22 15:04, Bruno Larsen wrote:
> gdb.base/maint.exp was using several gdb_expect statements, probably
> because this test case predates the existance of gdb_test_multiple. This
> commit updates the test case to use gdb_test_multiple, making it more
> resilient to internal errors and such.
> 
> The only gdb_expect left in the testcase is one that specifically looks
> for an internal error being triggered as a PASS.
> ---
>   gdb/testsuite/gdb.base/maint.exp | 109 ++++++++-----------------------
>   1 file changed, 29 insertions(+), 80 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.base/maint.exp b/gdb/testsuite/gdb.base/maint.exp
> index 45ccdc6584e..2efdda9aed7 100644
> --- a/gdb/testsuite/gdb.base/maint.exp
> +++ b/gdb/testsuite/gdb.base/maint.exp
> @@ -155,18 +155,10 @@ set have_psyms [expr ! ( $have_gdb_index || $readnow_p )]
>   # guo: on linux this command output is huge.  for some reason splitting up
>   # the regexp checks works.
>   #
> -send_gdb "maint check-psymtabs\n"
> -gdb_expect  {
> -    -re "^maint check-psymtabs" {
> -	gdb_expect {
> -	    -re "$gdb_prompt $" {
> -		pass "maint check-psymtabs"
> -	    }
> -	    timeout { fail "(timeout) maint check-psymtabs" }
> -	}
> +gdb_test_multiple "maint check-psymtabs" "maint check-psymtabs" {
> +    -re -wrap "^maint check-psymtabs.*" {
> +	pass "maint check-psymtabs"
>       }
> -    -re ".*$gdb_prompt $"     { fail "maint check-psymtabs" }
> -    timeout         { fail "(timeout) maint check-psymtabs" }
>   }
>   
>   # This command does not produce any output unless there is some problem
> @@ -270,62 +262,38 @@ if { $have_psyms } {
>   		       "maint print psymbols -pc" \
>   		       "maint print psymbols -pc main $psymbols_output"]
>       foreach { test_name command } $test_list {
> -	send_gdb "$command\n"
> -	    gdb_expect  {
> -		-re "^maint print psymbols \[^\n\]*\r\n$gdb_prompt $" {
> -		    send_gdb "shell ls $psymbols_output\n"
> -		    gdb_expect {
> -			-re "$psymbols_output_re\r\n$gdb_prompt $" {
> -			    # We want this grep to be as specific as possible,
> -			    # so it's less likely to match symbol file names in
> -			    # psymbols_output.  Yes, this actually happened;
> -			    # poor expect got tons of output, and timed out
> -			    # trying to match it.   --- Jim Blandy <jimb@cygnus.com>
> -			    send_gdb "shell grep 'main.*function' $psymbols_output\n"
> -			    gdb_expect {
> -				-re ".main., function, $hex.*$gdb_prompt $" {
> -				    pass "$test_name 1"
> -				}
> -				-re ".*main.  .., function, $hex.*$gdb_prompt $" {
> -				    pass "$test_name 2"
> -				}
> -				-re ".*$gdb_prompt $" { fail "$test_name" }
> -				timeout { fail "$test_name (timeout)" }
> -			    }
> +	gdb_test_multiple "$command" "$test_name" {
> +	    -re  -wrap "^maint print psymbols \[^\n\]*" {
> +		gdb_test_multiple "shell grep 'main.*function' $psymbols_output"\
> +		    "$test_name internal"{
> +			-re -wrap ".main., function, $hex.*" {
> +			    gdb_test "shell rm -f $psymbols_output" ".*" \
> +				"${test_name}: shell rm -f psymbols_output"
> +			    pass "$test_name 1"
> +			}
> +			-re -wrap ".*main.  .., function, $hex.*" {
>   			    gdb_test "shell rm -f $psymbols_output" ".*" \
>   				"${test_name}: shell rm -f psymbols_output"
> +			    pass "$test_name 2"
>   			}
> -			-re ".*$gdb_prompt $" { fail "$test_name" }
> -			timeout { fail "$test_name (timeout)" }
> -		    }
>   		}
> -		-re ".*$gdb_prompt $" { fail "$test_name" }
> -		timeout { fail "$test_name (timeout)" }
>   	    }
> +	}
>       }
>   }
>   
>   
>   set msymbols_output [standard_output_file msymbols_output]
>   set msymbols_output_re [string_to_regexp $msymbols_output]
> -send_gdb "maint print msymbols -objfile ${binfile} $msymbols_output\n"
> -gdb_expect  {
> -    -re "^maint print msymbols \[^\n\]*\r\n$gdb_prompt $" {
> -	send_gdb "shell ls $msymbols_output\n"
> -	gdb_expect {
> -	    -re "$msymbols_output_re\r\n$gdb_prompt $" {
> -		gdb_test "shell grep factorial $msymbols_output" \
> -		    "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
> -		    "maint print msymbols, absolute pathname"
> -		gdb_test "shell rm -f $msymbols_output" ".*" \
> -		    "shell rm -f msymbols_output"
> -	    }
> -	    -re ".*$gdb_prompt $" { fail "maint print msymbols" }
> -	    timeout { fail "maint print msymbols (timeout)" }
> -	}
> +gdb_test_multiple "maint print msymbols -objfile ${binfile} $msymbols_output" \
> +"printing msymbols" {
> +    -re -wrap "^maint print msymbols \[^\n\]*" {
> +	gdb_test "shell grep factorial $msymbols_output" \
> +	    "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
> +	    "maint print msymbols, absolute pathname"
> +	gdb_test "shell rm -f $msymbols_output" ".*" \
> +	    "shell rm -f msymbols_output"
>       }
> -    -re ".*$gdb_prompt $" { fail "maint print msymbols" }
> -    timeout { fail "maint print msymbols (timeout)" }
>   }
>   
>   # Check that maint print msymbols allows relative pathnames
> @@ -363,31 +331,12 @@ set test_list [list \
>   		   "maint print symbols -pc" \
>   		   "maint print symbols -pc main $symbols_output"]
>   foreach { test_name command } $test_list {
> -    send_gdb "$command\n"
> -    gdb_expect {
> -	-re "^maint print symbols \[^\n\]*\r\n$gdb_prompt $" {
> -	    send_gdb "shell ls $symbols_output\n"
> -	    gdb_expect {
> -		-re "$symbols_output_re\r\n$gdb_prompt $" {
> -		    # See comments for `maint print psymbols'.
> -		    send_gdb "shell grep 'main(.*block' $symbols_output\n"
> -		    gdb_expect {
> -			-re "int main\\(int, char \\*\\*, char \\*\\*\\); block.*$gdb_prompt $" {
> -			    pass "$test_name"
> -			}
> -			-re ".*$gdb_prompt $" { fail "$test_name" }
> -			timeout { fail "$test_name (timeout)" }
> -		    }
> -		    gdb_test "shell rm -f $symbols_output" ".*" \
> -			"$test_name: shell rm -f symbols_output"
> -		}
> -		-re ".*$gdb_prompt $" { fail "$test_name" }
> -		timeout { fail "$test_name (timeout)" }
> -	    }
> -	}
> -	-re ".*$gdb_prompt $" { fail "$test_name" }
> -	timeout { fail "$test_name (timeout)" }
> -    }
> +    gdb_test_no_output "$command" "$test_name generate"
> +    gdb_test "shell grep 'main(.*block' $symbols_output"\
> +	"int main\\(int, char \\*\\*, char \\*\\*\\); block.*"\
> +	"$test_name read"
> +    gdb_test "shell rm -f $symbols_output" ".*" \
> +	"$test_name: shell rm -f symbols_output"
>   }
>   
>   set msg "maint print type"


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PINGv2] [PATCH] gdb/testsuite: modernize gdb.base/maint.exp
  2022-05-23 14:14 ` [PING] " Bruno Larsen
@ 2022-05-30 11:14   ` Bruno Larsen
  2022-06-06 12:43     ` [PINGv3] " Bruno Larsen
  0 siblings, 1 reply; 25+ messages in thread
From: Bruno Larsen @ 2022-05-30 11:14 UTC (permalink / raw)
  To: gdb-patches

ping!

Cheers!
Bruno Larsen

On 5/23/22 11:14, Bruno Larsen wrote:
> ping
> 
> Cheers!
> Bruno Larsen
> 
> On 5/9/22 15:04, Bruno Larsen wrote:
>> gdb.base/maint.exp was using several gdb_expect statements, probably
>> because this test case predates the existance of gdb_test_multiple. This
>> commit updates the test case to use gdb_test_multiple, making it more
>> resilient to internal errors and such.
>>
>> The only gdb_expect left in the testcase is one that specifically looks
>> for an internal error being triggered as a PASS.
>> ---
>>   gdb/testsuite/gdb.base/maint.exp | 109 ++++++++-----------------------
>>   1 file changed, 29 insertions(+), 80 deletions(-)
>>
>> diff --git a/gdb/testsuite/gdb.base/maint.exp b/gdb/testsuite/gdb.base/maint.exp
>> index 45ccdc6584e..2efdda9aed7 100644
>> --- a/gdb/testsuite/gdb.base/maint.exp
>> +++ b/gdb/testsuite/gdb.base/maint.exp
>> @@ -155,18 +155,10 @@ set have_psyms [expr ! ( $have_gdb_index || $readnow_p )]
>>   # guo: on linux this command output is huge.  for some reason splitting up
>>   # the regexp checks works.
>>   #
>> -send_gdb "maint check-psymtabs\n"
>> -gdb_expect  {
>> -    -re "^maint check-psymtabs" {
>> -    gdb_expect {
>> -        -re "$gdb_prompt $" {
>> -        pass "maint check-psymtabs"
>> -        }
>> -        timeout { fail "(timeout) maint check-psymtabs" }
>> -    }
>> +gdb_test_multiple "maint check-psymtabs" "maint check-psymtabs" {
>> +    -re -wrap "^maint check-psymtabs.*" {
>> +    pass "maint check-psymtabs"
>>       }
>> -    -re ".*$gdb_prompt $"     { fail "maint check-psymtabs" }
>> -    timeout         { fail "(timeout) maint check-psymtabs" }
>>   }
>>   # This command does not produce any output unless there is some problem
>> @@ -270,62 +262,38 @@ if { $have_psyms } {
>>                  "maint print psymbols -pc" \
>>                  "maint print psymbols -pc main $psymbols_output"]
>>       foreach { test_name command } $test_list {
>> -    send_gdb "$command\n"
>> -        gdb_expect  {
>> -        -re "^maint print psymbols \[^\n\]*\r\n$gdb_prompt $" {
>> -            send_gdb "shell ls $psymbols_output\n"
>> -            gdb_expect {
>> -            -re "$psymbols_output_re\r\n$gdb_prompt $" {
>> -                # We want this grep to be as specific as possible,
>> -                # so it's less likely to match symbol file names in
>> -                # psymbols_output.  Yes, this actually happened;
>> -                # poor expect got tons of output, and timed out
>> -                # trying to match it.   --- Jim Blandy <jimb@cygnus.com>
>> -                send_gdb "shell grep 'main.*function' $psymbols_output\n"
>> -                gdb_expect {
>> -                -re ".main., function, $hex.*$gdb_prompt $" {
>> -                    pass "$test_name 1"
>> -                }
>> -                -re ".*main.  .., function, $hex.*$gdb_prompt $" {
>> -                    pass "$test_name 2"
>> -                }
>> -                -re ".*$gdb_prompt $" { fail "$test_name" }
>> -                timeout { fail "$test_name (timeout)" }
>> -                }
>> +    gdb_test_multiple "$command" "$test_name" {
>> +        -re  -wrap "^maint print psymbols \[^\n\]*" {
>> +        gdb_test_multiple "shell grep 'main.*function' $psymbols_output"\
>> +            "$test_name internal"{
>> +            -re -wrap ".main., function, $hex.*" {
>> +                gdb_test "shell rm -f $psymbols_output" ".*" \
>> +                "${test_name}: shell rm -f psymbols_output"
>> +                pass "$test_name 1"
>> +            }
>> +            -re -wrap ".*main.  .., function, $hex.*" {
>>                   gdb_test "shell rm -f $psymbols_output" ".*" \
>>                   "${test_name}: shell rm -f psymbols_output"
>> +                pass "$test_name 2"
>>               }
>> -            -re ".*$gdb_prompt $" { fail "$test_name" }
>> -            timeout { fail "$test_name (timeout)" }
>> -            }
>>           }
>> -        -re ".*$gdb_prompt $" { fail "$test_name" }
>> -        timeout { fail "$test_name (timeout)" }
>>           }
>> +    }
>>       }
>>   }
>>   set msymbols_output [standard_output_file msymbols_output]
>>   set msymbols_output_re [string_to_regexp $msymbols_output]
>> -send_gdb "maint print msymbols -objfile ${binfile} $msymbols_output\n"
>> -gdb_expect  {
>> -    -re "^maint print msymbols \[^\n\]*\r\n$gdb_prompt $" {
>> -    send_gdb "shell ls $msymbols_output\n"
>> -    gdb_expect {
>> -        -re "$msymbols_output_re\r\n$gdb_prompt $" {
>> -        gdb_test "shell grep factorial $msymbols_output" \
>> -            "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
>> -            "maint print msymbols, absolute pathname"
>> -        gdb_test "shell rm -f $msymbols_output" ".*" \
>> -            "shell rm -f msymbols_output"
>> -        }
>> -        -re ".*$gdb_prompt $" { fail "maint print msymbols" }
>> -        timeout { fail "maint print msymbols (timeout)" }
>> -    }
>> +gdb_test_multiple "maint print msymbols -objfile ${binfile} $msymbols_output" \
>> +"printing msymbols" {
>> +    -re -wrap "^maint print msymbols \[^\n\]*" {
>> +    gdb_test "shell grep factorial $msymbols_output" \
>> +        "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
>> +        "maint print msymbols, absolute pathname"
>> +    gdb_test "shell rm -f $msymbols_output" ".*" \
>> +        "shell rm -f msymbols_output"
>>       }
>> -    -re ".*$gdb_prompt $" { fail "maint print msymbols" }
>> -    timeout { fail "maint print msymbols (timeout)" }
>>   }
>>   # Check that maint print msymbols allows relative pathnames
>> @@ -363,31 +331,12 @@ set test_list [list \
>>              "maint print symbols -pc" \
>>              "maint print symbols -pc main $symbols_output"]
>>   foreach { test_name command } $test_list {
>> -    send_gdb "$command\n"
>> -    gdb_expect {
>> -    -re "^maint print symbols \[^\n\]*\r\n$gdb_prompt $" {
>> -        send_gdb "shell ls $symbols_output\n"
>> -        gdb_expect {
>> -        -re "$symbols_output_re\r\n$gdb_prompt $" {
>> -            # See comments for `maint print psymbols'.
>> -            send_gdb "shell grep 'main(.*block' $symbols_output\n"
>> -            gdb_expect {
>> -            -re "int main\\(int, char \\*\\*, char \\*\\*\\); block.*$gdb_prompt $" {
>> -                pass "$test_name"
>> -            }
>> -            -re ".*$gdb_prompt $" { fail "$test_name" }
>> -            timeout { fail "$test_name (timeout)" }
>> -            }
>> -            gdb_test "shell rm -f $symbols_output" ".*" \
>> -            "$test_name: shell rm -f symbols_output"
>> -        }
>> -        -re ".*$gdb_prompt $" { fail "$test_name" }
>> -        timeout { fail "$test_name (timeout)" }
>> -        }
>> -    }
>> -    -re ".*$gdb_prompt $" { fail "$test_name" }
>> -    timeout { fail "$test_name (timeout)" }
>> -    }
>> +    gdb_test_no_output "$command" "$test_name generate"
>> +    gdb_test "shell grep 'main(.*block' $symbols_output"\
>> +    "int main\\(int, char \\*\\*, char \\*\\*\\); block.*"\
>> +    "$test_name read"
>> +    gdb_test "shell rm -f $symbols_output" ".*" \
>> +    "$test_name: shell rm -f symbols_output"
>>   }
>>   set msg "maint print type"


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PINGv3] [PATCH] gdb/testsuite: modernize gdb.base/maint.exp
  2022-05-30 11:14   ` [PINGv2] " Bruno Larsen
@ 2022-06-06 12:43     ` Bruno Larsen
  2022-06-13 20:03       ` [PINGv4] " Bruno Larsen
  0 siblings, 1 reply; 25+ messages in thread
From: Bruno Larsen @ 2022-06-06 12:43 UTC (permalink / raw)
  To: gdb-patches

ping!

Cheers!
Bruno Larsen

On 5/30/22 08:14, Bruno Larsen wrote:
> ping!
> 
> Cheers!
> Bruno Larsen
> 
> On 5/23/22 11:14, Bruno Larsen wrote:
>> ping
>>
>> Cheers!
>> Bruno Larsen
>>
>> On 5/9/22 15:04, Bruno Larsen wrote:
>>> gdb.base/maint.exp was using several gdb_expect statements, probably
>>> because this test case predates the existance of gdb_test_multiple. This
>>> commit updates the test case to use gdb_test_multiple, making it more
>>> resilient to internal errors and such.
>>>
>>> The only gdb_expect left in the testcase is one that specifically looks
>>> for an internal error being triggered as a PASS.
>>> ---
>>>   gdb/testsuite/gdb.base/maint.exp | 109 ++++++++-----------------------
>>>   1 file changed, 29 insertions(+), 80 deletions(-)
>>>
>>> diff --git a/gdb/testsuite/gdb.base/maint.exp b/gdb/testsuite/gdb.base/maint.exp
>>> index 45ccdc6584e..2efdda9aed7 100644
>>> --- a/gdb/testsuite/gdb.base/maint.exp
>>> +++ b/gdb/testsuite/gdb.base/maint.exp
>>> @@ -155,18 +155,10 @@ set have_psyms [expr ! ( $have_gdb_index || $readnow_p )]
>>>   # guo: on linux this command output is huge.  for some reason splitting up
>>>   # the regexp checks works.
>>>   #
>>> -send_gdb "maint check-psymtabs\n"
>>> -gdb_expect  {
>>> -    -re "^maint check-psymtabs" {
>>> -    gdb_expect {
>>> -        -re "$gdb_prompt $" {
>>> -        pass "maint check-psymtabs"
>>> -        }
>>> -        timeout { fail "(timeout) maint check-psymtabs" }
>>> -    }
>>> +gdb_test_multiple "maint check-psymtabs" "maint check-psymtabs" {
>>> +    -re -wrap "^maint check-psymtabs.*" {
>>> +    pass "maint check-psymtabs"
>>>       }
>>> -    -re ".*$gdb_prompt $"     { fail "maint check-psymtabs" }
>>> -    timeout         { fail "(timeout) maint check-psymtabs" }
>>>   }
>>>   # This command does not produce any output unless there is some problem
>>> @@ -270,62 +262,38 @@ if { $have_psyms } {
>>>                  "maint print psymbols -pc" \
>>>                  "maint print psymbols -pc main $psymbols_output"]
>>>       foreach { test_name command } $test_list {
>>> -    send_gdb "$command\n"
>>> -        gdb_expect  {
>>> -        -re "^maint print psymbols \[^\n\]*\r\n$gdb_prompt $" {
>>> -            send_gdb "shell ls $psymbols_output\n"
>>> -            gdb_expect {
>>> -            -re "$psymbols_output_re\r\n$gdb_prompt $" {
>>> -                # We want this grep to be as specific as possible,
>>> -                # so it's less likely to match symbol file names in
>>> -                # psymbols_output.  Yes, this actually happened;
>>> -                # poor expect got tons of output, and timed out
>>> -                # trying to match it.   --- Jim Blandy <jimb@cygnus.com>
>>> -                send_gdb "shell grep 'main.*function' $psymbols_output\n"
>>> -                gdb_expect {
>>> -                -re ".main., function, $hex.*$gdb_prompt $" {
>>> -                    pass "$test_name 1"
>>> -                }
>>> -                -re ".*main.  .., function, $hex.*$gdb_prompt $" {
>>> -                    pass "$test_name 2"
>>> -                }
>>> -                -re ".*$gdb_prompt $" { fail "$test_name" }
>>> -                timeout { fail "$test_name (timeout)" }
>>> -                }
>>> +    gdb_test_multiple "$command" "$test_name" {
>>> +        -re  -wrap "^maint print psymbols \[^\n\]*" {
>>> +        gdb_test_multiple "shell grep 'main.*function' $psymbols_output"\
>>> +            "$test_name internal"{
>>> +            -re -wrap ".main., function, $hex.*" {
>>> +                gdb_test "shell rm -f $psymbols_output" ".*" \
>>> +                "${test_name}: shell rm -f psymbols_output"
>>> +                pass "$test_name 1"
>>> +            }
>>> +            -re -wrap ".*main.  .., function, $hex.*" {
>>>                   gdb_test "shell rm -f $psymbols_output" ".*" \
>>>                   "${test_name}: shell rm -f psymbols_output"
>>> +                pass "$test_name 2"
>>>               }
>>> -            -re ".*$gdb_prompt $" { fail "$test_name" }
>>> -            timeout { fail "$test_name (timeout)" }
>>> -            }
>>>           }
>>> -        -re ".*$gdb_prompt $" { fail "$test_name" }
>>> -        timeout { fail "$test_name (timeout)" }
>>>           }
>>> +    }
>>>       }
>>>   }
>>>   set msymbols_output [standard_output_file msymbols_output]
>>>   set msymbols_output_re [string_to_regexp $msymbols_output]
>>> -send_gdb "maint print msymbols -objfile ${binfile} $msymbols_output\n"
>>> -gdb_expect  {
>>> -    -re "^maint print msymbols \[^\n\]*\r\n$gdb_prompt $" {
>>> -    send_gdb "shell ls $msymbols_output\n"
>>> -    gdb_expect {
>>> -        -re "$msymbols_output_re\r\n$gdb_prompt $" {
>>> -        gdb_test "shell grep factorial $msymbols_output" \
>>> -            "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
>>> -            "maint print msymbols, absolute pathname"
>>> -        gdb_test "shell rm -f $msymbols_output" ".*" \
>>> -            "shell rm -f msymbols_output"
>>> -        }
>>> -        -re ".*$gdb_prompt $" { fail "maint print msymbols" }
>>> -        timeout { fail "maint print msymbols (timeout)" }
>>> -    }
>>> +gdb_test_multiple "maint print msymbols -objfile ${binfile} $msymbols_output" \
>>> +"printing msymbols" {
>>> +    -re -wrap "^maint print msymbols \[^\n\]*" {
>>> +    gdb_test "shell grep factorial $msymbols_output" \
>>> +        "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
>>> +        "maint print msymbols, absolute pathname"
>>> +    gdb_test "shell rm -f $msymbols_output" ".*" \
>>> +        "shell rm -f msymbols_output"
>>>       }
>>> -    -re ".*$gdb_prompt $" { fail "maint print msymbols" }
>>> -    timeout { fail "maint print msymbols (timeout)" }
>>>   }
>>>   # Check that maint print msymbols allows relative pathnames
>>> @@ -363,31 +331,12 @@ set test_list [list \
>>>              "maint print symbols -pc" \
>>>              "maint print symbols -pc main $symbols_output"]
>>>   foreach { test_name command } $test_list {
>>> -    send_gdb "$command\n"
>>> -    gdb_expect {
>>> -    -re "^maint print symbols \[^\n\]*\r\n$gdb_prompt $" {
>>> -        send_gdb "shell ls $symbols_output\n"
>>> -        gdb_expect {
>>> -        -re "$symbols_output_re\r\n$gdb_prompt $" {
>>> -            # See comments for `maint print psymbols'.
>>> -            send_gdb "shell grep 'main(.*block' $symbols_output\n"
>>> -            gdb_expect {
>>> -            -re "int main\\(int, char \\*\\*, char \\*\\*\\); block.*$gdb_prompt $" {
>>> -                pass "$test_name"
>>> -            }
>>> -            -re ".*$gdb_prompt $" { fail "$test_name" }
>>> -            timeout { fail "$test_name (timeout)" }
>>> -            }
>>> -            gdb_test "shell rm -f $symbols_output" ".*" \
>>> -            "$test_name: shell rm -f symbols_output"
>>> -        }
>>> -        -re ".*$gdb_prompt $" { fail "$test_name" }
>>> -        timeout { fail "$test_name (timeout)" }
>>> -        }
>>> -    }
>>> -    -re ".*$gdb_prompt $" { fail "$test_name" }
>>> -    timeout { fail "$test_name (timeout)" }
>>> -    }
>>> +    gdb_test_no_output "$command" "$test_name generate"
>>> +    gdb_test "shell grep 'main(.*block' $symbols_output"\
>>> +    "int main\\(int, char \\*\\*, char \\*\\*\\); block.*"\
>>> +    "$test_name read"
>>> +    gdb_test "shell rm -f $symbols_output" ".*" \
>>> +    "$test_name: shell rm -f symbols_output"
>>>   }
>>>   set msg "maint print type"


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PINGv4] [PATCH] gdb/testsuite: modernize gdb.base/maint.exp
  2022-06-06 12:43     ` [PINGv3] " Bruno Larsen
@ 2022-06-13 20:03       ` Bruno Larsen
  2022-06-20 13:29         ` [PINGv5] " Bruno Larsen
  0 siblings, 1 reply; 25+ messages in thread
From: Bruno Larsen @ 2022-06-13 20:03 UTC (permalink / raw)
  To: gdb-patches

ping!

Cheers!
Bruno Larsen

On 6/6/22 09:43, Bruno Larsen wrote:
> ping!
> 
> Cheers!
> Bruno Larsen
> 
> On 5/30/22 08:14, Bruno Larsen wrote:
>> ping!
>>
>> Cheers!
>> Bruno Larsen
>>
>> On 5/23/22 11:14, Bruno Larsen wrote:
>>> ping
>>>
>>> Cheers!
>>> Bruno Larsen
>>>
>>> On 5/9/22 15:04, Bruno Larsen wrote:
>>>> gdb.base/maint.exp was using several gdb_expect statements, probably
>>>> because this test case predates the existance of gdb_test_multiple. This
>>>> commit updates the test case to use gdb_test_multiple, making it more
>>>> resilient to internal errors and such.
>>>>
>>>> The only gdb_expect left in the testcase is one that specifically looks
>>>> for an internal error being triggered as a PASS.
>>>> ---
>>>>   gdb/testsuite/gdb.base/maint.exp | 109 ++++++++-----------------------
>>>>   1 file changed, 29 insertions(+), 80 deletions(-)
>>>>
>>>> diff --git a/gdb/testsuite/gdb.base/maint.exp b/gdb/testsuite/gdb.base/maint.exp
>>>> index 45ccdc6584e..2efdda9aed7 100644
>>>> --- a/gdb/testsuite/gdb.base/maint.exp
>>>> +++ b/gdb/testsuite/gdb.base/maint.exp
>>>> @@ -155,18 +155,10 @@ set have_psyms [expr ! ( $have_gdb_index || $readnow_p )]
>>>>   # guo: on linux this command output is huge.  for some reason splitting up
>>>>   # the regexp checks works.
>>>>   #
>>>> -send_gdb "maint check-psymtabs\n"
>>>> -gdb_expect  {
>>>> -    -re "^maint check-psymtabs" {
>>>> -    gdb_expect {
>>>> -        -re "$gdb_prompt $" {
>>>> -        pass "maint check-psymtabs"
>>>> -        }
>>>> -        timeout { fail "(timeout) maint check-psymtabs" }
>>>> -    }
>>>> +gdb_test_multiple "maint check-psymtabs" "maint check-psymtabs" {
>>>> +    -re -wrap "^maint check-psymtabs.*" {
>>>> +    pass "maint check-psymtabs"
>>>>       }
>>>> -    -re ".*$gdb_prompt $"     { fail "maint check-psymtabs" }
>>>> -    timeout         { fail "(timeout) maint check-psymtabs" }
>>>>   }
>>>>   # This command does not produce any output unless there is some problem
>>>> @@ -270,62 +262,38 @@ if { $have_psyms } {
>>>>                  "maint print psymbols -pc" \
>>>>                  "maint print psymbols -pc main $psymbols_output"]
>>>>       foreach { test_name command } $test_list {
>>>> -    send_gdb "$command\n"
>>>> -        gdb_expect  {
>>>> -        -re "^maint print psymbols \[^\n\]*\r\n$gdb_prompt $" {
>>>> -            send_gdb "shell ls $psymbols_output\n"
>>>> -            gdb_expect {
>>>> -            -re "$psymbols_output_re\r\n$gdb_prompt $" {
>>>> -                # We want this grep to be as specific as possible,
>>>> -                # so it's less likely to match symbol file names in
>>>> -                # psymbols_output.  Yes, this actually happened;
>>>> -                # poor expect got tons of output, and timed out
>>>> -                # trying to match it.   --- Jim Blandy <jimb@cygnus.com>
>>>> -                send_gdb "shell grep 'main.*function' $psymbols_output\n"
>>>> -                gdb_expect {
>>>> -                -re ".main., function, $hex.*$gdb_prompt $" {
>>>> -                    pass "$test_name 1"
>>>> -                }
>>>> -                -re ".*main.  .., function, $hex.*$gdb_prompt $" {
>>>> -                    pass "$test_name 2"
>>>> -                }
>>>> -                -re ".*$gdb_prompt $" { fail "$test_name" }
>>>> -                timeout { fail "$test_name (timeout)" }
>>>> -                }
>>>> +    gdb_test_multiple "$command" "$test_name" {
>>>> +        -re  -wrap "^maint print psymbols \[^\n\]*" {
>>>> +        gdb_test_multiple "shell grep 'main.*function' $psymbols_output"\
>>>> +            "$test_name internal"{
>>>> +            -re -wrap ".main., function, $hex.*" {
>>>> +                gdb_test "shell rm -f $psymbols_output" ".*" \
>>>> +                "${test_name}: shell rm -f psymbols_output"
>>>> +                pass "$test_name 1"
>>>> +            }
>>>> +            -re -wrap ".*main.  .., function, $hex.*" {
>>>>                   gdb_test "shell rm -f $psymbols_output" ".*" \
>>>>                   "${test_name}: shell rm -f psymbols_output"
>>>> +                pass "$test_name 2"
>>>>               }
>>>> -            -re ".*$gdb_prompt $" { fail "$test_name" }
>>>> -            timeout { fail "$test_name (timeout)" }
>>>> -            }
>>>>           }
>>>> -        -re ".*$gdb_prompt $" { fail "$test_name" }
>>>> -        timeout { fail "$test_name (timeout)" }
>>>>           }
>>>> +    }
>>>>       }
>>>>   }
>>>>   set msymbols_output [standard_output_file msymbols_output]
>>>>   set msymbols_output_re [string_to_regexp $msymbols_output]
>>>> -send_gdb "maint print msymbols -objfile ${binfile} $msymbols_output\n"
>>>> -gdb_expect  {
>>>> -    -re "^maint print msymbols \[^\n\]*\r\n$gdb_prompt $" {
>>>> -    send_gdb "shell ls $msymbols_output\n"
>>>> -    gdb_expect {
>>>> -        -re "$msymbols_output_re\r\n$gdb_prompt $" {
>>>> -        gdb_test "shell grep factorial $msymbols_output" \
>>>> -            "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
>>>> -            "maint print msymbols, absolute pathname"
>>>> -        gdb_test "shell rm -f $msymbols_output" ".*" \
>>>> -            "shell rm -f msymbols_output"
>>>> -        }
>>>> -        -re ".*$gdb_prompt $" { fail "maint print msymbols" }
>>>> -        timeout { fail "maint print msymbols (timeout)" }
>>>> -    }
>>>> +gdb_test_multiple "maint print msymbols -objfile ${binfile} $msymbols_output" \
>>>> +"printing msymbols" {
>>>> +    -re -wrap "^maint print msymbols \[^\n\]*" {
>>>> +    gdb_test "shell grep factorial $msymbols_output" \
>>>> +        "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
>>>> +        "maint print msymbols, absolute pathname"
>>>> +    gdb_test "shell rm -f $msymbols_output" ".*" \
>>>> +        "shell rm -f msymbols_output"
>>>>       }
>>>> -    -re ".*$gdb_prompt $" { fail "maint print msymbols" }
>>>> -    timeout { fail "maint print msymbols (timeout)" }
>>>>   }
>>>>   # Check that maint print msymbols allows relative pathnames
>>>> @@ -363,31 +331,12 @@ set test_list [list \
>>>>              "maint print symbols -pc" \
>>>>              "maint print symbols -pc main $symbols_output"]
>>>>   foreach { test_name command } $test_list {
>>>> -    send_gdb "$command\n"
>>>> -    gdb_expect {
>>>> -    -re "^maint print symbols \[^\n\]*\r\n$gdb_prompt $" {
>>>> -        send_gdb "shell ls $symbols_output\n"
>>>> -        gdb_expect {
>>>> -        -re "$symbols_output_re\r\n$gdb_prompt $" {
>>>> -            # See comments for `maint print psymbols'.
>>>> -            send_gdb "shell grep 'main(.*block' $symbols_output\n"
>>>> -            gdb_expect {
>>>> -            -re "int main\\(int, char \\*\\*, char \\*\\*\\); block.*$gdb_prompt $" {
>>>> -                pass "$test_name"
>>>> -            }
>>>> -            -re ".*$gdb_prompt $" { fail "$test_name" }
>>>> -            timeout { fail "$test_name (timeout)" }
>>>> -            }
>>>> -            gdb_test "shell rm -f $symbols_output" ".*" \
>>>> -            "$test_name: shell rm -f symbols_output"
>>>> -        }
>>>> -        -re ".*$gdb_prompt $" { fail "$test_name" }
>>>> -        timeout { fail "$test_name (timeout)" }
>>>> -        }
>>>> -    }
>>>> -    -re ".*$gdb_prompt $" { fail "$test_name" }
>>>> -    timeout { fail "$test_name (timeout)" }
>>>> -    }
>>>> +    gdb_test_no_output "$command" "$test_name generate"
>>>> +    gdb_test "shell grep 'main(.*block' $symbols_output"\
>>>> +    "int main\\(int, char \\*\\*, char \\*\\*\\); block.*"\
>>>> +    "$test_name read"
>>>> +    gdb_test "shell rm -f $symbols_output" ".*" \
>>>> +    "$test_name: shell rm -f symbols_output"
>>>>   }
>>>>   set msg "maint print type"


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PINGv5] [PATCH] gdb/testsuite: modernize gdb.base/maint.exp
  2022-06-13 20:03       ` [PINGv4] " Bruno Larsen
@ 2022-06-20 13:29         ` Bruno Larsen
  0 siblings, 0 replies; 25+ messages in thread
From: Bruno Larsen @ 2022-06-20 13:29 UTC (permalink / raw)
  To: gdb-patches

ping!

Cheers!
Bruno Larsen

On 6/13/22 17:03, Bruno Larsen wrote:
> ping!
> 
> Cheers!
> Bruno Larsen
> 
> On 6/6/22 09:43, Bruno Larsen wrote:
>> ping!
>>
>> Cheers!
>> Bruno Larsen
>>
>> On 5/30/22 08:14, Bruno Larsen wrote:
>>> ping!
>>>
>>> Cheers!
>>> Bruno Larsen
>>>
>>> On 5/23/22 11:14, Bruno Larsen wrote:
>>>> ping
>>>>
>>>> Cheers!
>>>> Bruno Larsen
>>>>
>>>> On 5/9/22 15:04, Bruno Larsen wrote:
>>>>> gdb.base/maint.exp was using several gdb_expect statements, probably
>>>>> because this test case predates the existance of gdb_test_multiple. This
>>>>> commit updates the test case to use gdb_test_multiple, making it more
>>>>> resilient to internal errors and such.
>>>>>
>>>>> The only gdb_expect left in the testcase is one that specifically looks
>>>>> for an internal error being triggered as a PASS.
>>>>> ---
>>>>>   gdb/testsuite/gdb.base/maint.exp | 109 ++++++++-----------------------
>>>>>   1 file changed, 29 insertions(+), 80 deletions(-)
>>>>>
>>>>> diff --git a/gdb/testsuite/gdb.base/maint.exp b/gdb/testsuite/gdb.base/maint.exp
>>>>> index 45ccdc6584e..2efdda9aed7 100644
>>>>> --- a/gdb/testsuite/gdb.base/maint.exp
>>>>> +++ b/gdb/testsuite/gdb.base/maint.exp
>>>>> @@ -155,18 +155,10 @@ set have_psyms [expr ! ( $have_gdb_index || $readnow_p )]
>>>>>   # guo: on linux this command output is huge.  for some reason splitting up
>>>>>   # the regexp checks works.
>>>>>   #
>>>>> -send_gdb "maint check-psymtabs\n"
>>>>> -gdb_expect  {
>>>>> -    -re "^maint check-psymtabs" {
>>>>> -    gdb_expect {
>>>>> -        -re "$gdb_prompt $" {
>>>>> -        pass "maint check-psymtabs"
>>>>> -        }
>>>>> -        timeout { fail "(timeout) maint check-psymtabs" }
>>>>> -    }
>>>>> +gdb_test_multiple "maint check-psymtabs" "maint check-psymtabs" {
>>>>> +    -re -wrap "^maint check-psymtabs.*" {
>>>>> +    pass "maint check-psymtabs"
>>>>>       }
>>>>> -    -re ".*$gdb_prompt $"     { fail "maint check-psymtabs" }
>>>>> -    timeout         { fail "(timeout) maint check-psymtabs" }
>>>>>   }
>>>>>   # This command does not produce any output unless there is some problem
>>>>> @@ -270,62 +262,38 @@ if { $have_psyms } {
>>>>>                  "maint print psymbols -pc" \
>>>>>                  "maint print psymbols -pc main $psymbols_output"]
>>>>>       foreach { test_name command } $test_list {
>>>>> -    send_gdb "$command\n"
>>>>> -        gdb_expect  {
>>>>> -        -re "^maint print psymbols \[^\n\]*\r\n$gdb_prompt $" {
>>>>> -            send_gdb "shell ls $psymbols_output\n"
>>>>> -            gdb_expect {
>>>>> -            -re "$psymbols_output_re\r\n$gdb_prompt $" {
>>>>> -                # We want this grep to be as specific as possible,
>>>>> -                # so it's less likely to match symbol file names in
>>>>> -                # psymbols_output.  Yes, this actually happened;
>>>>> -                # poor expect got tons of output, and timed out
>>>>> -                # trying to match it.   --- Jim Blandy <jimb@cygnus.com>
>>>>> -                send_gdb "shell grep 'main.*function' $psymbols_output\n"
>>>>> -                gdb_expect {
>>>>> -                -re ".main., function, $hex.*$gdb_prompt $" {
>>>>> -                    pass "$test_name 1"
>>>>> -                }
>>>>> -                -re ".*main.  .., function, $hex.*$gdb_prompt $" {
>>>>> -                    pass "$test_name 2"
>>>>> -                }
>>>>> -                -re ".*$gdb_prompt $" { fail "$test_name" }
>>>>> -                timeout { fail "$test_name (timeout)" }
>>>>> -                }
>>>>> +    gdb_test_multiple "$command" "$test_name" {
>>>>> +        -re  -wrap "^maint print psymbols \[^\n\]*" {
>>>>> +        gdb_test_multiple "shell grep 'main.*function' $psymbols_output"\
>>>>> +            "$test_name internal"{
>>>>> +            -re -wrap ".main., function, $hex.*" {
>>>>> +                gdb_test "shell rm -f $psymbols_output" ".*" \
>>>>> +                "${test_name}: shell rm -f psymbols_output"
>>>>> +                pass "$test_name 1"
>>>>> +            }
>>>>> +            -re -wrap ".*main.  .., function, $hex.*" {
>>>>>                   gdb_test "shell rm -f $psymbols_output" ".*" \
>>>>>                   "${test_name}: shell rm -f psymbols_output"
>>>>> +                pass "$test_name 2"
>>>>>               }
>>>>> -            -re ".*$gdb_prompt $" { fail "$test_name" }
>>>>> -            timeout { fail "$test_name (timeout)" }
>>>>> -            }
>>>>>           }
>>>>> -        -re ".*$gdb_prompt $" { fail "$test_name" }
>>>>> -        timeout { fail "$test_name (timeout)" }
>>>>>           }
>>>>> +    }
>>>>>       }
>>>>>   }
>>>>>   set msymbols_output [standard_output_file msymbols_output]
>>>>>   set msymbols_output_re [string_to_regexp $msymbols_output]
>>>>> -send_gdb "maint print msymbols -objfile ${binfile} $msymbols_output\n"
>>>>> -gdb_expect  {
>>>>> -    -re "^maint print msymbols \[^\n\]*\r\n$gdb_prompt $" {
>>>>> -    send_gdb "shell ls $msymbols_output\n"
>>>>> -    gdb_expect {
>>>>> -        -re "$msymbols_output_re\r\n$gdb_prompt $" {
>>>>> -        gdb_test "shell grep factorial $msymbols_output" \
>>>>> -            "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
>>>>> -            "maint print msymbols, absolute pathname"
>>>>> -        gdb_test "shell rm -f $msymbols_output" ".*" \
>>>>> -            "shell rm -f msymbols_output"
>>>>> -        }
>>>>> -        -re ".*$gdb_prompt $" { fail "maint print msymbols" }
>>>>> -        timeout { fail "maint print msymbols (timeout)" }
>>>>> -    }
>>>>> +gdb_test_multiple "maint print msymbols -objfile ${binfile} $msymbols_output" \
>>>>> +"printing msymbols" {
>>>>> +    -re -wrap "^maint print msymbols \[^\n\]*" {
>>>>> +    gdb_test "shell grep factorial $msymbols_output" \
>>>>> +        "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
>>>>> +        "maint print msymbols, absolute pathname"
>>>>> +    gdb_test "shell rm -f $msymbols_output" ".*" \
>>>>> +        "shell rm -f msymbols_output"
>>>>>       }
>>>>> -    -re ".*$gdb_prompt $" { fail "maint print msymbols" }
>>>>> -    timeout { fail "maint print msymbols (timeout)" }
>>>>>   }
>>>>>   # Check that maint print msymbols allows relative pathnames
>>>>> @@ -363,31 +331,12 @@ set test_list [list \
>>>>>              "maint print symbols -pc" \
>>>>>              "maint print symbols -pc main $symbols_output"]
>>>>>   foreach { test_name command } $test_list {
>>>>> -    send_gdb "$command\n"
>>>>> -    gdb_expect {
>>>>> -    -re "^maint print symbols \[^\n\]*\r\n$gdb_prompt $" {
>>>>> -        send_gdb "shell ls $symbols_output\n"
>>>>> -        gdb_expect {
>>>>> -        -re "$symbols_output_re\r\n$gdb_prompt $" {
>>>>> -            # See comments for `maint print psymbols'.
>>>>> -            send_gdb "shell grep 'main(.*block' $symbols_output\n"
>>>>> -            gdb_expect {
>>>>> -            -re "int main\\(int, char \\*\\*, char \\*\\*\\); block.*$gdb_prompt $" {
>>>>> -                pass "$test_name"
>>>>> -            }
>>>>> -            -re ".*$gdb_prompt $" { fail "$test_name" }
>>>>> -            timeout { fail "$test_name (timeout)" }
>>>>> -            }
>>>>> -            gdb_test "shell rm -f $symbols_output" ".*" \
>>>>> -            "$test_name: shell rm -f symbols_output"
>>>>> -        }
>>>>> -        -re ".*$gdb_prompt $" { fail "$test_name" }
>>>>> -        timeout { fail "$test_name (timeout)" }
>>>>> -        }
>>>>> -    }
>>>>> -    -re ".*$gdb_prompt $" { fail "$test_name" }
>>>>> -    timeout { fail "$test_name (timeout)" }
>>>>> -    }
>>>>> +    gdb_test_no_output "$command" "$test_name generate"
>>>>> +    gdb_test "shell grep 'main(.*block' $symbols_output"\
>>>>> +    "int main\\(int, char \\*\\*, char \\*\\*\\); block.*"\
>>>>> +    "$test_name read"
>>>>> +    gdb_test "shell rm -f $symbols_output" ".*" \
>>>>> +    "$test_name: shell rm -f symbols_output"
>>>>>   }
>>>>>   set msg "maint print type"


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] gdb/testsuite: modernize gdb.base/maint.exp
  2022-05-09 18:04 [PATCH] gdb/testsuite: modernize gdb.base/maint.exp Bruno Larsen
  2022-05-23 14:14 ` [PING] " Bruno Larsen
@ 2022-06-21 15:52 ` Andrew Burgess
  2022-06-21 19:45   ` Bruno Larsen
  2022-06-24 15:22   ` Pedro Alves
  2022-06-28 18:36 ` [PATCHv3] " Bruno Larsen
  2022-06-29 14:53 ` [PATCHv4] " Bruno Larsen
  3 siblings, 2 replies; 25+ messages in thread
From: Andrew Burgess @ 2022-06-21 15:52 UTC (permalink / raw)
  To: Bruno Larsen, gdb-patches

Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:

> gdb.base/maint.exp was using several gdb_expect statements, probably
> because this test case predates the existance of gdb_test_multiple. This
> commit updates the test case to use gdb_test_multiple, making it more
> resilient to internal errors and such.
>
> The only gdb_expect left in the testcase is one that specifically looks
> for an internal error being triggered as a PASS.
> ---
>  gdb/testsuite/gdb.base/maint.exp | 109 ++++++++-----------------------
>  1 file changed, 29 insertions(+), 80 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.base/maint.exp b/gdb/testsuite/gdb.base/maint.exp
> index 45ccdc6584e..2efdda9aed7 100644
> --- a/gdb/testsuite/gdb.base/maint.exp
> +++ b/gdb/testsuite/gdb.base/maint.exp
> @@ -155,18 +155,10 @@ set have_psyms [expr ! ( $have_gdb_index || $readnow_p )]
>  # guo: on linux this command output is huge.  for some reason splitting up
>  # the regexp checks works.

I think this comment should either be deleted, or rewritten.

Plus, this comment seems to directly contradict the preceeding comment
that says:

  # this command does not produce any output
  # unless there is some problem with the symtabs and psymtabs
  # so that branch will really never be covered in this tests here!!

Which is it?  The command produces no output, or it produces a huge
amount of output?

Turns out, the first comment is correct, there's no output, at least on
my Linux machine.

Looking at maintenance_check_psymtabs, I guess there could, potentially,
be lots of output, if there is a lot of problems with the generated file
that we're debugging for this test, so maybe when the code below, that
you're changing, was added, some targets were seeing such problems?

If we assume that there are still targets out there that do have errors
then we should probably write a comment that's not so contradictory...

>  #
> -send_gdb "maint check-psymtabs\n"
> -gdb_expect  {
> -    -re "^maint check-psymtabs" {
> -	gdb_expect {
> -	    -re "$gdb_prompt $" {
> -		pass "maint check-psymtabs"
> -	    }
> -	    timeout { fail "(timeout) maint check-psymtabs" }
> -	}
> +gdb_test_multiple "maint check-psymtabs" "maint check-psymtabs" {

You can just do:

  gdb_test_multiple "maint check-psymtabs" "" {

the message will default to the command.

> +    -re -wrap "^maint check-psymtabs.*" {

I suspect that this only works because for you, like me, 'maint
check-psymtabs' doesn't produce any output.

The original checks didn't require the entire command output to be
loaded into the expect buffer, while your updated pattern does.  The
"for some reason" that is mentioned in the 'guo' comment is almost
certainly that the expect buffer was getting full.

The patterns should probably be:

    -re "^maint check-psymtabs\r\n" {
	exp_continue
    }

    -re "^$gdb_prompt $" {
	pass $gdb_test_name
    }

    -re "^\[^\r\n\]+\r\n" {
	exp_continue
    }

This will allow the command to run, and discard any lines of output that
are not the prompt.

We could make this test stricter by changing the last pattern to match
all possible lines that maintenance_check_psymtabs can produce (I think
there's only 4 or 5).

We could also make the test stricter by ensuring that the initial 'maint
check-psymtabs' line is seen.

> +	pass "maint check-psymtabs"

You can do:

  pass $gdb_test_name

the '$gdb_test_name' is automatically created based on the value of the
message by gdb_test_multiple.

>      }
> -    -re ".*$gdb_prompt $"     { fail "maint check-psymtabs" }
> -    timeout         { fail "(timeout) maint check-psymtabs" }
>  }



>  
>  # This command does not produce any output unless there is some problem
> @@ -270,62 +262,38 @@ if { $have_psyms } {
>  		       "maint print psymbols -pc" \
>  		       "maint print psymbols -pc main $psymbols_output"]
>      foreach { test_name command } $test_list {
> -	send_gdb "$command\n"
> -	    gdb_expect  {
> -		-re "^maint print psymbols \[^\n\]*\r\n$gdb_prompt $" {
> -		    send_gdb "shell ls $psymbols_output\n"
> -		    gdb_expect {
> -			-re "$psymbols_output_re\r\n$gdb_prompt $" {
> -			    # We want this grep to be as specific as possible,
> -			    # so it's less likely to match symbol file names in
> -			    # psymbols_output.  Yes, this actually happened;
> -			    # poor expect got tons of output, and timed out
> -			    # trying to match it.   --- Jim Blandy <jimb@cygnus.com>
> -			    send_gdb "shell grep 'main.*function' $psymbols_output\n"
> -			    gdb_expect {
> -				-re ".main., function, $hex.*$gdb_prompt $" {
> -				    pass "$test_name 1"
> -				}
> -				-re ".*main.  .., function, $hex.*$gdb_prompt $" {
> -				    pass "$test_name 2"
> -				}
> -				-re ".*$gdb_prompt $" { fail "$test_name" }
> -				timeout { fail "$test_name (timeout)" }
> -			    }
> +	gdb_test_multiple "$command" "$test_name" {
> +	    -re  -wrap "^maint print psymbols \[^\n\]*" {
> +		gdb_test_multiple "shell grep 'main.*function' $psymbols_output"\
> +		    "$test_name internal"{

I think there should be a space before the `\` and `{` at the end of the
two previous lines.  Not required, that just seems to be the GDB style.

> +			-re -wrap ".main., function, $hex.*" {
> +			    gdb_test "shell rm -f $psymbols_output" ".*" \
> +				"${test_name}: shell rm -f psymbols_output"

I wonder why the 'rm' needed to move into both of the pass blocks?  It
seemed like having it after the gdb_test_multiple would be better.

> +			    pass "$test_name 1"

You could use:

    pass "$gdb_test_name, pattern 1"

here, and similar, with ', pattern 2' for the next 'pass' call.

> +			}
> +			-re -wrap ".*main.  .., function, $hex.*" {
>  			    gdb_test "shell rm -f $psymbols_output" ".*" \
>  				"${test_name}: shell rm -f psymbols_output"
> +			    pass "$test_name 2"
>  			}
> -			-re ".*$gdb_prompt $" { fail "$test_name" }
> -			timeout { fail "$test_name (timeout)" }
> -		    }
>  		}
> -		-re ".*$gdb_prompt $" { fail "$test_name" }
> -		timeout { fail "$test_name (timeout)" }
>  	    }
> +	}
>      }
>  }
>  
>  
>  set msymbols_output [standard_output_file msymbols_output]
>  set msymbols_output_re [string_to_regexp $msymbols_output]
> -send_gdb "maint print msymbols -objfile ${binfile} $msymbols_output\n"
> -gdb_expect  {
> -    -re "^maint print msymbols \[^\n\]*\r\n$gdb_prompt $" {
> -	send_gdb "shell ls $msymbols_output\n"
> -	gdb_expect {
> -	    -re "$msymbols_output_re\r\n$gdb_prompt $" {
> -		gdb_test "shell grep factorial $msymbols_output" \
> -		    "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
> -		    "maint print msymbols, absolute pathname"
> -		gdb_test "shell rm -f $msymbols_output" ".*" \
> -		    "shell rm -f msymbols_output"
> -	    }
> -	    -re ".*$gdb_prompt $" { fail "maint print msymbols" }
> -	    timeout { fail "maint print msymbols (timeout)" }
> -	}
> +gdb_test_multiple "maint print msymbols -objfile ${binfile} $msymbols_output" \
> +"printing msymbols" {

I think this line should be indented.

> +    -re -wrap "^maint print msymbols \[^\n\]*" {
> +	gdb_test "shell grep factorial $msymbols_output" \
> +	    "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
> +	    "maint print msymbols, absolute pathname"
> +	gdb_test "shell rm -f $msymbols_output" ".*" \
> +	    "shell rm -f msymbols_output"
>      }

Couldn't these tests just be serialised, rather than being nested? e.g.:

   gdb_test_no_output "maint print msymbols -objfile ${binfile} $msymbols_output" \
       "printing msymbols"

   gdb_test "shell grep factorial $msymbols_output" \
       "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
       "maint print msymbols, absolute pathname"

   gdb_test "shell rm -f $msymbols_output" ".*" \
       "shell rm -f msymbols_output"

I haven't tested this, so maybe there's a reason why this wouldn't work.

Thanks,
Andrew



> -    -re ".*$gdb_prompt $" { fail "maint print msymbols" }
> -    timeout { fail "maint print msymbols (timeout)" }
>  }
>  
>  # Check that maint print msymbols allows relative pathnames
> @@ -363,31 +331,12 @@ set test_list [list \
>  		   "maint print symbols -pc" \
>  		   "maint print symbols -pc main $symbols_output"]
>  foreach { test_name command } $test_list {
> -    send_gdb "$command\n"
> -    gdb_expect {
> -	-re "^maint print symbols \[^\n\]*\r\n$gdb_prompt $" {
> -	    send_gdb "shell ls $symbols_output\n"
> -	    gdb_expect {
> -		-re "$symbols_output_re\r\n$gdb_prompt $" {
> -		    # See comments for `maint print psymbols'.
> -		    send_gdb "shell grep 'main(.*block' $symbols_output\n"
> -		    gdb_expect {
> -			-re "int main\\(int, char \\*\\*, char \\*\\*\\); block.*$gdb_prompt $" {
> -			    pass "$test_name"
> -			}
> -			-re ".*$gdb_prompt $" { fail "$test_name" }
> -			timeout { fail "$test_name (timeout)" }
> -		    }
> -		    gdb_test "shell rm -f $symbols_output" ".*" \
> -			"$test_name: shell rm -f symbols_output"
> -		}
> -		-re ".*$gdb_prompt $" { fail "$test_name" }
> -		timeout { fail "$test_name (timeout)" }
> -	    }
> -	}
> -	-re ".*$gdb_prompt $" { fail "$test_name" }
> -	timeout { fail "$test_name (timeout)" }
> -    }
> +    gdb_test_no_output "$command" "$test_name generate"
> +    gdb_test "shell grep 'main(.*block' $symbols_output"\
> +	"int main\\(int, char \\*\\*, char \\*\\*\\); block.*"\
> +	"$test_name read"
> +    gdb_test "shell rm -f $symbols_output" ".*" \
> +	"$test_name: shell rm -f symbols_output"
>  }
>  
>  set msg "maint print type"
> -- 
> 2.31.1


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH] gdb/testsuite: modernize gdb.base/maint.exp
  2022-06-21 15:52 ` Andrew Burgess
@ 2022-06-21 19:45   ` Bruno Larsen
  2022-06-21 19:54     ` [PATCHv2] " Bruno Larsen
                       ` (3 more replies)
  2022-06-24 15:22   ` Pedro Alves
  1 sibling, 4 replies; 25+ messages in thread
From: Bruno Larsen @ 2022-06-21 19:45 UTC (permalink / raw)
  To: gdb-patches; +Cc: aburgess, Bruno Larsen

gdb.base/maint.exp was using several gdb_expect statements, probably
because this test case predates the existance of gdb_test_multiple. This
commit updates the test case to use gdb_test_multiple, making it more
resilient to internal errors and such.

The only gdb_expect left in the testcase is one that specifically looks
for an internal error being triggered as a PASS.
---

Changes for v2:
 - Addressed Andrew's comments
 - rebased on current master.

---

 gdb/testsuite/gdb.base/maint.exp | 130 +++++++++++--------------------
 1 file changed, 46 insertions(+), 84 deletions(-)

diff --git a/gdb/testsuite/gdb.base/maint.exp b/gdb/testsuite/gdb.base/maint.exp
index 2817c6eafb9..31b999073f9 100644
--- a/gdb/testsuite/gdb.base/maint.exp
+++ b/gdb/testsuite/gdb.base/maint.exp
@@ -151,22 +151,28 @@ set have_psyms [expr ! ( $have_gdb_index || $readnow_p )]
 # unless there is some problem with the symtabs and psymtabs
 # so that branch will really never be covered in this tests here!!
 #
+# When there is a problem, there may be loads of output, which can
+# overwhelm the expect buffer. Splitting it seems to fix those
+# issues.
+
+set seen_command 0
+gdb_test_multiple "maint check-psymtabs" "" {
+    -re "^maint check-psymtabs\r\n" {
+	set seen_command 1
+	exp_continue
+    }
 
-# guo: on linux this command output is huge.  for some reason splitting up
-# the regexp checks works.
-#
-send_gdb "maint check-psymtabs\n"
-gdb_expect  {
-    -re "^maint check-psymtabs" {
-	gdb_expect {
-	    -re "$gdb_prompt $" {
-		pass "maint check-psymtabs"
-	    }
-	    timeout { fail "(timeout) maint check-psymtabs" }
+    -re "^$gdb_prompt $" {
+	if { $seen_command == 1 } {
+	    pass "maint check-psymtabs"
+	} else {
+	    fail $gdb_test_name
 	}
     }
-    -re ".*$gdb_prompt $"     { fail "maint check-psymtabs" }
-    timeout         { fail "(timeout) maint check-psymtabs" }
+
+    -re "^\[^\r\n\]+\r\n" {
+	exp_continue
+    }
 }
 
 # This command does not produce any output unless there is some problem
@@ -272,62 +278,37 @@ if { $have_psyms } {
 		       "maint print psymbols -pc" \
 		       "maint print psymbols -pc main $psymbols_output"]
     foreach { test_name command } $test_list {
-	send_gdb "$command\n"
-	    gdb_expect  {
-		-re "^maint print psymbols \[^\n\]*\r\n$gdb_prompt $" {
-		    send_gdb "shell ls $psymbols_output\n"
-		    gdb_expect {
-			-re "$psymbols_output_re\r\n$gdb_prompt $" {
-			    # We want this grep to be as specific as possible,
-			    # so it's less likely to match symbol file names in
-			    # psymbols_output.  Yes, this actually happened;
-			    # poor expect got tons of output, and timed out
-			    # trying to match it.   --- Jim Blandy <jimb@cygnus.com>
-			    send_gdb "shell grep 'main.*function' $psymbols_output\n"
-			    gdb_expect {
-				-re ".main., function, $hex.*$gdb_prompt $" {
-				    pass "$test_name 1"
-				}
-				-re ".*main.  .., function, $hex.*$gdb_prompt $" {
-				    pass "$test_name 2"
-				}
-				-re ".*$gdb_prompt $" { fail "$test_name" }
-				timeout { fail "$test_name (timeout)" }
-			    }
-			    gdb_test "shell rm -f $psymbols_output" ".*" \
-				"${test_name}: shell rm -f psymbols_output"
+	gdb_test_multiple "$command" "$test_name" {
+	    -re  -wrap "^maint print psymbols \[^\n\]*" {
+		gdb_test_multiple "shell grep 'main.*function' $psymbols_output" \
+		    "$test_name internal" {
+			-re -wrap ".main., function, $hex.*" {
+			    pass "$test_name, pattern 1"
+			}
+			-re -wrap ".*main.  .., function, $hex.*" {
+			    pass "$test_name, pattern 2"
 			}
-			-re ".*$gdb_prompt $" { fail "$test_name" }
-			timeout { fail "$test_name (timeout)" }
-		    }
 		}
-		-re ".*$gdb_prompt $" { fail "$test_name" }
-		timeout { fail "$test_name (timeout)" }
 	    }
+	}
+	gdb_test "shell rm -f $psymbols_output" ".*" \
+	    "${test_name}: shell rm -f psymbols_output"
     }
 }
 
 
 set msymbols_output [standard_output_file msymbols_output]
 set msymbols_output_re [string_to_regexp $msymbols_output]
-send_gdb "maint print msymbols -objfile ${binfile} $msymbols_output\n"
-gdb_expect  {
-    -re "^maint print msymbols \[^\n\]*\r\n$gdb_prompt $" {
-	send_gdb "shell ls $msymbols_output\n"
-	gdb_expect {
-	    -re "$msymbols_output_re\r\n$gdb_prompt $" {
-		gdb_test "shell grep factorial $msymbols_output" \
-		    "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
-		    "maint print msymbols, absolute pathname"
-		gdb_test "shell rm -f $msymbols_output" ".*" \
-		    "shell rm -f msymbols_output"
-	    }
-	    -re ".*$gdb_prompt $" { fail "maint print msymbols" }
-	    timeout { fail "maint print msymbols (timeout)" }
-	}
+gdb_test_multiple "maint print msymbols -objfile ${binfile} $msymbols_output" \
+		    "printing msymbols" {
+
+    -re -wrap "^maint print msymbols \[^\n\]*" {
+	gdb_test "shell grep factorial $msymbols_output" \
+	    "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
+	    "maint print msymbols, absolute pathname"
+	gdb_test "shell rm -f $msymbols_output" ".*" \
+	    "shell rm -f msymbols_output"
     }
-    -re ".*$gdb_prompt $" { fail "maint print msymbols" }
-    timeout { fail "maint print msymbols (timeout)" }
 }
 
 # Check that maint print msymbols allows relative pathnames
@@ -365,31 +346,12 @@ set test_list [list \
 		   "maint print symbols -pc" \
 		   "maint print symbols -pc main $symbols_output"]
 foreach { test_name command } $test_list {
-    send_gdb "$command\n"
-    gdb_expect {
-	-re "^maint print symbols \[^\n\]*\r\n$gdb_prompt $" {
-	    send_gdb "shell ls $symbols_output\n"
-	    gdb_expect {
-		-re "$symbols_output_re\r\n$gdb_prompt $" {
-		    # See comments for `maint print psymbols'.
-		    send_gdb "shell grep 'main(.*block' $symbols_output\n"
-		    gdb_expect {
-			-re "int main\\(int, char \\*\\*, char \\*\\*\\); block.*$gdb_prompt $" {
-			    pass "$test_name"
-			}
-			-re ".*$gdb_prompt $" { fail "$test_name" }
-			timeout { fail "$test_name (timeout)" }
-		    }
-		    gdb_test "shell rm -f $symbols_output" ".*" \
-			"$test_name: shell rm -f symbols_output"
-		}
-		-re ".*$gdb_prompt $" { fail "$test_name" }
-		timeout { fail "$test_name (timeout)" }
-	    }
-	}
-	-re ".*$gdb_prompt $" { fail "$test_name" }
-	timeout { fail "$test_name (timeout)" }
-    }
+    gdb_test_no_output "$command" "$test_name generate"
+    gdb_test "shell grep 'main(.*block' $symbols_output"\
+	"int main\\(int, char \\*\\*, char \\*\\*\\); block.*"\
+	"$test_name read"
+    gdb_test "shell rm -f $symbols_output" ".*" \
+	"$test_name: shell rm -f symbols_output"
 }
 
 set msg "maint print type"
-- 
2.31.1


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCHv2] gdb/testsuite: modernize gdb.base/maint.exp
  2022-06-21 19:45   ` Bruno Larsen
@ 2022-06-21 19:54     ` Bruno Larsen
  2022-06-24 13:20     ` [PATCH] " Andrew Burgess
                       ` (2 subsequent siblings)
  3 siblings, 0 replies; 25+ messages in thread
From: Bruno Larsen @ 2022-06-21 19:54 UTC (permalink / raw)
  To: gdb-patches; +Cc: aburgess

Sorry, forgot to update the title to v2, but it is the correct patch.

Cheers!
Bruno Larsen

On 6/21/22 16:45, Bruno Larsen wrote:
> gdb.base/maint.exp was using several gdb_expect statements, probably
> because this test case predates the existance of gdb_test_multiple. This
> commit updates the test case to use gdb_test_multiple, making it more
> resilient to internal errors and such.
> 
> The only gdb_expect left in the testcase is one that specifically looks
> for an internal error being triggered as a PASS.
> ---
> 
> Changes for v2:
>   - Addressed Andrew's comments
>   - rebased on current master.
> 
> ---
> 
>   gdb/testsuite/gdb.base/maint.exp | 130 +++++++++++--------------------
>   1 file changed, 46 insertions(+), 84 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.base/maint.exp b/gdb/testsuite/gdb.base/maint.exp
> index 2817c6eafb9..31b999073f9 100644
> --- a/gdb/testsuite/gdb.base/maint.exp
> +++ b/gdb/testsuite/gdb.base/maint.exp
> @@ -151,22 +151,28 @@ set have_psyms [expr ! ( $have_gdb_index || $readnow_p )]
>   # unless there is some problem with the symtabs and psymtabs
>   # so that branch will really never be covered in this tests here!!
>   #
> +# When there is a problem, there may be loads of output, which can
> +# overwhelm the expect buffer. Splitting it seems to fix those
> +# issues.
> +
> +set seen_command 0
> +gdb_test_multiple "maint check-psymtabs" "" {
> +    -re "^maint check-psymtabs\r\n" {
> +	set seen_command 1
> +	exp_continue
> +    }
>   
> -# guo: on linux this command output is huge.  for some reason splitting up
> -# the regexp checks works.
> -#
> -send_gdb "maint check-psymtabs\n"
> -gdb_expect  {
> -    -re "^maint check-psymtabs" {
> -	gdb_expect {
> -	    -re "$gdb_prompt $" {
> -		pass "maint check-psymtabs"
> -	    }
> -	    timeout { fail "(timeout) maint check-psymtabs" }
> +    -re "^$gdb_prompt $" {
> +	if { $seen_command == 1 } {
> +	    pass "maint check-psymtabs"
> +	} else {
> +	    fail $gdb_test_name
>   	}
>       }
> -    -re ".*$gdb_prompt $"     { fail "maint check-psymtabs" }
> -    timeout         { fail "(timeout) maint check-psymtabs" }
> +
> +    -re "^\[^\r\n\]+\r\n" {
> +	exp_continue
> +    }
>   }
>   
>   # This command does not produce any output unless there is some problem
> @@ -272,62 +278,37 @@ if { $have_psyms } {
>   		       "maint print psymbols -pc" \
>   		       "maint print psymbols -pc main $psymbols_output"]
>       foreach { test_name command } $test_list {
> -	send_gdb "$command\n"
> -	    gdb_expect  {
> -		-re "^maint print psymbols \[^\n\]*\r\n$gdb_prompt $" {
> -		    send_gdb "shell ls $psymbols_output\n"
> -		    gdb_expect {
> -			-re "$psymbols_output_re\r\n$gdb_prompt $" {
> -			    # We want this grep to be as specific as possible,
> -			    # so it's less likely to match symbol file names in
> -			    # psymbols_output.  Yes, this actually happened;
> -			    # poor expect got tons of output, and timed out
> -			    # trying to match it.   --- Jim Blandy <jimb@cygnus.com>
> -			    send_gdb "shell grep 'main.*function' $psymbols_output\n"
> -			    gdb_expect {
> -				-re ".main., function, $hex.*$gdb_prompt $" {
> -				    pass "$test_name 1"
> -				}
> -				-re ".*main.  .., function, $hex.*$gdb_prompt $" {
> -				    pass "$test_name 2"
> -				}
> -				-re ".*$gdb_prompt $" { fail "$test_name" }
> -				timeout { fail "$test_name (timeout)" }
> -			    }
> -			    gdb_test "shell rm -f $psymbols_output" ".*" \
> -				"${test_name}: shell rm -f psymbols_output"
> +	gdb_test_multiple "$command" "$test_name" {
> +	    -re  -wrap "^maint print psymbols \[^\n\]*" {
> +		gdb_test_multiple "shell grep 'main.*function' $psymbols_output" \
> +		    "$test_name internal" {
> +			-re -wrap ".main., function, $hex.*" {
> +			    pass "$test_name, pattern 1"
> +			}
> +			-re -wrap ".*main.  .., function, $hex.*" {
> +			    pass "$test_name, pattern 2"
>   			}
> -			-re ".*$gdb_prompt $" { fail "$test_name" }
> -			timeout { fail "$test_name (timeout)" }
> -		    }
>   		}
> -		-re ".*$gdb_prompt $" { fail "$test_name" }
> -		timeout { fail "$test_name (timeout)" }
>   	    }
> +	}
> +	gdb_test "shell rm -f $psymbols_output" ".*" \
> +	    "${test_name}: shell rm -f psymbols_output"
>       }
>   }
>   
>   
>   set msymbols_output [standard_output_file msymbols_output]
>   set msymbols_output_re [string_to_regexp $msymbols_output]
> -send_gdb "maint print msymbols -objfile ${binfile} $msymbols_output\n"
> -gdb_expect  {
> -    -re "^maint print msymbols \[^\n\]*\r\n$gdb_prompt $" {
> -	send_gdb "shell ls $msymbols_output\n"
> -	gdb_expect {
> -	    -re "$msymbols_output_re\r\n$gdb_prompt $" {
> -		gdb_test "shell grep factorial $msymbols_output" \
> -		    "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
> -		    "maint print msymbols, absolute pathname"
> -		gdb_test "shell rm -f $msymbols_output" ".*" \
> -		    "shell rm -f msymbols_output"
> -	    }
> -	    -re ".*$gdb_prompt $" { fail "maint print msymbols" }
> -	    timeout { fail "maint print msymbols (timeout)" }
> -	}
> +gdb_test_multiple "maint print msymbols -objfile ${binfile} $msymbols_output" \
> +		    "printing msymbols" {
> +
> +    -re -wrap "^maint print msymbols \[^\n\]*" {
> +	gdb_test "shell grep factorial $msymbols_output" \
> +	    "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
> +	    "maint print msymbols, absolute pathname"
> +	gdb_test "shell rm -f $msymbols_output" ".*" \
> +	    "shell rm -f msymbols_output"
>       }
> -    -re ".*$gdb_prompt $" { fail "maint print msymbols" }
> -    timeout { fail "maint print msymbols (timeout)" }
>   }
>   
>   # Check that maint print msymbols allows relative pathnames
> @@ -365,31 +346,12 @@ set test_list [list \
>   		   "maint print symbols -pc" \
>   		   "maint print symbols -pc main $symbols_output"]
>   foreach { test_name command } $test_list {
> -    send_gdb "$command\n"
> -    gdb_expect {
> -	-re "^maint print symbols \[^\n\]*\r\n$gdb_prompt $" {
> -	    send_gdb "shell ls $symbols_output\n"
> -	    gdb_expect {
> -		-re "$symbols_output_re\r\n$gdb_prompt $" {
> -		    # See comments for `maint print psymbols'.
> -		    send_gdb "shell grep 'main(.*block' $symbols_output\n"
> -		    gdb_expect {
> -			-re "int main\\(int, char \\*\\*, char \\*\\*\\); block.*$gdb_prompt $" {
> -			    pass "$test_name"
> -			}
> -			-re ".*$gdb_prompt $" { fail "$test_name" }
> -			timeout { fail "$test_name (timeout)" }
> -		    }
> -		    gdb_test "shell rm -f $symbols_output" ".*" \
> -			"$test_name: shell rm -f symbols_output"
> -		}
> -		-re ".*$gdb_prompt $" { fail "$test_name" }
> -		timeout { fail "$test_name (timeout)" }
> -	    }
> -	}
> -	-re ".*$gdb_prompt $" { fail "$test_name" }
> -	timeout { fail "$test_name (timeout)" }
> -    }
> +    gdb_test_no_output "$command" "$test_name generate"
> +    gdb_test "shell grep 'main(.*block' $symbols_output"\
> +	"int main\\(int, char \\*\\*, char \\*\\*\\); block.*"\
> +	"$test_name read"
> +    gdb_test "shell rm -f $symbols_output" ".*" \
> +	"$test_name: shell rm -f symbols_output"
>   }
>   
>   set msg "maint print type"


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] gdb/testsuite: modernize gdb.base/maint.exp
  2022-06-21 19:45   ` Bruno Larsen
  2022-06-21 19:54     ` [PATCHv2] " Bruno Larsen
@ 2022-06-24 13:20     ` Andrew Burgess
  2022-06-24 15:13       ` Pedro Alves
  2022-06-24 15:16     ` Pedro Alves
  2022-06-24 15:23     ` Pedro Alves
  3 siblings, 1 reply; 25+ messages in thread
From: Andrew Burgess @ 2022-06-24 13:20 UTC (permalink / raw)
  To: Bruno Larsen, gdb-patches

Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:

> gdb.base/maint.exp was using several gdb_expect statements, probably
> because this test case predates the existance of gdb_test_multiple. This
> commit updates the test case to use gdb_test_multiple, making it more
> resilient to internal errors and such.
>
> The only gdb_expect left in the testcase is one that specifically looks
> for an internal error being triggered as a PASS.
> ---
>
> Changes for v2:
>  - Addressed Andrew's comments
>  - rebased on current master.
>
> ---
>
>  gdb/testsuite/gdb.base/maint.exp | 130 +++++++++++--------------------
>  1 file changed, 46 insertions(+), 84 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.base/maint.exp b/gdb/testsuite/gdb.base/maint.exp
> index 2817c6eafb9..31b999073f9 100644
> --- a/gdb/testsuite/gdb.base/maint.exp
> +++ b/gdb/testsuite/gdb.base/maint.exp
> @@ -151,22 +151,28 @@ set have_psyms [expr ! ( $have_gdb_index || $readnow_p )]
>  # unless there is some problem with the symtabs and psymtabs
>  # so that branch will really never be covered in this tests here!!
>  #
> +# When there is a problem, there may be loads of output, which can
> +# overwhelm the expect buffer. Splitting it seems to fix those
> +# issues.
> +
> +set seen_command 0

  set seen_command false

> +gdb_test_multiple "maint check-psymtabs" "" {
> +    -re "^maint check-psymtabs\r\n" {
> +	set seen_command 1

  set seen_command true

> +	exp_continue
> +    }
>  
> -# guo: on linux this command output is huge.  for some reason splitting up
> -# the regexp checks works.
> -#
> -send_gdb "maint check-psymtabs\n"
> -gdb_expect  {
> -    -re "^maint check-psymtabs" {
> -	gdb_expect {
> -	    -re "$gdb_prompt $" {
> -		pass "maint check-psymtabs"
> -	    }
> -	    timeout { fail "(timeout) maint check-psymtabs" }
> +    -re "^$gdb_prompt $" {
> +	if { $seen_command == 1 } {
> +	    pass "maint check-psymtabs"
> +	} else {
> +	    fail $gdb_test_name
>  	}

You could use just:

  gdb_assert { $seen_command }

here.  Or maybe:

  gdb_assert { $seen_command } $gdb_test_name

Or possibly even:

  gdb_assert { $seen_command }
  pass $gdb_test_name

I think option #2 would be my pick, but whatever you prefer is fine I think.

>      }
> -    -re ".*$gdb_prompt $"     { fail "maint check-psymtabs" }
> -    timeout         { fail "(timeout) maint check-psymtabs" }
> +
> +    -re "^\[^\r\n\]+\r\n" {
> +	exp_continue
> +    }
>  }
>  
>  # This command does not produce any output unless there is some problem
> @@ -272,62 +278,37 @@ if { $have_psyms } {
>  		       "maint print psymbols -pc" \
>  		       "maint print psymbols -pc main $psymbols_output"]
>      foreach { test_name command } $test_list {
> -	send_gdb "$command\n"
> -	    gdb_expect  {
> -		-re "^maint print psymbols \[^\n\]*\r\n$gdb_prompt $" {
> -		    send_gdb "shell ls $psymbols_output\n"
> -		    gdb_expect {
> -			-re "$psymbols_output_re\r\n$gdb_prompt $" {
> -			    # We want this grep to be as specific as possible,
> -			    # so it's less likely to match symbol file names in
> -			    # psymbols_output.  Yes, this actually happened;
> -			    # poor expect got tons of output, and timed out
> -			    # trying to match it.   --- Jim Blandy <jimb@cygnus.com>
> -			    send_gdb "shell grep 'main.*function' $psymbols_output\n"
> -			    gdb_expect {
> -				-re ".main., function, $hex.*$gdb_prompt $" {
> -				    pass "$test_name 1"
> -				}
> -				-re ".*main.  .., function, $hex.*$gdb_prompt $" {
> -				    pass "$test_name 2"
> -				}
> -				-re ".*$gdb_prompt $" { fail "$test_name" }
> -				timeout { fail "$test_name (timeout)" }
> -			    }
> -			    gdb_test "shell rm -f $psymbols_output" ".*" \
> -				"${test_name}: shell rm -f psymbols_output"
> +	gdb_test_multiple "$command" "$test_name" {
> +	    -re  -wrap "^maint print psymbols \[^\n\]*" {
> +		gdb_test_multiple "shell grep 'main.*function' $psymbols_output" \
> +		    "$test_name internal" {
> +			-re -wrap ".main., function, $hex.*" {
> +			    pass "$test_name, pattern 1"
> +			}
> +			-re -wrap ".*main.  .., function, $hex.*" {
> +			    pass "$test_name, pattern 2"
>  			}
> -			-re ".*$gdb_prompt $" { fail "$test_name" }
> -			timeout { fail "$test_name (timeout)" }
> -		    }
>  		}
> -		-re ".*$gdb_prompt $" { fail "$test_name" }
> -		timeout { fail "$test_name (timeout)" }
>  	    }
> +	}
> +	gdb_test "shell rm -f $psymbols_output" ".*" \
> +	    "${test_name}: shell rm -f psymbols_output"
>      }
>  }
>  
>  
>  set msymbols_output [standard_output_file msymbols_output]
>  set msymbols_output_re [string_to_regexp $msymbols_output]
> -send_gdb "maint print msymbols -objfile ${binfile} $msymbols_output\n"
> -gdb_expect  {
> -    -re "^maint print msymbols \[^\n\]*\r\n$gdb_prompt $" {
> -	send_gdb "shell ls $msymbols_output\n"
> -	gdb_expect {
> -	    -re "$msymbols_output_re\r\n$gdb_prompt $" {
> -		gdb_test "shell grep factorial $msymbols_output" \
> -		    "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
> -		    "maint print msymbols, absolute pathname"
> -		gdb_test "shell rm -f $msymbols_output" ".*" \
> -		    "shell rm -f msymbols_output"
> -	    }
> -	    -re ".*$gdb_prompt $" { fail "maint print msymbols" }
> -	    timeout { fail "maint print msymbols (timeout)" }
> -	}
> +gdb_test_multiple "maint print msymbols -objfile ${binfile} $msymbols_output" \
> +		    "printing msymbols" {
> +
> +    -re -wrap "^maint print msymbols \[^\n\]*" {
> +	gdb_test "shell grep factorial $msymbols_output" \
> +	    "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
> +	    "maint print msymbols, absolute pathname"
> +	gdb_test "shell rm -f $msymbols_output" ".*" \
> +	    "shell rm -f msymbols_output"
>      }
> -    -re ".*$gdb_prompt $" { fail "maint print msymbols" }
> -    timeout { fail "maint print msymbols (timeout)" }
>  }

I think this block can be simplified to just:

  gdb_test_no_output "maint print msymbols -objfile ${binfile} $msymbols_output" \
      "maint print symbols, write to output file"
  gdb_test "shell grep factorial $msymbols_output" \
      "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
      "maint print msymbols, absolute pathname"
  gdb_test "shell rm -f $msymbols_output" ".*" \
      "shell rm -f msymbols_output"

I think this patch is fine with these changes made.

Thanks,
Andrew


>  
>  # Check that maint print msymbols allows relative pathnames
> @@ -365,31 +346,12 @@ set test_list [list \
>  		   "maint print symbols -pc" \
>  		   "maint print symbols -pc main $symbols_output"]
>  foreach { test_name command } $test_list {
> -    send_gdb "$command\n"
> -    gdb_expect {
> -	-re "^maint print symbols \[^\n\]*\r\n$gdb_prompt $" {
> -	    send_gdb "shell ls $symbols_output\n"
> -	    gdb_expect {
> -		-re "$symbols_output_re\r\n$gdb_prompt $" {
> -		    # See comments for `maint print psymbols'.
> -		    send_gdb "shell grep 'main(.*block' $symbols_output\n"
> -		    gdb_expect {
> -			-re "int main\\(int, char \\*\\*, char \\*\\*\\); block.*$gdb_prompt $" {
> -			    pass "$test_name"
> -			}
> -			-re ".*$gdb_prompt $" { fail "$test_name" }
> -			timeout { fail "$test_name (timeout)" }
> -		    }
> -		    gdb_test "shell rm -f $symbols_output" ".*" \
> -			"$test_name: shell rm -f symbols_output"
> -		}
> -		-re ".*$gdb_prompt $" { fail "$test_name" }
> -		timeout { fail "$test_name (timeout)" }
> -	    }
> -	}
> -	-re ".*$gdb_prompt $" { fail "$test_name" }
> -	timeout { fail "$test_name (timeout)" }
> -    }
> +    gdb_test_no_output "$command" "$test_name generate"
> +    gdb_test "shell grep 'main(.*block' $symbols_output"\
> +	"int main\\(int, char \\*\\*, char \\*\\*\\); block.*"\
> +	"$test_name read"
> +    gdb_test "shell rm -f $symbols_output" ".*" \
> +	"$test_name: shell rm -f symbols_output"
>  }
>  
>  set msg "maint print type"
> -- 
> 2.31.1


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] gdb/testsuite: modernize gdb.base/maint.exp
  2022-06-24 13:20     ` [PATCH] " Andrew Burgess
@ 2022-06-24 15:13       ` Pedro Alves
  0 siblings, 0 replies; 25+ messages in thread
From: Pedro Alves @ 2022-06-24 15:13 UTC (permalink / raw)
  To: Andrew Burgess, Bruno Larsen, gdb-patches

On 2022-06-24 14:20, Andrew Burgess via Gdb-patches wrote:
> You could use just:
> 
>   gdb_assert { $seen_command }
> 
> here.  Or maybe:
> 
>   gdb_assert { $seen_command } $gdb_test_name
> 
> Or possibly even:
> 
>   gdb_assert { $seen_command }
>   pass $gdb_test_name
> 
> I think option #2 would be my pick, but whatever you prefer is fine I think.
> 

I think option #2:

  gdb_assert { $seen_command } $gdb_test_name

is the right one, as it keeps pass/fail balanced if this test ever fails
without reaching the prompt regexp match.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] gdb/testsuite: modernize gdb.base/maint.exp
  2022-06-21 19:45   ` Bruno Larsen
  2022-06-21 19:54     ` [PATCHv2] " Bruno Larsen
  2022-06-24 13:20     ` [PATCH] " Andrew Burgess
@ 2022-06-24 15:16     ` Pedro Alves
  2022-06-24 15:23     ` Pedro Alves
  3 siblings, 0 replies; 25+ messages in thread
From: Pedro Alves @ 2022-06-24 15:16 UTC (permalink / raw)
  To: Bruno Larsen, gdb-patches; +Cc: aburgess

On 2022-06-21 20:45, Bruno Larsen via Gdb-patches wrote:
> +# When there is a problem, there may be loads of output, which can
> +# overwhelm the expect buffer. Splitting it seems to fix those
> +# issues.
> +
> +set seen_command 0
> +gdb_test_multiple "maint check-psymtabs" "" {
> +    -re "^maint check-psymtabs\r\n" {
> +	set seen_command 1
> +	exp_continue
> +    }
>  
> -# guo: on linux this command output is huge.  for some reason splitting up
> -# the regexp checks works.
> -#
> -send_gdb "maint check-psymtabs\n"
> -gdb_expect  {
> -    -re "^maint check-psymtabs" {
> -	gdb_expect {
> -	    -re "$gdb_prompt $" {
> -		pass "maint check-psymtabs"
> -	    }
> -	    timeout { fail "(timeout) maint check-psymtabs" }
> +    -re "^$gdb_prompt $" {
> +	if { $seen_command == 1 } {
> +	    pass "maint check-psymtabs"
> +	} else {
> +	    fail $gdb_test_name
>  	}
>      }
> -    -re ".*$gdb_prompt $"     { fail "maint check-psymtabs" }
> -    timeout         { fail "(timeout) maint check-psymtabs" }
> +
> +    -re "^\[^\r\n\]+\r\n" {
> +	exp_continue
> +    }

I think that instead of adding this line match, you can pass -lbl to gdb_test_multiple.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] gdb/testsuite: modernize gdb.base/maint.exp
  2022-06-21 15:52 ` Andrew Burgess
  2022-06-21 19:45   ` Bruno Larsen
@ 2022-06-24 15:22   ` Pedro Alves
  2022-06-24 17:51     ` Keith Seitz
  1 sibling, 1 reply; 25+ messages in thread
From: Pedro Alves @ 2022-06-24 15:22 UTC (permalink / raw)
  To: Andrew Burgess, Bruno Larsen, gdb-patches

On 2022-06-21 16:52, Andrew Burgess via Gdb-patches wrote:
>> +			    pass "$test_name 1"
> You could use:
> 
>     pass "$gdb_test_name, pattern 1"
> 
> here, and similar, with ', pattern 2' for the next 'pass' call.
> 

How about 

   pass "$gdb_test_name (pattern 1)"

   pass "$gdb_test_name (pattern 2)"

?

The idea being that the text in the trail parens is not considered part of the
test name, so when comparing gdb.sum files and matching test names, that parens part
should be discarded.  Whether this test passes with pattern 1 or 2 should make
no difference IIUC, thus I think it should not be part of the (part that counts
as real) test name.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] gdb/testsuite: modernize gdb.base/maint.exp
  2022-06-21 19:45   ` Bruno Larsen
                       ` (2 preceding siblings ...)
  2022-06-24 15:16     ` Pedro Alves
@ 2022-06-24 15:23     ` Pedro Alves
  3 siblings, 0 replies; 25+ messages in thread
From: Pedro Alves @ 2022-06-24 15:23 UTC (permalink / raw)
  To: Bruno Larsen, gdb-patches; +Cc: aburgess

BTW, thanks a lot for doing this.  I love patches like this, finally getting rid of a lot of cruft!

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] gdb/testsuite: modernize gdb.base/maint.exp
  2022-06-24 15:22   ` Pedro Alves
@ 2022-06-24 17:51     ` Keith Seitz
  2022-06-24 17:54       ` Bruno Larsen
                         ` (2 more replies)
  0 siblings, 3 replies; 25+ messages in thread
From: Keith Seitz @ 2022-06-24 17:51 UTC (permalink / raw)
  To: Pedro Alves, Andrew Burgess, Bruno Larsen, gdb-patches

On 6/24/22 08:22, Pedro Alves wrote:
> On 2022-06-21 16:52, Andrew Burgess via Gdb-patches wrote:
>>> +			    pass "$test_name 1"
>> You could use:
>>
>>      pass "$gdb_test_name, pattern 1"
>>
>> here, and similar, with ', pattern 2' for the next 'pass' call.
>>
> 
> How about
> 
>     pass "$gdb_test_name (pattern 1)"
> 
>     pass "$gdb_test_name (pattern 2)"
> 
> ?
> 
> The idea being that the text in the trail parens is not considered part of the
> test name, so when comparing gdb.sum files and matching test names, that parens part
> should be discarded.  Whether this test passes with pattern 1 or 2 should make
> no difference IIUC, thus I think it should not be part of the (part that counts
> as real) test name.
> 
I think it no surprise that I disdain this " (whatever)" idiom in test names. There
is also a long-standing guideline in the wiki against this:

https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Do_not_use_.22tail_parentheses.22_on_test_messages

Has there been some unannounced, sekrit change to this policy?

How is, e.g., "- pattern 1" any less desirable/informative than "(pattern 1)"?

Keith


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] gdb/testsuite: modernize gdb.base/maint.exp
  2022-06-24 17:51     ` Keith Seitz
@ 2022-06-24 17:54       ` Bruno Larsen
  2022-06-24 18:41       ` Pedro Alves
  2022-06-27 10:13       ` Andrew Burgess
  2 siblings, 0 replies; 25+ messages in thread
From: Bruno Larsen @ 2022-06-24 17:54 UTC (permalink / raw)
  To: Keith Seitz, Pedro Alves, Andrew Burgess, gdb-patches


On 6/24/22 14:51, Keith Seitz wrote:
> On 6/24/22 08:22, Pedro Alves wrote:
>> On 2022-06-21 16:52, Andrew Burgess via Gdb-patches wrote:
>>>> +                pass "$test_name 1"
>>> You could use:
>>>
>>>      pass "$gdb_test_name, pattern 1"
>>>
>>> here, and similar, with ', pattern 2' for the next 'pass' call.
>>>
>>
>> How about
>>
>>     pass "$gdb_test_name (pattern 1)"
>>
>>     pass "$gdb_test_name (pattern 2)"
>>
>> ?
>>
>> The idea being that the text in the trail parens is not considered part of the
>> test name, so when comparing gdb.sum files and matching test names, that parens part
>> should be discarded.  Whether this test passes with pattern 1 or 2 should make
>> no difference IIUC, thus I think it should not be part of the (part that counts
>> as real) test name.
>>
> I think it no surprise that I disdain this " (whatever)" idiom in test names. There
> is also a long-standing guideline in the wiki against this:
> 
> https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Do_not_use_.22tail_parentheses.22_on_test_messages
> 
> Has there been some unannounced, sekrit change to this policy?
> 
> How is, e.g., "- pattern 1" any less desirable/informative than "(pattern 1)"?

Taking this further, knowing which pattern was matched is actually useful in this situation?

When first making the change, I just blindly applied the gdb_test_multiple without seeing what
was going on, but looking at it again, it will just show which of the correct "spellings" GDB used
to respond to the command.

Personally, I don't see why it matters. Am I missing something?

Cheers!
Bruno Larsen

> 
> Keith
> 


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] gdb/testsuite: modernize gdb.base/maint.exp
  2022-06-24 17:51     ` Keith Seitz
  2022-06-24 17:54       ` Bruno Larsen
@ 2022-06-24 18:41       ` Pedro Alves
  2022-06-27 10:13       ` Andrew Burgess
  2 siblings, 0 replies; 25+ messages in thread
From: Pedro Alves @ 2022-06-24 18:41 UTC (permalink / raw)
  To: Keith Seitz, Andrew Burgess, Bruno Larsen, gdb-patches

Hi Keith,

On 2022-06-24 18:51, Keith Seitz wrote:
> On 6/24/22 08:22, Pedro Alves wrote:
>> On 2022-06-21 16:52, Andrew Burgess via Gdb-patches wrote:
>>>> +                pass "$test_name 1"
>>> You could use:
>>>
>>>      pass "$gdb_test_name, pattern 1"
>>>
>>> here, and similar, with ', pattern 2' for the next 'pass' call.
>>>
>>
>> How about
>>
>>     pass "$gdb_test_name (pattern 1)"
>>
>>     pass "$gdb_test_name (pattern 2)"
>>
>> ?
>>
>> The idea being that the text in the trail parens is not considered part of the
>> test name, so when comparing gdb.sum files and matching test names, that parens part
>> should be discarded.  Whether this test passes with pattern 1 or 2 should make
>> no difference IIUC, thus I think it should not be part of the (part that counts
>> as real) test name.
>>
> I think it no surprise that I disdain this " (whatever)" idiom in test names. There
> is also a long-standing guideline in the wiki against this:
> 
> https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Do_not_use_.22tail_parentheses.22_on_test_messages
> 
> Has there been some unannounced, sekrit change to this policy?

There has not.

As mentioned in the wiki, the idea of not putting trailing " (foo)" in the test names,
is that timeouts, xfail, kfail, internal error, etc. info is appended to the test
name in gdb.sum.  So this means that tools that compare test results and track
regressions/progressions at the individual test level should consider these the same test:

 PASS: check foo thing
 FAIL: check foo thing (timeout)
 FAIL: check foo thing (GDB internal error)
 KFAIL: check foo thing (PRMS: gdb/99999)
 (etc.)

So what you shouldn't do is use trailing parens to de-DUPLICATE test names.  Like,
this is wrong:

 gdb_test "foo cmd" " = 1" "foo cmd (1)"
 gdb_test "foo cmd" " = 2" "foo cmd (2)"

because the test analysis tooling should consider that those two tests have the
same name, thus, they are duplicates.  The duplication-detection machinery we
have today directly in the testsuite doesn't flag that, but I think it should.

You should thus write instead something like:

 gdb_test "foo cmd" " = 1" "foo cmd, 1st"
 gdb_test "foo cmd" " = 2" "foo cmd, 2nd"

Now, the case at hand is different -- we have a single test, with multiple
regexps, like:

 gdb_test_multiple "foo cmd" "some test name" {
    -re -wrap "pattern 1" {
       # machine/run 1 hits this one.
       pass "$gdb_test_name - pattern 1"
    }
    -re -wrap "pattern 2" {
       # machine/run 2 hits this one.
       pass "$gdb_test_name - pattern 2"
    }
 }

I think that's wrong, because depending on whatever pattern you get, you get
a different test name, making it harder to automatically compare test results.
Depending on machines or testsuite runs, you get one of these:

   PASS: some test name - pattern 1
   PASS: some test name - pattern 2
   FAIL: some test name (timeout)
   FAIL: some test name (GDB internal error)

By making it instead:

 gdb_test_multiple "foo cmd" "some test name" {
    -re -wrap "pattern 1" {
       # machine 1 hits this one.
       pass "$gdb_test_name (pattern 1)"
    }
    -re -wrap "pattern 2" {
       # machine 2 hits this one.
       pass "$gdb_test_name (pattern 2)"
    }
 }

and given the rule that trailing (foo) doesn't count, all the pass
calls above, and the pass/fail calls inside gdb_test_multiple have
the same test message.

It's effectively the same as:

gdb_test_multiple "foo cmd" "some test name" {
   -re -wrap "pattern 1" {
      # machine 1 hits this one.
      pass $gdb_test_name
   }
   -re -wrap "pattern 2" {
      # machine 2 hits this one.
      pass $gdb_test_name
   }
}

An alternative is to really write "pass $gdb_test_name", and precede it with
'verbose -log "got pattern 1"' or some such.  I'm fine with that, but given the fact
that tooling should already be ignoring the trailing parens, I don't think we need
to stop using trailing parens in scenarios like these.

> 
> How is, e.g., "- pattern 1" any less desirable/informative than "(pattern 1)"?

I hope I've clarified it above.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] gdb/testsuite: modernize gdb.base/maint.exp
  2022-06-24 17:51     ` Keith Seitz
  2022-06-24 17:54       ` Bruno Larsen
  2022-06-24 18:41       ` Pedro Alves
@ 2022-06-27 10:13       ` Andrew Burgess
  2 siblings, 0 replies; 25+ messages in thread
From: Andrew Burgess @ 2022-06-27 10:13 UTC (permalink / raw)
  To: Keith Seitz, Pedro Alves, Bruno Larsen, gdb-patches

Keith Seitz via Gdb-patches <gdb-patches@sourceware.org> writes:

> On 6/24/22 08:22, Pedro Alves wrote:
>> On 2022-06-21 16:52, Andrew Burgess via Gdb-patches wrote:
>>>> +			    pass "$test_name 1"
>>> You could use:
>>>
>>>      pass "$gdb_test_name, pattern 1"
>>>
>>> here, and similar, with ', pattern 2' for the next 'pass' call.
>>>
>> 
>> How about
>> 
>>     pass "$gdb_test_name (pattern 1)"
>> 
>>     pass "$gdb_test_name (pattern 2)"
>> 
>> ?
>> 
>> The idea being that the text in the trail parens is not considered part of the
>> test name, so when comparing gdb.sum files and matching test names, that parens part
>> should be discarded.  Whether this test passes with pattern 1 or 2 should make
>> no difference IIUC, thus I think it should not be part of the (part that counts
>> as real) test name.
>> 
> I think it no surprise that I disdain this " (whatever)" idiom in test names. There
> is also a long-standing guideline in the wiki against this:
>
> https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Do_not_use_.22tail_parentheses.22_on_test_messages

Rather than being against what Pedro said, that wiki page aligns exactly
with Pedro's advice, to quote:

  "This is because our our regression analysis tools make the assumption
  that the text inserted between parentheses is extra information rather
  than part of the test name."

Thus:

  pass "$gdb_test_name (pattern 1)"
  pass "$gdb_test_name (pattern 2)"

Would, for the purpose of analysis, both be considered as:

  pass "$gdb_test_name"
  pass "$gdb_test_name"

So, if today I hit 'pattern 1' and tomorrow hit 'pattern 2' then
compared these results, I would see no changes, and all is good.

I did originally consider suggesting that Bruno make this change, but in
the end didn't because I would normally only use this for situations
where the output is known unstable, maybe something like this imaginary
situation:

  pass "$gdb_test_name (thread exited early)"
  
  pass "$gdb_test_name (thread exited late)"

where it will depend on kernel scheduling which pass you actually hit.

In contrast, I think the case Bruno is addressing depends on platform
specific information, so I wouldn't expect GDB to switch outputs between
separate test runs, and if it did, I think I'd want to know about it.

So, for me, I'd not go with the parenthesis, but I wouldn't block the
patch if they were used.

Thanks,
Andrew

>
> Has there been some unannounced, sekrit change to this policy?
>
> How is, e.g., "- pattern 1" any less desirable/informative than "(pattern 1)"?
>
> Keith


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCHv3] gdb/testsuite: modernize gdb.base/maint.exp
  2022-05-09 18:04 [PATCH] gdb/testsuite: modernize gdb.base/maint.exp Bruno Larsen
  2022-05-23 14:14 ` [PING] " Bruno Larsen
  2022-06-21 15:52 ` Andrew Burgess
@ 2022-06-28 18:36 ` Bruno Larsen
  2022-06-29 10:33   ` Andrew Burgess
  2022-06-29 14:53 ` [PATCHv4] " Bruno Larsen
  3 siblings, 1 reply; 25+ messages in thread
From: Bruno Larsen @ 2022-06-28 18:36 UTC (permalink / raw)
  To: gdb-patches

gdb.base/maint.exp was using several gdb_expect statements, probably
because this test case predates the existance of gdb_test_multiple. This
commit updates the test case to use gdb_test_multiple, making it more
resilient to internal errors and such.

The only gdb_expect left in the testcase is one that specifically looks
for an internal error being triggered as a PASS.
---

Changes for version 3:
    * changed patterns when testing psymbols, now using trailing
parenthesis
    * used -lbl when running "check psymtabs"
    * made use of boolean variables and gdb_assert when testing
psymtabs
    * Simplified msymbols and psymbosl test to 3 tests, instead of
nested tests

Changes for v2:
 - Addressed Andrew's comments
 - rebased on current master.

---
 gdb/testsuite/gdb.base/maint.exp | 142 ++++++++++---------------------
 1 file changed, 43 insertions(+), 99 deletions(-)

diff --git a/gdb/testsuite/gdb.base/maint.exp b/gdb/testsuite/gdb.base/maint.exp
index 2817c6eafb9..aea8a10df0c 100644
--- a/gdb/testsuite/gdb.base/maint.exp
+++ b/gdb/testsuite/gdb.base/maint.exp
@@ -151,22 +151,20 @@ set have_psyms [expr ! ( $have_gdb_index || $readnow_p )]
 # unless there is some problem with the symtabs and psymtabs
 # so that branch will really never be covered in this tests here!!
 #
+# When there is a problem, there may be loads of output, which can
+# overwhelm the expect buffer. Splitting it seems to fix those
+# issues.
+
+set seen_command false
+gdb_test_multiple "maint check-psymtabs" "" -lbl {
+    -re "^maint check-psymtabs\r\n" {
+	set seen_command true
+	exp_continue
+    }
 
-# guo: on linux this command output is huge.  for some reason splitting up
-# the regexp checks works.
-#
-send_gdb "maint check-psymtabs\n"
-gdb_expect  {
-    -re "^maint check-psymtabs" {
-	gdb_expect {
-	    -re "$gdb_prompt $" {
-		pass "maint check-psymtabs"
-	    }
-	    timeout { fail "(timeout) maint check-psymtabs" }
-	}
+    -re "^$gdb_prompt $" {
+	gdb_assert { $seen_command } $gdb_test_name
     }
-    -re ".*$gdb_prompt $"     { fail "maint check-psymtabs" }
-    timeout         { fail "(timeout) maint check-psymtabs" }
 }
 
 # This command does not produce any output unless there is some problem
@@ -272,63 +270,33 @@ if { $have_psyms } {
 		       "maint print psymbols -pc" \
 		       "maint print psymbols -pc main $psymbols_output"]
     foreach { test_name command } $test_list {
-	send_gdb "$command\n"
-	    gdb_expect  {
-		-re "^maint print psymbols \[^\n\]*\r\n$gdb_prompt $" {
-		    send_gdb "shell ls $psymbols_output\n"
-		    gdb_expect {
-			-re "$psymbols_output_re\r\n$gdb_prompt $" {
-			    # We want this grep to be as specific as possible,
-			    # so it's less likely to match symbol file names in
-			    # psymbols_output.  Yes, this actually happened;
-			    # poor expect got tons of output, and timed out
-			    # trying to match it.   --- Jim Blandy <jimb@cygnus.com>
-			    send_gdb "shell grep 'main.*function' $psymbols_output\n"
-			    gdb_expect {
-				-re ".main., function, $hex.*$gdb_prompt $" {
-				    pass "$test_name 1"
-				}
-				-re ".*main.  .., function, $hex.*$gdb_prompt $" {
-				    pass "$test_name 2"
-				}
-				-re ".*$gdb_prompt $" { fail "$test_name" }
-				timeout { fail "$test_name (timeout)" }
-			    }
-			    gdb_test "shell rm -f $psymbols_output" ".*" \
-				"${test_name}: shell rm -f psymbols_output"
+	gdb_test_multiple "$command" "$test_name" {
+	    -re  -wrap "^maint print psymbols \[^\n\]*" {
+		gdb_test_multiple "shell grep 'main.*function' $psymbols_output" \
+		    "$test_name internal" {
+			-re -wrap ".main., function, $hex.*" {
+			    pass "$test_name (pattern 1)"
+			}
+			-re -wrap ".*main.  .., function, $hex.*" {
+			    pass "$test_name (pattern 2)"
 			}
-			-re ".*$gdb_prompt $" { fail "$test_name" }
-			timeout { fail "$test_name (timeout)" }
-		    }
 		}
-		-re ".*$gdb_prompt $" { fail "$test_name" }
-		timeout { fail "$test_name (timeout)" }
 	    }
+	}
+	gdb_test "shell rm -f $psymbols_output" ".*" \
+	    "${test_name}: shell rm -f psymbols_output"
     }
 }
 
 
 set msymbols_output [standard_output_file msymbols_output]
 set msymbols_output_re [string_to_regexp $msymbols_output]
-send_gdb "maint print msymbols -objfile ${binfile} $msymbols_output\n"
-gdb_expect  {
-    -re "^maint print msymbols \[^\n\]*\r\n$gdb_prompt $" {
-	send_gdb "shell ls $msymbols_output\n"
-	gdb_expect {
-	    -re "$msymbols_output_re\r\n$gdb_prompt $" {
-		gdb_test "shell grep factorial $msymbols_output" \
-		    "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
-		    "maint print msymbols, absolute pathname"
-		gdb_test "shell rm -f $msymbols_output" ".*" \
-		    "shell rm -f msymbols_output"
-	    }
-	    -re ".*$gdb_prompt $" { fail "maint print msymbols" }
-	    timeout { fail "maint print msymbols (timeout)" }
-	}
-    }
-    -re ".*$gdb_prompt $" { fail "maint print msymbols" }
-    timeout { fail "maint print msymbols (timeout)" }
-}
+gdb_test_no_output "maint print msymbols -objfile ${binfile} $msymbols_output" \
+    "print msymbols to file, with absolute path"
+gdb_test "shell grep factorial $msymbols_output" \
+    "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
+    "maint print msymbols, absolute pathname"
+gdb_test "shell rm -f $msymbols_output" ".*" "remove absolute path msymbols"
 
 # Check that maint print msymbols allows relative pathnames
 set mydir [pwd]
@@ -336,18 +304,13 @@ gdb_test "cd [standard_output_file {}]" \
     "Working directory .*\..*" \
     "cd to objdir"
 
-gdb_test_multiple "maint print msymbols -objfile ${testfile} msymbols_output2" "maint print msymbols" {
-    -re "^maint print msymbols \[^\n\]*\r\n$gdb_prompt $" {
-    	gdb_test_multiple "shell ls msymbols_output2" "maint print msymbols" {
-	    -re "msymbols_output2\r\n$gdb_prompt $" {
-		gdb_test "shell grep factorial msymbols_output2" \
-		    "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
-		    "maint print msymbols, relative pathname"
-		gdb_test "shell rm -f msymbols_output2" ".*"
-	    }
-	}
-    }
-}
+gdb_test_no_output "maint print msymbols -objfile ${testfile} $msymbols_output"\
+    "print msymbols to file, with relative path"
+gdb_test "shell grep factorial $msymbols_output" \
+    "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
+    "maint print msymbols, relative pathname"
+gdb_test "shell rm -f msymbols_output" ".*" "remove relative path msymbols"
+
 gdb_test "cd ${mydir}" \
     "Working directory [string_to_regexp ${mydir}]\..*" \
     "cd to mydir"
@@ -365,31 +328,12 @@ set test_list [list \
 		   "maint print symbols -pc" \
 		   "maint print symbols -pc main $symbols_output"]
 foreach { test_name command } $test_list {
-    send_gdb "$command\n"
-    gdb_expect {
-	-re "^maint print symbols \[^\n\]*\r\n$gdb_prompt $" {
-	    send_gdb "shell ls $symbols_output\n"
-	    gdb_expect {
-		-re "$symbols_output_re\r\n$gdb_prompt $" {
-		    # See comments for `maint print psymbols'.
-		    send_gdb "shell grep 'main(.*block' $symbols_output\n"
-		    gdb_expect {
-			-re "int main\\(int, char \\*\\*, char \\*\\*\\); block.*$gdb_prompt $" {
-			    pass "$test_name"
-			}
-			-re ".*$gdb_prompt $" { fail "$test_name" }
-			timeout { fail "$test_name (timeout)" }
-		    }
-		    gdb_test "shell rm -f $symbols_output" ".*" \
-			"$test_name: shell rm -f symbols_output"
-		}
-		-re ".*$gdb_prompt $" { fail "$test_name" }
-		timeout { fail "$test_name (timeout)" }
-	    }
-	}
-	-re ".*$gdb_prompt $" { fail "$test_name" }
-	timeout { fail "$test_name (timeout)" }
-    }
+    gdb_test_no_output "$command" "$test_name generate"
+    gdb_test "shell grep 'main(.*block' $symbols_output"\
+	"int main\\(int, char \\*\\*, char \\*\\*\\); block.*"\
+	"$test_name read"
+    gdb_test "shell rm -f $symbols_output" ".*" \
+	"$test_name: shell rm -f symbols_output"
 }
 
 set msg "maint print type"
-- 
2.31.1


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCHv3] gdb/testsuite: modernize gdb.base/maint.exp
  2022-06-28 18:36 ` [PATCHv3] " Bruno Larsen
@ 2022-06-29 10:33   ` Andrew Burgess
  2022-06-29 14:00     ` Bruno Larsen
  0 siblings, 1 reply; 25+ messages in thread
From: Andrew Burgess @ 2022-06-29 10:33 UTC (permalink / raw)
  To: Bruno Larsen, gdb-patches

Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:

> gdb.base/maint.exp was using several gdb_expect statements, probably
> because this test case predates the existance of gdb_test_multiple. This
> commit updates the test case to use gdb_test_multiple, making it more
> resilient to internal errors and such.
>
> The only gdb_expect left in the testcase is one that specifically looks
> for an internal error being triggered as a PASS.
> ---
>
> Changes for version 3:
>     * changed patterns when testing psymbols, now using trailing
> parenthesis
>     * used -lbl when running "check psymtabs"
>     * made use of boolean variables and gdb_assert when testing
> psymtabs
>     * Simplified msymbols and psymbosl test to 3 tests, instead of
> nested tests
>
> Changes for v2:
>  - Addressed Andrew's comments
>  - rebased on current master.
>
> ---
>  gdb/testsuite/gdb.base/maint.exp | 142 ++++++++++---------------------
>  1 file changed, 43 insertions(+), 99 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.base/maint.exp b/gdb/testsuite/gdb.base/maint.exp
> index 2817c6eafb9..aea8a10df0c 100644
> --- a/gdb/testsuite/gdb.base/maint.exp
> +++ b/gdb/testsuite/gdb.base/maint.exp
> @@ -151,22 +151,20 @@ set have_psyms [expr ! ( $have_gdb_index || $readnow_p )]
>  # unless there is some problem with the symtabs and psymtabs
>  # so that branch will really never be covered in this tests here!!
>  #
> +# When there is a problem, there may be loads of output, which can
> +# overwhelm the expect buffer. Splitting it seems to fix those
> +# issues.
> +
> +set seen_command false
> +gdb_test_multiple "maint check-psymtabs" "" -lbl {

Bruno,

Thanks for continuing to work on this...

Sorry to be a pain, but I don't think -lbl will actually do what you
want here.  To me the implementation of -lbl just looks weird, and I'm
not sure how it is expected to help.

Your patterns are:

    -re "^maint check-psymtabs\r\n" {
	set seen_command true
	exp_continue
    }

    -re "^$gdb_prompt $" {
	gdb_assert { $seen_command } $gdb_test_name
    }

And -lbl adds:

    -re "\r\n\[^\r\n\]*(?=\r\n)" {
        exp_continue
    }

The problem I think exists here is that your first pattern runs from the
star of the buffer and consumes the trailing \r\n.  Your prompt pattern
also expects to start from the start of the buffer.

If the output is:

  maint check-psymtabs\r\n
  (gdb) 

Then these patterns will work fine.  But if your output is:

  maint check-psymtabs\r\n
  Some other line here\r\n
  (gdb)

Then your first pattern will consume the first line, including the
trailing \r\n.  The -lbl pattern will then fail to match the second line
as the -lbl pattern expects a leading \r\n.  And now your pattern will
fail to match the third line as the second line is still in the buffer.

The -lbl pattern always leaves a trailing \r\n in the expect buffer,
which will cause your prompt pattern to fail.

Personally, I'd just drop the use of -lbl and add what I think is the
more correct pattern:

  -re "^\[^\r\n\]+\r\n" {
    exp_continue
  }

which is what you originally proposed.  But if you really want to make
use of -lbl then you can change your patterns to:

    -re "^maint check-psymtabs(?=\r\n)" {
	set seen_command true
	exp_continue
    }

    -re "\r\n$gdb_prompt $" {
	gdb_assert { $seen_command } $gdb_test_name
    }

I don't think that's an improvement, but I think that should work.

> +    -re "^maint check-psymtabs\r\n" {
> +	set seen_command true
> +	exp_continue
> +    }
>  
> -# guo: on linux this command output is huge.  for some reason splitting up
> -# the regexp checks works.
> -#
> -send_gdb "maint check-psymtabs\n"
> -gdb_expect  {
> -    -re "^maint check-psymtabs" {
> -	gdb_expect {
> -	    -re "$gdb_prompt $" {
> -		pass "maint check-psymtabs"
> -	    }
> -	    timeout { fail "(timeout) maint check-psymtabs" }
> -	}
> +    -re "^$gdb_prompt $" {
> +	gdb_assert { $seen_command } $gdb_test_name
>      }
> -    -re ".*$gdb_prompt $"     { fail "maint check-psymtabs" }
> -    timeout         { fail "(timeout) maint check-psymtabs" }
>  }
>  
>  # This command does not produce any output unless there is some problem
> @@ -272,63 +270,33 @@ if { $have_psyms } {
>  		       "maint print psymbols -pc" \
>  		       "maint print psymbols -pc main $psymbols_output"]
>      foreach { test_name command } $test_list {
> -	send_gdb "$command\n"
> -	    gdb_expect  {
> -		-re "^maint print psymbols \[^\n\]*\r\n$gdb_prompt $" {
> -		    send_gdb "shell ls $psymbols_output\n"
> -		    gdb_expect {
> -			-re "$psymbols_output_re\r\n$gdb_prompt $" {
> -			    # We want this grep to be as specific as possible,
> -			    # so it's less likely to match symbol file names in
> -			    # psymbols_output.  Yes, this actually happened;
> -			    # poor expect got tons of output, and timed out
> -			    # trying to match it.   --- Jim Blandy <jimb@cygnus.com>
> -			    send_gdb "shell grep 'main.*function' $psymbols_output\n"
> -			    gdb_expect {
> -				-re ".main., function, $hex.*$gdb_prompt $" {
> -				    pass "$test_name 1"
> -				}
> -				-re ".*main.  .., function, $hex.*$gdb_prompt $" {
> -				    pass "$test_name 2"
> -				}
> -				-re ".*$gdb_prompt $" { fail "$test_name" }
> -				timeout { fail "$test_name (timeout)" }
> -			    }
> -			    gdb_test "shell rm -f $psymbols_output" ".*" \
> -				"${test_name}: shell rm -f psymbols_output"
> +	gdb_test_multiple "$command" "$test_name" {
> +	    -re  -wrap "^maint print psymbols \[^\n\]*" {
> +		gdb_test_multiple "shell grep 'main.*function' $psymbols_output" \
> +		    "$test_name internal" {
> +			-re -wrap ".main., function, $hex.*" {
> +			    pass "$test_name (pattern 1)"
> +			}
> +			-re -wrap ".*main.  .., function, $hex.*" {
> +			    pass "$test_name (pattern 2)"
>  			}
> -			-re ".*$gdb_prompt $" { fail "$test_name" }
> -			timeout { fail "$test_name (timeout)" }
> -		    }
>  		}
> -		-re ".*$gdb_prompt $" { fail "$test_name" }
> -		timeout { fail "$test_name (timeout)" }
>  	    }
> +	}
> +	gdb_test "shell rm -f $psymbols_output" ".*" \
> +	    "${test_name}: shell rm -f psymbols_output"
>      }

Again, apologies for only just spotting this, but I think this test can
be further simplified to just:

    foreach { test_name command } $test_list {
        with_test_prefix "$test_name" {
          gdb_test_no_output "$command" "collect output"
          gdb_test_multiple "shell grep 'main.*function' $psymbols_output" \
            "$analyse output" {
  	        -re -wrap ".main., function, $hex.*" {
  	            pass "$gdb_test_name (pattern 1)"
  	        }
  	        -re -wrap ".*main.  .., function, $hex.*" {
  	            pass "$gdb_test_name (pattern 2)"
                }
          }
  	  gdb_test "shell rm -f $psymbols_output" ".*" \
  	      "shell rm -f psymbols_output"
        }
    }

I think we already made a similar change later on in this script, so
this just brings this test into line with the later ones.

Thanks,
Andrew

>  }
>  
>  
>  set msymbols_output [standard_output_file msymbols_output]
>  set msymbols_output_re [string_to_regexp $msymbols_output]
> -send_gdb "maint print msymbols -objfile ${binfile} $msymbols_output\n"
> -gdb_expect  {
> -    -re "^maint print msymbols \[^\n\]*\r\n$gdb_prompt $" {
> -	send_gdb "shell ls $msymbols_output\n"
> -	gdb_expect {
> -	    -re "$msymbols_output_re\r\n$gdb_prompt $" {
> -		gdb_test "shell grep factorial $msymbols_output" \
> -		    "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
> -		    "maint print msymbols, absolute pathname"
> -		gdb_test "shell rm -f $msymbols_output" ".*" \
> -		    "shell rm -f msymbols_output"
> -	    }
> -	    -re ".*$gdb_prompt $" { fail "maint print msymbols" }
> -	    timeout { fail "maint print msymbols (timeout)" }
> -	}
> -    }
> -    -re ".*$gdb_prompt $" { fail "maint print msymbols" }
> -    timeout { fail "maint print msymbols (timeout)" }
> -}
> +gdb_test_no_output "maint print msymbols -objfile ${binfile} $msymbols_output" \
> +    "print msymbols to file, with absolute path"
> +gdb_test "shell grep factorial $msymbols_output" \
> +    "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
> +    "maint print msymbols, absolute pathname"
> +gdb_test "shell rm -f $msymbols_output" ".*" "remove absolute path msymbols"
>  
>  # Check that maint print msymbols allows relative pathnames
>  set mydir [pwd]
> @@ -336,18 +304,13 @@ gdb_test "cd [standard_output_file {}]" \
>      "Working directory .*\..*" \
>      "cd to objdir"
>  
> -gdb_test_multiple "maint print msymbols -objfile ${testfile} msymbols_output2" "maint print msymbols" {
> -    -re "^maint print msymbols \[^\n\]*\r\n$gdb_prompt $" {
> -    	gdb_test_multiple "shell ls msymbols_output2" "maint print msymbols" {
> -	    -re "msymbols_output2\r\n$gdb_prompt $" {
> -		gdb_test "shell grep factorial msymbols_output2" \
> -		    "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
> -		    "maint print msymbols, relative pathname"
> -		gdb_test "shell rm -f msymbols_output2" ".*"
> -	    }
> -	}
> -    }
> -}
> +gdb_test_no_output "maint print msymbols -objfile ${testfile} $msymbols_output"\
> +    "print msymbols to file, with relative path"
> +gdb_test "shell grep factorial $msymbols_output" \
> +    "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
> +    "maint print msymbols, relative pathname"
> +gdb_test "shell rm -f msymbols_output" ".*" "remove relative path msymbols"
> +
>  gdb_test "cd ${mydir}" \
>      "Working directory [string_to_regexp ${mydir}]\..*" \
>      "cd to mydir"
> @@ -365,31 +328,12 @@ set test_list [list \
>  		   "maint print symbols -pc" \
>  		   "maint print symbols -pc main $symbols_output"]
>  foreach { test_name command } $test_list {
> -    send_gdb "$command\n"
> -    gdb_expect {
> -	-re "^maint print symbols \[^\n\]*\r\n$gdb_prompt $" {
> -	    send_gdb "shell ls $symbols_output\n"
> -	    gdb_expect {
> -		-re "$symbols_output_re\r\n$gdb_prompt $" {
> -		    # See comments for `maint print psymbols'.
> -		    send_gdb "shell grep 'main(.*block' $symbols_output\n"
> -		    gdb_expect {
> -			-re "int main\\(int, char \\*\\*, char \\*\\*\\); block.*$gdb_prompt $" {
> -			    pass "$test_name"
> -			}
> -			-re ".*$gdb_prompt $" { fail "$test_name" }
> -			timeout { fail "$test_name (timeout)" }
> -		    }
> -		    gdb_test "shell rm -f $symbols_output" ".*" \
> -			"$test_name: shell rm -f symbols_output"
> -		}
> -		-re ".*$gdb_prompt $" { fail "$test_name" }
> -		timeout { fail "$test_name (timeout)" }
> -	    }
> -	}
> -	-re ".*$gdb_prompt $" { fail "$test_name" }
> -	timeout { fail "$test_name (timeout)" }
> -    }
> +    gdb_test_no_output "$command" "$test_name generate"
> +    gdb_test "shell grep 'main(.*block' $symbols_output"\
> +	"int main\\(int, char \\*\\*, char \\*\\*\\); block.*"\
> +	"$test_name read"
> +    gdb_test "shell rm -f $symbols_output" ".*" \
> +	"$test_name: shell rm -f symbols_output"
>  }
>  
>  set msg "maint print type"
> -- 
> 2.31.1


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCHv3] gdb/testsuite: modernize gdb.base/maint.exp
  2022-06-29 10:33   ` Andrew Burgess
@ 2022-06-29 14:00     ` Bruno Larsen
  0 siblings, 0 replies; 25+ messages in thread
From: Bruno Larsen @ 2022-06-29 14:00 UTC (permalink / raw)
  To: Andrew Burgess, gdb-patches


On 6/29/22 07:33, Andrew Burgess wrote:
> Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
>> gdb.base/maint.exp was using several gdb_expect statements, probably
>> because this test case predates the existance of gdb_test_multiple. This
>> commit updates the test case to use gdb_test_multiple, making it more
>> resilient to internal errors and such.
>>
>> The only gdb_expect left in the testcase is one that specifically looks
>> for an internal error being triggered as a PASS.
>> ---
>>
>> Changes for version 3:
>>      * changed patterns when testing psymbols, now using trailing
>> parenthesis
>>      * used -lbl when running "check psymtabs"
>>      * made use of boolean variables and gdb_assert when testing
>> psymtabs
>>      * Simplified msymbols and psymbosl test to 3 tests, instead of
>> nested tests
>>
>> Changes for v2:
>>   - Addressed Andrew's comments
>>   - rebased on current master.
>>
>> ---
>>   gdb/testsuite/gdb.base/maint.exp | 142 ++++++++++---------------------
>>   1 file changed, 43 insertions(+), 99 deletions(-)
>>
>> diff --git a/gdb/testsuite/gdb.base/maint.exp b/gdb/testsuite/gdb.base/maint.exp
>> index 2817c6eafb9..aea8a10df0c 100644
>> --- a/gdb/testsuite/gdb.base/maint.exp
>> +++ b/gdb/testsuite/gdb.base/maint.exp
>> @@ -151,22 +151,20 @@ set have_psyms [expr ! ( $have_gdb_index || $readnow_p )]
>>   # unless there is some problem with the symtabs and psymtabs
>>   # so that branch will really never be covered in this tests here!!
>>   #
>> +# When there is a problem, there may be loads of output, which can
>> +# overwhelm the expect buffer. Splitting it seems to fix those
>> +# issues.
>> +
>> +set seen_command false
>> +gdb_test_multiple "maint check-psymtabs" "" -lbl {
> 
> Bruno,
> 
> Thanks for continuing to work on this...
> 
> Sorry to be a pain, but I don't think -lbl will actually do what you
> want here.  To me the implementation of -lbl just looks weird, and I'm
> not sure how it is expected to help.

Don't worry. This is why reviews are useful!

> 
> Your patterns are:
> 
>      -re "^maint check-psymtabs\r\n" {
> 	set seen_command true
> 	exp_continue
>      }
> 
>      -re "^$gdb_prompt $" {
> 	gdb_assert { $seen_command } $gdb_test_name
>      }
> 
> And -lbl adds:
> 
>      -re "\r\n\[^\r\n\]*(?=\r\n)" {
>          exp_continue
>      }
> 
> The problem I think exists here is that your first pattern runs from the
> star of the buffer and consumes the trailing \r\n.  Your prompt pattern
> also expects to start from the start of the buffer.
> 
> If the output is:
> 
>    maint check-psymtabs\r\n
>    (gdb)
> 
> Then these patterns will work fine.  But if your output is:
> 
>    maint check-psymtabs\r\n
>    Some other line here\r\n
>    (gdb)
> 
> Then your first pattern will consume the first line, including the
> trailing \r\n.  The -lbl pattern will then fail to match the second line
> as the -lbl pattern expects a leading \r\n.  And now your pattern will
> fail to match the third line as the second line is still in the buffer.
> 
> The -lbl pattern always leaves a trailing \r\n in the expect buffer,
> which will cause your prompt pattern to fail.
> 
> Personally, I'd just drop the use of -lbl and add what I think is the
> more correct pattern:
> 
>    -re "^\[^\r\n\]+\r\n" {
>      exp_continue
>    }
> 
> which is what you originally proposed.  But if you really want to make
> use of -lbl then you can change your patterns to:
> 
>      -re "^maint check-psymtabs(?=\r\n)" {
> 	set seen_command true
> 	exp_continue
>      }
> 
>      -re "\r\n$gdb_prompt $" {
> 	gdb_assert { $seen_command } $gdb_test_name
>      }
> 
> I don't think that's an improvement, but I think that should work.

This makes sense. I didn't really look at how -lbl worked because I wanted
to reduce complexity on the gdb_test_multiple call, but if I need to check
end of line without consuming, I don't think we're really reducing the
complexity of anything. I'll revert this to how it was on v2.

> 
>> +    -re "^maint check-psymtabs\r\n" {
>> +	set seen_command true
>> +	exp_continue
>> +    }
>>   
>> -# guo: on linux this command output is huge.  for some reason splitting up
>> -# the regexp checks works.
>> -#
>> -send_gdb "maint check-psymtabs\n"
>> -gdb_expect  {
>> -    -re "^maint check-psymtabs" {
>> -	gdb_expect {
>> -	    -re "$gdb_prompt $" {
>> -		pass "maint check-psymtabs"
>> -	    }
>> -	    timeout { fail "(timeout) maint check-psymtabs" }
>> -	}
>> +    -re "^$gdb_prompt $" {
>> +	gdb_assert { $seen_command } $gdb_test_name
>>       }
>> -    -re ".*$gdb_prompt $"     { fail "maint check-psymtabs" }
>> -    timeout         { fail "(timeout) maint check-psymtabs" }
>>   }
>>   
>>   # This command does not produce any output unless there is some problem
>> @@ -272,63 +270,33 @@ if { $have_psyms } {
>>   		       "maint print psymbols -pc" \
>>   		       "maint print psymbols -pc main $psymbols_output"]
>>       foreach { test_name command } $test_list {
>> -	send_gdb "$command\n"
>> -	    gdb_expect  {
>> -		-re "^maint print psymbols \[^\n\]*\r\n$gdb_prompt $" {
>> -		    send_gdb "shell ls $psymbols_output\n"
>> -		    gdb_expect {
>> -			-re "$psymbols_output_re\r\n$gdb_prompt $" {
>> -			    # We want this grep to be as specific as possible,
>> -			    # so it's less likely to match symbol file names in
>> -			    # psymbols_output.  Yes, this actually happened;
>> -			    # poor expect got tons of output, and timed out
>> -			    # trying to match it.   --- Jim Blandy <jimb@cygnus.com>
>> -			    send_gdb "shell grep 'main.*function' $psymbols_output\n"
>> -			    gdb_expect {
>> -				-re ".main., function, $hex.*$gdb_prompt $" {
>> -				    pass "$test_name 1"
>> -				}
>> -				-re ".*main.  .., function, $hex.*$gdb_prompt $" {
>> -				    pass "$test_name 2"
>> -				}
>> -				-re ".*$gdb_prompt $" { fail "$test_name" }
>> -				timeout { fail "$test_name (timeout)" }
>> -			    }
>> -			    gdb_test "shell rm -f $psymbols_output" ".*" \
>> -				"${test_name}: shell rm -f psymbols_output"
>> +	gdb_test_multiple "$command" "$test_name" {
>> +	    -re  -wrap "^maint print psymbols \[^\n\]*" {
>> +		gdb_test_multiple "shell grep 'main.*function' $psymbols_output" \
>> +		    "$test_name internal" {
>> +			-re -wrap ".main., function, $hex.*" {
>> +			    pass "$test_name (pattern 1)"
>> +			}
>> +			-re -wrap ".*main.  .., function, $hex.*" {
>> +			    pass "$test_name (pattern 2)"
>>   			}
>> -			-re ".*$gdb_prompt $" { fail "$test_name" }
>> -			timeout { fail "$test_name (timeout)" }
>> -		    }
>>   		}
>> -		-re ".*$gdb_prompt $" { fail "$test_name" }
>> -		timeout { fail "$test_name (timeout)" }
>>   	    }
>> +	}
>> +	gdb_test "shell rm -f $psymbols_output" ".*" \
>> +	    "${test_name}: shell rm -f psymbols_output"
>>       }
> 
> Again, apologies for only just spotting this, but I think this test can
> be further simplified to just:
> 
>      foreach { test_name command } $test_list {
>          with_test_prefix "$test_name" {
>            gdb_test_no_output "$command" "collect output"
>            gdb_test_multiple "shell grep 'main.*function' $psymbols_output" \
>              "$analyse output" {
>    	        -re -wrap ".main., function, $hex.*" {
>    	            pass "$gdb_test_name (pattern 1)"
>    	        }
>    	        -re -wrap ".*main.  .., function, $hex.*" {
>    	            pass "$gdb_test_name (pattern 2)"
>                  }
>            }
>    	  gdb_test "shell rm -f $psymbols_output" ".*" \
>    	      "shell rm -f psymbols_output"
>          }
>      }
> 
> I think we already made a similar change later on in this script, so
> this just brings this test into line with the later ones.

Alright, makes sense too. v4 coming soon!

Cheers!
Bruno Larsen

> 
> Thanks,
> Andrew
> 
>>   }
>>   
>>   
>>   set msymbols_output [standard_output_file msymbols_output]
>>   set msymbols_output_re [string_to_regexp $msymbols_output]
>> -send_gdb "maint print msymbols -objfile ${binfile} $msymbols_output\n"
>> -gdb_expect  {
>> -    -re "^maint print msymbols \[^\n\]*\r\n$gdb_prompt $" {
>> -	send_gdb "shell ls $msymbols_output\n"
>> -	gdb_expect {
>> -	    -re "$msymbols_output_re\r\n$gdb_prompt $" {
>> -		gdb_test "shell grep factorial $msymbols_output" \
>> -		    "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
>> -		    "maint print msymbols, absolute pathname"
>> -		gdb_test "shell rm -f $msymbols_output" ".*" \
>> -		    "shell rm -f msymbols_output"
>> -	    }
>> -	    -re ".*$gdb_prompt $" { fail "maint print msymbols" }
>> -	    timeout { fail "maint print msymbols (timeout)" }
>> -	}
>> -    }
>> -    -re ".*$gdb_prompt $" { fail "maint print msymbols" }
>> -    timeout { fail "maint print msymbols (timeout)" }
>> -}
>> +gdb_test_no_output "maint print msymbols -objfile ${binfile} $msymbols_output" \
>> +    "print msymbols to file, with absolute path"
>> +gdb_test "shell grep factorial $msymbols_output" \
>> +    "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
>> +    "maint print msymbols, absolute pathname"
>> +gdb_test "shell rm -f $msymbols_output" ".*" "remove absolute path msymbols"
>>   
>>   # Check that maint print msymbols allows relative pathnames
>>   set mydir [pwd]
>> @@ -336,18 +304,13 @@ gdb_test "cd [standard_output_file {}]" \
>>       "Working directory .*\..*" \
>>       "cd to objdir"
>>   
>> -gdb_test_multiple "maint print msymbols -objfile ${testfile} msymbols_output2" "maint print msymbols" {
>> -    -re "^maint print msymbols \[^\n\]*\r\n$gdb_prompt $" {
>> -    	gdb_test_multiple "shell ls msymbols_output2" "maint print msymbols" {
>> -	    -re "msymbols_output2\r\n$gdb_prompt $" {
>> -		gdb_test "shell grep factorial msymbols_output2" \
>> -		    "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
>> -		    "maint print msymbols, relative pathname"
>> -		gdb_test "shell rm -f msymbols_output2" ".*"
>> -	    }
>> -	}
>> -    }
>> -}
>> +gdb_test_no_output "maint print msymbols -objfile ${testfile} $msymbols_output"\
>> +    "print msymbols to file, with relative path"
>> +gdb_test "shell grep factorial $msymbols_output" \
>> +    "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
>> +    "maint print msymbols, relative pathname"
>> +gdb_test "shell rm -f msymbols_output" ".*" "remove relative path msymbols"
>> +
>>   gdb_test "cd ${mydir}" \
>>       "Working directory [string_to_regexp ${mydir}]\..*" \
>>       "cd to mydir"
>> @@ -365,31 +328,12 @@ set test_list [list \
>>   		   "maint print symbols -pc" \
>>   		   "maint print symbols -pc main $symbols_output"]
>>   foreach { test_name command } $test_list {
>> -    send_gdb "$command\n"
>> -    gdb_expect {
>> -	-re "^maint print symbols \[^\n\]*\r\n$gdb_prompt $" {
>> -	    send_gdb "shell ls $symbols_output\n"
>> -	    gdb_expect {
>> -		-re "$symbols_output_re\r\n$gdb_prompt $" {
>> -		    # See comments for `maint print psymbols'.
>> -		    send_gdb "shell grep 'main(.*block' $symbols_output\n"
>> -		    gdb_expect {
>> -			-re "int main\\(int, char \\*\\*, char \\*\\*\\); block.*$gdb_prompt $" {
>> -			    pass "$test_name"
>> -			}
>> -			-re ".*$gdb_prompt $" { fail "$test_name" }
>> -			timeout { fail "$test_name (timeout)" }
>> -		    }
>> -		    gdb_test "shell rm -f $symbols_output" ".*" \
>> -			"$test_name: shell rm -f symbols_output"
>> -		}
>> -		-re ".*$gdb_prompt $" { fail "$test_name" }
>> -		timeout { fail "$test_name (timeout)" }
>> -	    }
>> -	}
>> -	-re ".*$gdb_prompt $" { fail "$test_name" }
>> -	timeout { fail "$test_name (timeout)" }
>> -    }
>> +    gdb_test_no_output "$command" "$test_name generate"
>> +    gdb_test "shell grep 'main(.*block' $symbols_output"\
>> +	"int main\\(int, char \\*\\*, char \\*\\*\\); block.*"\
>> +	"$test_name read"
>> +    gdb_test "shell rm -f $symbols_output" ".*" \
>> +	"$test_name: shell rm -f symbols_output"
>>   }
>>   
>>   set msg "maint print type"
>> -- 
>> 2.31.1
> 


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCHv4] gdb/testsuite: modernize gdb.base/maint.exp
  2022-05-09 18:04 [PATCH] gdb/testsuite: modernize gdb.base/maint.exp Bruno Larsen
                   ` (2 preceding siblings ...)
  2022-06-28 18:36 ` [PATCHv3] " Bruno Larsen
@ 2022-06-29 14:53 ` Bruno Larsen
  2022-07-13 11:22   ` [PING][PATCHv4] " Bruno Larsen
  3 siblings, 1 reply; 25+ messages in thread
From: Bruno Larsen @ 2022-06-29 14:53 UTC (permalink / raw)
  To: gdb-patches

gdb.base/maint.exp was using several gdb_expect statements, probably
because this test case predates the existance of gdb_test_multiple. This
commit updates the test case to use gdb_test_multiple, making it more
resilient to internal errors and such.

The only gdb_expect left in the testcase is one that specifically looks
for an internal error being triggered as a PASS.
---

Changes for v4:
    * Reverted -lbl change, as it wouldn't make the pattern simpler
    * Simplified another psymbols test

Changes for version 3:
    * changed patterns when testing psymbols, now using trailing
parenthesis
    * used -lbl when running "check psymtabs"
    * made use of boolean variables and gdb_assert when testing
psymtabs
    * Simplified msymbols and psymbosl test to 3 tests, instead of
nested tests

Changes for v2:
 - Addressed Andrew's comments
 - rebased on current master.

---
 gdb/testsuite/gdb.base/maint.exp | 146 ++++++++++---------------------
 1 file changed, 45 insertions(+), 101 deletions(-)

diff --git a/gdb/testsuite/gdb.base/maint.exp b/gdb/testsuite/gdb.base/maint.exp
index 2817c6eafb9..16628b4a8b6 100644
--- a/gdb/testsuite/gdb.base/maint.exp
+++ b/gdb/testsuite/gdb.base/maint.exp
@@ -151,22 +151,24 @@ set have_psyms [expr ! ( $have_gdb_index || $readnow_p )]
 # unless there is some problem with the symtabs and psymtabs
 # so that branch will really never be covered in this tests here!!
 #
+# When there is a problem, there may be loads of output, which can
+# overwhelm the expect buffer. Splitting it seems to fix those
+# issues.
+
+set seen_command false
+gdb_test_multiple "maint check-psymtabs" "" {
+    -re "^maint check-psymtabs\r\n" {
+	set seen_command true
+	exp_continue
+    }
 
-# guo: on linux this command output is huge.  for some reason splitting up
-# the regexp checks works.
-#
-send_gdb "maint check-psymtabs\n"
-gdb_expect  {
-    -re "^maint check-psymtabs" {
-	gdb_expect {
-	    -re "$gdb_prompt $" {
-		pass "maint check-psymtabs"
-	    }
-	    timeout { fail "(timeout) maint check-psymtabs" }
-	}
+    -re "^$gdb_prompt $" {
+	gdb_assert { $seen_command } $gdb_test_name
+    }
+
+    -re "^\[^\r\n\]+\r\n" {
+	exp_continue
     }
-    -re ".*$gdb_prompt $"     { fail "maint check-psymtabs" }
-    timeout         { fail "(timeout) maint check-psymtabs" }
 }
 
 # This command does not produce any output unless there is some problem
@@ -272,63 +274,29 @@ if { $have_psyms } {
 		       "maint print psymbols -pc" \
 		       "maint print psymbols -pc main $psymbols_output"]
     foreach { test_name command } $test_list {
-	send_gdb "$command\n"
-	    gdb_expect  {
-		-re "^maint print psymbols \[^\n\]*\r\n$gdb_prompt $" {
-		    send_gdb "shell ls $psymbols_output\n"
-		    gdb_expect {
-			-re "$psymbols_output_re\r\n$gdb_prompt $" {
-			    # We want this grep to be as specific as possible,
-			    # so it's less likely to match symbol file names in
-			    # psymbols_output.  Yes, this actually happened;
-			    # poor expect got tons of output, and timed out
-			    # trying to match it.   --- Jim Blandy <jimb@cygnus.com>
-			    send_gdb "shell grep 'main.*function' $psymbols_output\n"
-			    gdb_expect {
-				-re ".main., function, $hex.*$gdb_prompt $" {
-				    pass "$test_name 1"
-				}
-				-re ".*main.  .., function, $hex.*$gdb_prompt $" {
-				    pass "$test_name 2"
-				}
-				-re ".*$gdb_prompt $" { fail "$test_name" }
-				timeout { fail "$test_name (timeout)" }
-			    }
-			    gdb_test "shell rm -f $psymbols_output" ".*" \
-				"${test_name}: shell rm -f psymbols_output"
-			}
-			-re ".*$gdb_prompt $" { fail "$test_name" }
-			timeout { fail "$test_name (timeout)" }
-		    }
+	gdb_test_no_output "$command" "collecting data for $test_name"
+	gdb_test_multiple "shell grep 'main.*function' $psymbols_output" "" {
+		-re -wrap ".main., function, $hex.*" {
+		    pass "$test_name (pattern 1)"
 		}
-		-re ".*$gdb_prompt $" { fail "$test_name" }
-		timeout { fail "$test_name (timeout)" }
-	    }
+		-re -wrap ".*main.  .., function, $hex.*" {
+		    pass "$test_name (pattern 2)"
+		}
+	}
+	gdb_test "shell rm -f $psymbols_output" ".*" \
+	    "${test_name}: shell rm -f psymbols_output"
     }
 }
 
 
 set msymbols_output [standard_output_file msymbols_output]
 set msymbols_output_re [string_to_regexp $msymbols_output]
-send_gdb "maint print msymbols -objfile ${binfile} $msymbols_output\n"
-gdb_expect  {
-    -re "^maint print msymbols \[^\n\]*\r\n$gdb_prompt $" {
-	send_gdb "shell ls $msymbols_output\n"
-	gdb_expect {
-	    -re "$msymbols_output_re\r\n$gdb_prompt $" {
-		gdb_test "shell grep factorial $msymbols_output" \
-		    "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
-		    "maint print msymbols, absolute pathname"
-		gdb_test "shell rm -f $msymbols_output" ".*" \
-		    "shell rm -f msymbols_output"
-	    }
-	    -re ".*$gdb_prompt $" { fail "maint print msymbols" }
-	    timeout { fail "maint print msymbols (timeout)" }
-	}
-    }
-    -re ".*$gdb_prompt $" { fail "maint print msymbols" }
-    timeout { fail "maint print msymbols (timeout)" }
-}
+gdb_test_no_output "maint print msymbols -objfile ${binfile} $msymbols_output" \
+    "print msymbols to file, with absolute path"
+gdb_test "shell grep factorial $msymbols_output" \
+    "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
+    "maint print msymbols, absolute pathname"
+gdb_test "shell rm -f $msymbols_output" ".*" "remove absolute path msymbols"
 
 # Check that maint print msymbols allows relative pathnames
 set mydir [pwd]
@@ -336,18 +304,13 @@ gdb_test "cd [standard_output_file {}]" \
     "Working directory .*\..*" \
     "cd to objdir"
 
-gdb_test_multiple "maint print msymbols -objfile ${testfile} msymbols_output2" "maint print msymbols" {
-    -re "^maint print msymbols \[^\n\]*\r\n$gdb_prompt $" {
-    	gdb_test_multiple "shell ls msymbols_output2" "maint print msymbols" {
-	    -re "msymbols_output2\r\n$gdb_prompt $" {
-		gdb_test "shell grep factorial msymbols_output2" \
-		    "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
-		    "maint print msymbols, relative pathname"
-		gdb_test "shell rm -f msymbols_output2" ".*"
-	    }
-	}
-    }
-}
+gdb_test_no_output "maint print msymbols -objfile ${testfile} $msymbols_output"\
+    "print msymbols to file, with relative path"
+gdb_test "shell grep factorial $msymbols_output" \
+    "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
+    "maint print msymbols, relative pathname"
+gdb_test "shell rm -f msymbols_output" ".*" "remove relative path msymbols"
+
 gdb_test "cd ${mydir}" \
     "Working directory [string_to_regexp ${mydir}]\..*" \
     "cd to mydir"
@@ -365,31 +328,12 @@ set test_list [list \
 		   "maint print symbols -pc" \
 		   "maint print symbols -pc main $symbols_output"]
 foreach { test_name command } $test_list {
-    send_gdb "$command\n"
-    gdb_expect {
-	-re "^maint print symbols \[^\n\]*\r\n$gdb_prompt $" {
-	    send_gdb "shell ls $symbols_output\n"
-	    gdb_expect {
-		-re "$symbols_output_re\r\n$gdb_prompt $" {
-		    # See comments for `maint print psymbols'.
-		    send_gdb "shell grep 'main(.*block' $symbols_output\n"
-		    gdb_expect {
-			-re "int main\\(int, char \\*\\*, char \\*\\*\\); block.*$gdb_prompt $" {
-			    pass "$test_name"
-			}
-			-re ".*$gdb_prompt $" { fail "$test_name" }
-			timeout { fail "$test_name (timeout)" }
-		    }
-		    gdb_test "shell rm -f $symbols_output" ".*" \
-			"$test_name: shell rm -f symbols_output"
-		}
-		-re ".*$gdb_prompt $" { fail "$test_name" }
-		timeout { fail "$test_name (timeout)" }
-	    }
-	}
-	-re ".*$gdb_prompt $" { fail "$test_name" }
-	timeout { fail "$test_name (timeout)" }
-    }
+    gdb_test_no_output "$command" "$test_name generate"
+    gdb_test "shell grep 'main(.*block' $symbols_output"\
+	"int main\\(int, char \\*\\*, char \\*\\*\\); block.*"\
+	"$test_name read"
+    gdb_test "shell rm -f $symbols_output" ".*" \
+	"$test_name: shell rm -f symbols_output"
 }
 
 set msg "maint print type"
-- 
2.31.1


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PING][PATCHv4] gdb/testsuite: modernize gdb.base/maint.exp
  2022-06-29 14:53 ` [PATCHv4] " Bruno Larsen
@ 2022-07-13 11:22   ` Bruno Larsen
  2022-07-15 16:49     ` Tom Tromey
  0 siblings, 1 reply; 25+ messages in thread
From: Bruno Larsen @ 2022-07-13 11:22 UTC (permalink / raw)
  To: gdb-patches

ping!

Cheers!
Bruno Larsen

On 6/29/22 11:53, Bruno Larsen wrote:
> gdb.base/maint.exp was using several gdb_expect statements, probably
> because this test case predates the existance of gdb_test_multiple. This
> commit updates the test case to use gdb_test_multiple, making it more
> resilient to internal errors and such.
> 
> The only gdb_expect left in the testcase is one that specifically looks
> for an internal error being triggered as a PASS.
> ---
> 
> Changes for v4:
>      * Reverted -lbl change, as it wouldn't make the pattern simpler
>      * Simplified another psymbols test
> 
> Changes for version 3:
>      * changed patterns when testing psymbols, now using trailing
> parenthesis
>      * used -lbl when running "check psymtabs"
>      * made use of boolean variables and gdb_assert when testing
> psymtabs
>      * Simplified msymbols and psymbosl test to 3 tests, instead of
> nested tests
> 
> Changes for v2:
>   - Addressed Andrew's comments
>   - rebased on current master.
> 
> ---
>   gdb/testsuite/gdb.base/maint.exp | 146 ++++++++++---------------------
>   1 file changed, 45 insertions(+), 101 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.base/maint.exp b/gdb/testsuite/gdb.base/maint.exp
> index 2817c6eafb9..16628b4a8b6 100644
> --- a/gdb/testsuite/gdb.base/maint.exp
> +++ b/gdb/testsuite/gdb.base/maint.exp
> @@ -151,22 +151,24 @@ set have_psyms [expr ! ( $have_gdb_index || $readnow_p )]
>   # unless there is some problem with the symtabs and psymtabs
>   # so that branch will really never be covered in this tests here!!
>   #
> +# When there is a problem, there may be loads of output, which can
> +# overwhelm the expect buffer. Splitting it seems to fix those
> +# issues.
> +
> +set seen_command false
> +gdb_test_multiple "maint check-psymtabs" "" {
> +    -re "^maint check-psymtabs\r\n" {
> +	set seen_command true
> +	exp_continue
> +    }
>   
> -# guo: on linux this command output is huge.  for some reason splitting up
> -# the regexp checks works.
> -#
> -send_gdb "maint check-psymtabs\n"
> -gdb_expect  {
> -    -re "^maint check-psymtabs" {
> -	gdb_expect {
> -	    -re "$gdb_prompt $" {
> -		pass "maint check-psymtabs"
> -	    }
> -	    timeout { fail "(timeout) maint check-psymtabs" }
> -	}
> +    -re "^$gdb_prompt $" {
> +	gdb_assert { $seen_command } $gdb_test_name
> +    }
> +
> +    -re "^\[^\r\n\]+\r\n" {
> +	exp_continue
>       }
> -    -re ".*$gdb_prompt $"     { fail "maint check-psymtabs" }
> -    timeout         { fail "(timeout) maint check-psymtabs" }
>   }
>   
>   # This command does not produce any output unless there is some problem
> @@ -272,63 +274,29 @@ if { $have_psyms } {
>   		       "maint print psymbols -pc" \
>   		       "maint print psymbols -pc main $psymbols_output"]
>       foreach { test_name command } $test_list {
> -	send_gdb "$command\n"
> -	    gdb_expect  {
> -		-re "^maint print psymbols \[^\n\]*\r\n$gdb_prompt $" {
> -		    send_gdb "shell ls $psymbols_output\n"
> -		    gdb_expect {
> -			-re "$psymbols_output_re\r\n$gdb_prompt $" {
> -			    # We want this grep to be as specific as possible,
> -			    # so it's less likely to match symbol file names in
> -			    # psymbols_output.  Yes, this actually happened;
> -			    # poor expect got tons of output, and timed out
> -			    # trying to match it.   --- Jim Blandy <jimb@cygnus.com>
> -			    send_gdb "shell grep 'main.*function' $psymbols_output\n"
> -			    gdb_expect {
> -				-re ".main., function, $hex.*$gdb_prompt $" {
> -				    pass "$test_name 1"
> -				}
> -				-re ".*main.  .., function, $hex.*$gdb_prompt $" {
> -				    pass "$test_name 2"
> -				}
> -				-re ".*$gdb_prompt $" { fail "$test_name" }
> -				timeout { fail "$test_name (timeout)" }
> -			    }
> -			    gdb_test "shell rm -f $psymbols_output" ".*" \
> -				"${test_name}: shell rm -f psymbols_output"
> -			}
> -			-re ".*$gdb_prompt $" { fail "$test_name" }
> -			timeout { fail "$test_name (timeout)" }
> -		    }
> +	gdb_test_no_output "$command" "collecting data for $test_name"
> +	gdb_test_multiple "shell grep 'main.*function' $psymbols_output" "" {
> +		-re -wrap ".main., function, $hex.*" {
> +		    pass "$test_name (pattern 1)"
>   		}
> -		-re ".*$gdb_prompt $" { fail "$test_name" }
> -		timeout { fail "$test_name (timeout)" }
> -	    }
> +		-re -wrap ".*main.  .., function, $hex.*" {
> +		    pass "$test_name (pattern 2)"
> +		}
> +	}
> +	gdb_test "shell rm -f $psymbols_output" ".*" \
> +	    "${test_name}: shell rm -f psymbols_output"
>       }
>   }
>   
>   
>   set msymbols_output [standard_output_file msymbols_output]
>   set msymbols_output_re [string_to_regexp $msymbols_output]
> -send_gdb "maint print msymbols -objfile ${binfile} $msymbols_output\n"
> -gdb_expect  {
> -    -re "^maint print msymbols \[^\n\]*\r\n$gdb_prompt $" {
> -	send_gdb "shell ls $msymbols_output\n"
> -	gdb_expect {
> -	    -re "$msymbols_output_re\r\n$gdb_prompt $" {
> -		gdb_test "shell grep factorial $msymbols_output" \
> -		    "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
> -		    "maint print msymbols, absolute pathname"
> -		gdb_test "shell rm -f $msymbols_output" ".*" \
> -		    "shell rm -f msymbols_output"
> -	    }
> -	    -re ".*$gdb_prompt $" { fail "maint print msymbols" }
> -	    timeout { fail "maint print msymbols (timeout)" }
> -	}
> -    }
> -    -re ".*$gdb_prompt $" { fail "maint print msymbols" }
> -    timeout { fail "maint print msymbols (timeout)" }
> -}
> +gdb_test_no_output "maint print msymbols -objfile ${binfile} $msymbols_output" \
> +    "print msymbols to file, with absolute path"
> +gdb_test "shell grep factorial $msymbols_output" \
> +    "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
> +    "maint print msymbols, absolute pathname"
> +gdb_test "shell rm -f $msymbols_output" ".*" "remove absolute path msymbols"
>   
>   # Check that maint print msymbols allows relative pathnames
>   set mydir [pwd]
> @@ -336,18 +304,13 @@ gdb_test "cd [standard_output_file {}]" \
>       "Working directory .*\..*" \
>       "cd to objdir"
>   
> -gdb_test_multiple "maint print msymbols -objfile ${testfile} msymbols_output2" "maint print msymbols" {
> -    -re "^maint print msymbols \[^\n\]*\r\n$gdb_prompt $" {
> -    	gdb_test_multiple "shell ls msymbols_output2" "maint print msymbols" {
> -	    -re "msymbols_output2\r\n$gdb_prompt $" {
> -		gdb_test "shell grep factorial msymbols_output2" \
> -		    "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
> -		    "maint print msymbols, relative pathname"
> -		gdb_test "shell rm -f msymbols_output2" ".*"
> -	    }
> -	}
> -    }
> -}
> +gdb_test_no_output "maint print msymbols -objfile ${testfile} $msymbols_output"\
> +    "print msymbols to file, with relative path"
> +gdb_test "shell grep factorial $msymbols_output" \
> +    "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
> +    "maint print msymbols, relative pathname"
> +gdb_test "shell rm -f msymbols_output" ".*" "remove relative path msymbols"
> +
>   gdb_test "cd ${mydir}" \
>       "Working directory [string_to_regexp ${mydir}]\..*" \
>       "cd to mydir"
> @@ -365,31 +328,12 @@ set test_list [list \
>   		   "maint print symbols -pc" \
>   		   "maint print symbols -pc main $symbols_output"]
>   foreach { test_name command } $test_list {
> -    send_gdb "$command\n"
> -    gdb_expect {
> -	-re "^maint print symbols \[^\n\]*\r\n$gdb_prompt $" {
> -	    send_gdb "shell ls $symbols_output\n"
> -	    gdb_expect {
> -		-re "$symbols_output_re\r\n$gdb_prompt $" {
> -		    # See comments for `maint print psymbols'.
> -		    send_gdb "shell grep 'main(.*block' $symbols_output\n"
> -		    gdb_expect {
> -			-re "int main\\(int, char \\*\\*, char \\*\\*\\); block.*$gdb_prompt $" {
> -			    pass "$test_name"
> -			}
> -			-re ".*$gdb_prompt $" { fail "$test_name" }
> -			timeout { fail "$test_name (timeout)" }
> -		    }
> -		    gdb_test "shell rm -f $symbols_output" ".*" \
> -			"$test_name: shell rm -f symbols_output"
> -		}
> -		-re ".*$gdb_prompt $" { fail "$test_name" }
> -		timeout { fail "$test_name (timeout)" }
> -	    }
> -	}
> -	-re ".*$gdb_prompt $" { fail "$test_name" }
> -	timeout { fail "$test_name (timeout)" }
> -    }
> +    gdb_test_no_output "$command" "$test_name generate"
> +    gdb_test "shell grep 'main(.*block' $symbols_output"\
> +	"int main\\(int, char \\*\\*, char \\*\\*\\); block.*"\
> +	"$test_name read"
> +    gdb_test "shell rm -f $symbols_output" ".*" \
> +	"$test_name: shell rm -f symbols_output"
>   }
>   
>   set msg "maint print type"


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PING][PATCHv4] gdb/testsuite: modernize gdb.base/maint.exp
  2022-07-13 11:22   ` [PING][PATCHv4] " Bruno Larsen
@ 2022-07-15 16:49     ` Tom Tromey
  2022-07-15 17:20       ` Bruno Larsen
  0 siblings, 1 reply; 25+ messages in thread
From: Tom Tromey @ 2022-07-15 16:49 UTC (permalink / raw)
  To: Bruno Larsen via Gdb-patches

>> gdb.base/maint.exp was using several gdb_expect statements, probably
>> because this test case predates the existance of gdb_test_multiple. This
>> commit updates the test case to use gdb_test_multiple, making it more
>> resilient to internal errors and such.
>> The only gdb_expect left in the testcase is one that specifically
>> looks
>> for an internal error being triggered as a PASS.

Thank you.  I think this is ok.

Tom

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PING][PATCHv4] gdb/testsuite: modernize gdb.base/maint.exp
  2022-07-15 16:49     ` Tom Tromey
@ 2022-07-15 17:20       ` Bruno Larsen
  0 siblings, 0 replies; 25+ messages in thread
From: Bruno Larsen @ 2022-07-15 17:20 UTC (permalink / raw)
  To: Tom Tromey, Bruno Larsen via Gdb-patches


On 7/15/22 13:49, Tom Tromey wrote:
>>> gdb.base/maint.exp was using several gdb_expect statements, probably
>>> because this test case predates the existance of gdb_test_multiple. This
>>> commit updates the test case to use gdb_test_multiple, making it more
>>> resilient to internal errors and such.
>>> The only gdb_expect left in the testcase is one that specifically
>>> looks
>>> for an internal error being triggered as a PASS.
> 
> Thank you.  I think this is ok.

thanks! Pushed


Cheers!
Bruno Larsen
> 
> Tom
> 


^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2022-07-15 17:20 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-09 18:04 [PATCH] gdb/testsuite: modernize gdb.base/maint.exp Bruno Larsen
2022-05-23 14:14 ` [PING] " Bruno Larsen
2022-05-30 11:14   ` [PINGv2] " Bruno Larsen
2022-06-06 12:43     ` [PINGv3] " Bruno Larsen
2022-06-13 20:03       ` [PINGv4] " Bruno Larsen
2022-06-20 13:29         ` [PINGv5] " Bruno Larsen
2022-06-21 15:52 ` Andrew Burgess
2022-06-21 19:45   ` Bruno Larsen
2022-06-21 19:54     ` [PATCHv2] " Bruno Larsen
2022-06-24 13:20     ` [PATCH] " Andrew Burgess
2022-06-24 15:13       ` Pedro Alves
2022-06-24 15:16     ` Pedro Alves
2022-06-24 15:23     ` Pedro Alves
2022-06-24 15:22   ` Pedro Alves
2022-06-24 17:51     ` Keith Seitz
2022-06-24 17:54       ` Bruno Larsen
2022-06-24 18:41       ` Pedro Alves
2022-06-27 10:13       ` Andrew Burgess
2022-06-28 18:36 ` [PATCHv3] " Bruno Larsen
2022-06-29 10:33   ` Andrew Burgess
2022-06-29 14:00     ` Bruno Larsen
2022-06-29 14:53 ` [PATCHv4] " Bruno Larsen
2022-07-13 11:22   ` [PING][PATCHv4] " Bruno Larsen
2022-07-15 16:49     ` Tom Tromey
2022-07-15 17:20       ` Bruno Larsen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).