[03/17] Fix silent gdb.base/annota1.exp test coverage regression

Message ID 20210603190243.2609886-4-pedro@palves.net
State New
Headers show
Series
  • Interrupting programs that block/ignore SIGINT
Related show

Commit Message

Pedro Alves June 3, 2021, 7:02 p.m.
This commit fixes a test coverage regression caused by:

 commit b001de2320446ec803b4ee5e0b9710b025b84469
 Author:     Andrew Burgess <andrew.burgess@embecosm.com>
 AuthorDate: Mon Nov 26 17:56:39 2018 +0000
 Commit:     Andrew Burgess <andrew.burgess@embecosm.com>
 CommitDate: Wed Dec 12 17:33:52 2018 +0000

     gdb: Update test pattern to deal with native-extended-gdbserver

While looking at a regression caused by a local patch I was working
on, I noticed this:

 pre-prompt
 (gdb)
 prompt
 PASS: gdb.base/annota1.exp: breakpoint info
 PASS: gdb.base/annota1.exp: run until main breakpoint
 run

 post-prompt
 Starting program: /home/pedro/gdb/mygit/build/gdb/testsuite/outputs/gdb.base/annota1/annota1
 next

Note how above, we get the "run until main breakpoint" pass even
before "run" shows up in the log!  The issue is that the test isn't
really testing anything, it always passes regardless of the gdb
output.

There are a few problems here, fixed by this commit:

 - using {} to build the list for [join], when the strings we're
   joining include variable names that must be expanded.  Such list
   need to be built with [list] instead.

 - [join] joins strings with a space character by default.  We need to
   pass the empty string as second parameter so that it just concats
   the strings.

 - doing too much in a "-re" (calling procedures), which doesn't work
   correctly.  I've seen this before and never digged deeper into why.
   Probably something to do with how gdb_test_multiple is implemented.
   Regardless, easy and clear enough to build the pattern first into a
   variable.

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <pedro@palves.net>

	* gdb.base/annota1.exp: Build list using [list] instead of {}.
	Tell [join] to join with no character.  Build expected pattern in
	separate variable instead of in the -re expression directly.

Change-Id: Ib3c89290f0e9ae4a0a43422853fcd4a7a7e12b18
---
 gdb/testsuite/gdb.base/annota1.exp | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

-- 
2.26.2

Comments

Andrew Burgess June 3, 2021, 7:28 p.m. | #1
* Pedro Alves <pedro@palves.net> [2021-06-03 20:02:29 +0100]:

> This commit fixes a test coverage regression caused by:

> 

>  commit b001de2320446ec803b4ee5e0b9710b025b84469

>  Author:     Andrew Burgess <andrew.burgess@embecosm.com>

>  AuthorDate: Mon Nov 26 17:56:39 2018 +0000

>  Commit:     Andrew Burgess <andrew.burgess@embecosm.com>

>  CommitDate: Wed Dec 12 17:33:52 2018 +0000

> 

>      gdb: Update test pattern to deal with native-extended-gdbserver

> 

> While looking at a regression caused by a local patch I was working

> on, I noticed this:

> 

>  pre-prompt

>  (gdb)

>  prompt

>  PASS: gdb.base/annota1.exp: breakpoint info

>  PASS: gdb.base/annota1.exp: run until main breakpoint

>  run

> 

>  post-prompt

>  Starting program: /home/pedro/gdb/mygit/build/gdb/testsuite/outputs/gdb.base/annota1/annota1

>  next

> 

> Note how above, we get the "run until main breakpoint" pass even

> before "run" shows up in the log!  The issue is that the test isn't

> really testing anything, it always passes regardless of the gdb

> output.

> 

> There are a few problems here, fixed by this commit:

> 

>  - using {} to build the list for [join], when the strings we're

>    joining include variable names that must be expanded.  Such list

>    need to be built with [list] instead.

> 

>  - [join] joins strings with a space character by default.  We need to

>    pass the empty string as second parameter so that it just concats

>    the strings.

> 

>  - doing too much in a "-re" (calling procedures), which doesn't work

>    correctly.  I've seen this before and never digged deeper into why.

>    Probably something to do with how gdb_test_multiple is implemented.

>    Regardless, easy and clear enough to build the pattern first into a

>    variable.

> 

> gdb/testsuite/ChangeLog:

> yyyy-mm-dd  Pedro Alves  <pedro@palves.net>

> 

> 	* gdb.base/annota1.exp: Build list using [list] instead of {}.

> 	Tell [join] to join with no character.  Build expected pattern in

> 	separate variable instead of in the -re expression directly.


LGTM.

Thanks,
Andrew


> 

> Change-Id: Ib3c89290f0e9ae4a0a43422853fcd4a7a7e12b18

> ---

>  gdb/testsuite/gdb.base/annota1.exp | 9 ++++++---

>  1 file changed, 6 insertions(+), 3 deletions(-)

> 

> diff --git a/gdb/testsuite/gdb.base/annota1.exp b/gdb/testsuite/gdb.base/annota1.exp

> index e2a9423c68b..3689f49fa1b 100644

> --- a/gdb/testsuite/gdb.base/annota1.exp

> +++ b/gdb/testsuite/gdb.base/annota1.exp

> @@ -126,8 +126,8 @@ gdb_test_multiple "info break" "breakpoint info" {

>  #

>  #exp_internal 1

>  set binexp [string_to_regexp $binfile]

> -gdb_test_multiple "run" "run until main breakpoint" {

> -    -re [join { "\r\n\032\032post-prompt\r\nStarting program: $binexp " \

> +

> +set run_re [join [list "\r\n\032\032post-prompt\r\nStarting program: $binexp " \

>  		    "\(\(\r\nReading \[^\r\n\]+\)|\(\r\nwarning: File transfers from remote targets can be slow\[^\r\n\]+\)\)*" \

>  		    "\(\r\nwarning: Skipping \[^\r\n\]+ .gdb_index section in \[^\r\n\]+\r\nDo \"set use-deprecated-index-sections on\" before the file is read\r\nto use the section anyway\\.\)?" \

>  		    "\(\(\r\n\r\n\032\032frames-invalid\)|\(\r\n\r\n\032\032breakpoints-invalid\)\)*\r\n\r\n" \

> @@ -146,7 +146,10 @@ gdb_test_multiple "run" "run until main breakpoint" {

>  		    "\032\032frame-source-end\r\n\r\n\r\n" \

>  		    "\032\032source.*$srcfile:$main_line:.*:beg:$hex\r\n\r\n" \

>  		    "\032\032frame-end\r\n\r\n" \

> -		    "\032\032stopped.*$gdb_prompt$" } ] {

> +		    "\032\032stopped.*$gdb_prompt$" ] "" ]

> +

> +gdb_test_multiple "run" "run until main breakpoint" {

> +    -re $run_re {

>  	pass $gdb_test_name

>      }

>  }

> -- 

> 2.26.2

>
Pedro Alves June 14, 2021, 8:30 p.m. | #2
On 2021-06-03 8:28 p.m., Andrew Burgess wrote:
> * Pedro Alves <pedro@palves.net> [2021-06-03 20:02:29 +0100]:


>> gdb/testsuite/ChangeLog:

>> yyyy-mm-dd  Pedro Alves  <pedro@palves.net>

>>

>> 	* gdb.base/annota1.exp: Build list using [list] instead of {}.

>> 	Tell [join] to join with no character.  Build expected pattern in

>> 	separate variable instead of in the -re expression directly.

> 

> LGTM.


Thanks, I'm applying this one.

Pedro Alves

Patch

diff --git a/gdb/testsuite/gdb.base/annota1.exp b/gdb/testsuite/gdb.base/annota1.exp
index e2a9423c68b..3689f49fa1b 100644
--- a/gdb/testsuite/gdb.base/annota1.exp
+++ b/gdb/testsuite/gdb.base/annota1.exp
@@ -126,8 +126,8 @@  gdb_test_multiple "info break" "breakpoint info" {
 #
 #exp_internal 1
 set binexp [string_to_regexp $binfile]
-gdb_test_multiple "run" "run until main breakpoint" {
-    -re [join { "\r\n\032\032post-prompt\r\nStarting program: $binexp " \
+
+set run_re [join [list "\r\n\032\032post-prompt\r\nStarting program: $binexp " \
 		    "\(\(\r\nReading \[^\r\n\]+\)|\(\r\nwarning: File transfers from remote targets can be slow\[^\r\n\]+\)\)*" \
 		    "\(\r\nwarning: Skipping \[^\r\n\]+ .gdb_index section in \[^\r\n\]+\r\nDo \"set use-deprecated-index-sections on\" before the file is read\r\nto use the section anyway\\.\)?" \
 		    "\(\(\r\n\r\n\032\032frames-invalid\)|\(\r\n\r\n\032\032breakpoints-invalid\)\)*\r\n\r\n" \
@@ -146,7 +146,10 @@  gdb_test_multiple "run" "run until main breakpoint" {
 		    "\032\032frame-source-end\r\n\r\n\r\n" \
 		    "\032\032source.*$srcfile:$main_line:.*:beg:$hex\r\n\r\n" \
 		    "\032\032frame-end\r\n\r\n" \
-		    "\032\032stopped.*$gdb_prompt$" } ] {
+		    "\032\032stopped.*$gdb_prompt$" ] "" ]
+
+gdb_test_multiple "run" "run until main breakpoint" {
+    -re $run_re {
 	pass $gdb_test_name
     }
 }