[gdb/testsuite] Add -cooked pattern flag to gdb_test_multiple

  [ was: Re: [PATCH][gdb/testsuite] Introduce gdb_test_ext ]

On 19-09-19 21:24, Andrew Burgess wrote:
> On further thought, I actually think there's no need for an extra
> function at all, we can get all the benefit (as I see it) by possibly
> updating gdb_test_multiple.  I'm travelling right now so can't code
> this up, but I think a solution that does something like this:
> 
>      gdb_test_multiple "command" "test name" {
>        -re "full regexp here$gdb_prompt" {
>          pass $gdb_test_multiple_name
>        }
>        -output "pattern without prompt" {
>          fail $gdb_test_multiple_name
>        }
>      }
> 
> So using '-re' and '-output' to specialise the behaviour of
> gdb_test_multiple, and adding in the $gdb_test_multiple_name variable.
> 
> When I get back to my desk I'll try to code this up.

Hi,

I took a stab at this. I'm not sure about the naming though.

For the pattern flag I used the name -cooked. Perhaps -wrap is better?
Any better suggestions?

I used gdb_test_multiple_message (using the 'message' postfix because it
matched the name of the gdb_test_multiple argument) for the convenience
variable, but it's a tad long, perhaps we could abbreviate to 'gtm_message'?

Thanks,
- Tom
  

* Tom de Vries <tdevries@suse.de> [2019-10-05 08:05:03 +0200]:

> [ was: Re: [PATCH][gdb/testsuite] Introduce gdb_test_ext ]
> 
> On 19-09-19 21:24, Andrew Burgess wrote:
> > On further thought, I actually think there's no need for an extra
> > function at all, we can get all the benefit (as I see it) by possibly
> > updating gdb_test_multiple.  I'm travelling right now so can't code
> > this up, but I think a solution that does something like this:
> > 
> >      gdb_test_multiple "command" "test name" {
> >        -re "full regexp here$gdb_prompt" {
> >          pass $gdb_test_multiple_name
> >        }
> >        -output "pattern without prompt" {
> >          fail $gdb_test_multiple_name
> >        }
> >      }
> > 
> > So using '-re' and '-output' to specialise the behaviour of
> > gdb_test_multiple, and adding in the $gdb_test_multiple_name variable.
> > 
> > When I get back to my desk I'll try to code this up.
> 
> Hi,
> 
> I took a stab at this. I'm not sure about the naming though.
> 
> For the pattern flag I used the name -cooked. Perhaps -wrap is better?
> Any better suggestions?
> 
> I used gdb_test_multiple_message (using the 'message' postfix because it
> matched the name of the gdb_test_multiple argument) for the convenience
> variable, but it's a tad long, perhaps we could abbreviate to
> 'gtm_message'?

This duplicates work in this patch:

  https://sourceware.org/ml/gdb-patches/2019-10/msg00023.html

I was just about to merge that patch, but I'll leave it for a couple
of days in case you'd like to provide feedback.

Pedro's suggestion, which I was going to adopt was 'gdb_test_name'.

Thanks,
Andrew




> 
> Thanks,
> - Tom

> [gdb/testsuite] Add -cooked pattern flag to gdb_test_multiple
> 
> Currently, in order to rewrite:
> ...
> gdb_test <command> <pattern> <message>
> ...
> using gdb_test_multiple, we get:
> ...
> set msg <message>
> gdb_test_multiple <command> $msg {
>     -re "\[\r\n\]*(?:<pattern>)\[\r\n\]+$gdb_prompt $" {
>     	pass $msg
>     }
> }
> ...
> 
> Add to gdb_test_multiple:
> - a '-cooked' pattern flag, that wraps the regexp pattern as gdb_test wraps
>   its message argument.
> - a convenience variable gdb_test_multiple_message
> 
> This allows us to rewrite into the more compact:
> ...
> gdb_test_multiple <command> <message> {
>     -re -cooked <pattern> {
>         pass $gdb_test_multiple_message
>     }
> }
> ...
> 
> Tested on x86_64-linux.
> 
> gdb/testsuite/ChangeLog:
> 
> 2019-10-04  Tom de Vries  <tdevries@suse.de>
> 
> 	* lib/gdb.exp (gdb_test_multiple): Add -cooked pattern flag and
> 	convenience variable gdb_test_multiple_message.
> 	* gdb.reverse/step-precsave.exp: Rewrite gdb_test_multiple containing
> 	kfail using -cooked pattern flag and convenience variable
> 	gdb_test_multiple_message.
> 
> ---
>  gdb/testsuite/gdb.reverse/step-precsave.exp | 13 +++++--------
>  gdb/testsuite/lib/gdb.exp                   | 25 ++++++++++++++++++++++++-
>  2 files changed, 29 insertions(+), 9 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.reverse/step-precsave.exp b/gdb/testsuite/gdb.reverse/step-precsave.exp
> index 2073b8a1542..aaddd3b4ada 100644
> --- a/gdb/testsuite/gdb.reverse/step-precsave.exp
> +++ b/gdb/testsuite/gdb.reverse/step-precsave.exp
> @@ -46,15 +46,12 @@ gdb_test "break $end_of_main" \
>  
>  # This can take awhile.
>  with_timeout_factor 20 {
> -    set test "run to end of main"
> -    set pass_pattern "Breakpoint .* end of main .*"
> -    set kfail_pattern "Process record does not support instruction 0xc5 at.*"
> -    gdb_test_multiple "continue" $test {
> -	-re "\[\r\n\]*(?:$pass_pattern)\[\r\n\]+$gdb_prompt $" {
> -	    pass $test
> +    gdb_test_multiple "continue" "run to end of main" {
> +	-re -cooked "Breakpoint .* end of main .*" {
> +	    pass $gdb_test_multiple_message
>  	}
> -	-re "\[\r\n\]*(?:$kfail_pattern)\[\r\n\]+$gdb_prompt $" {
> -	    kfail "record/23188" $test
> +	-re -cooked "Process record does not support instruction 0xc5 at.*" {
> +	    kfail "record/23188" $gdb_test_multiple_message
>  	}
>      }
>  }
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 3a1f053cf8a..4a47ddc9d37 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -772,6 +772,11 @@ proc gdb_test_multiple { command message user_code { prompt_regexp "" } } {
>  	error "Invalid newline in \"$message\" test"
>      }
>  
> +    upvar 1 gdb_test_multiple_message gdb_test_multiple_message
> +    if { [info exists gdb_test_multiple_message] } {
> +	set gdb_test_multiple_message_save $gdb_test_multiple_message
> +    }
> +
>      if {$use_gdb_stub
>  	&& [regexp -nocase {^\s*(r|run|star|start|at|att|atta|attac|attach)\M} \
>  	    $command]} {
> @@ -807,6 +812,7 @@ proc gdb_test_multiple { command message user_code { prompt_regexp "" } } {
>      set patterns ""
>      set expecting_action 0
>      set expecting_arg 0
> +    set cooked 0
>      foreach item $user_code subst_item $subst_code {
>  	if { $item == "-n" || $item == "-notransfer" || $item == "-nocase" } {
>  	    lappend processed_code $item
> @@ -821,6 +827,10 @@ proc gdb_test_multiple { command message user_code { prompt_regexp "" } } {
>  	    lappend processed_code $item
>  	    continue
>  	}
> +	if { $item == "-cooked" } {
> +	    set cooked 1
> +	    continue
> +	}
>  	if { $expecting_arg } {
>  	    set expecting_arg 0
>  	    lappend processed_code $subst_item
> @@ -834,7 +844,14 @@ proc gdb_test_multiple { command message user_code { prompt_regexp "" } } {
>  	    continue
>  	}
>  	set expecting_action 1
> -	lappend processed_code $subst_item
> +	if { $cooked } {
> +	    # Wrap subst_item as is done for the gdb_test PATTERN argument.
> +	    lappend processed_code \
> +		"\[\r\n\]*(?:$subst_item)\[\r\n\]+$gdb_prompt $"
> +	    set cooked 0
> +	} else {
> +	    lappend processed_code $subst_item
> +	}
>  	if {$patterns != ""} {
>  	    append patterns "; "
>  	}
> @@ -1038,8 +1055,14 @@ proc gdb_test_multiple { command message user_code { prompt_regexp "" } } {
>  	}
>      }
>  
> +    set gdb_test_multiple_message $message
>      set result 0
>      set code [catch {gdb_expect $code} string]
> +    if { [info exists gdb_test_multiple_message_save] } {
> +	set gdb_test_multiple_message $gdb_test_multiple_message_save
> +    } else {
> +	unset gdb_test_multiple_message
> +    }
>      if {$code == 1} {
>  	global errorInfo errorCode
>  	return -code error -errorinfo $errorInfo -errorcode $errorCode $string
  

On 05-10-19 17:38, Andrew Burgess wrote:
> * Tom de Vries <tdevries@suse.de> [2019-10-05 08:05:03 +0200]:
> 
>> [ was: Re: [PATCH][gdb/testsuite] Introduce gdb_test_ext ]
>>
>> On 19-09-19 21:24, Andrew Burgess wrote:
>>> On further thought, I actually think there's no need for an extra
>>> function at all, we can get all the benefit (as I see it) by possibly
>>> updating gdb_test_multiple.  I'm travelling right now so can't code
>>> this up, but I think a solution that does something like this:
>>>
>>>      gdb_test_multiple "command" "test name" {
>>>        -re "full regexp here$gdb_prompt" {
>>>          pass $gdb_test_multiple_name
>>>        }
>>>        -output "pattern without prompt" {
>>>          fail $gdb_test_multiple_name
>>>        }
>>>      }
>>>
>>> So using '-re' and '-output' to specialise the behaviour of
>>> gdb_test_multiple, and adding in the $gdb_test_multiple_name variable.
>>>
>>> When I get back to my desk I'll try to code this up.
>>
>> Hi,
>>
>> I took a stab at this. I'm not sure about the naming though.
>>
>> For the pattern flag I used the name -cooked. Perhaps -wrap is better?
>> Any better suggestions?
>>
>> I used gdb_test_multiple_message (using the 'message' postfix because it
>> matched the name of the gdb_test_multiple argument) for the convenience
>> variable, but it's a tad long, perhaps we could abbreviate to
>> 'gtm_message'?
> 
> This duplicates work in this patch:
> 
>   https://sourceware.org/ml/gdb-patches/2019-10/msg00023.html
> 

I see, that's unfortunate. [ FWIW, it would have helped me if you would
have submitted continuing this thread, or cc-ed me on the new thread, or
mentioned the new thread at this thread. ]

> I was just about to merge that patch, but I'll leave it for a couple
> of days in case you'd like to provide feedback.
> 
> Pedro's suggestion, which I was going to adopt was 'gdb_test_name'.

OK, I'll take a look and follow up at the new thread if I have any comments.

Thanks,
- Tom
  

* Tom de Vries <tdevries@suse.de> [2019-10-06 08:37:31 +0200]:

> On 05-10-19 17:38, Andrew Burgess wrote:
> > * Tom de Vries <tdevries@suse.de> [2019-10-05 08:05:03 +0200]:
> > 
> >> [ was: Re: [PATCH][gdb/testsuite] Introduce gdb_test_ext ]
> >>
> >> On 19-09-19 21:24, Andrew Burgess wrote:
> >>> On further thought, I actually think there's no need for an extra
> >>> function at all, we can get all the benefit (as I see it) by possibly
> >>> updating gdb_test_multiple.  I'm travelling right now so can't code
> >>> this up, but I think a solution that does something like this:
> >>>
> >>>      gdb_test_multiple "command" "test name" {
> >>>        -re "full regexp here$gdb_prompt" {
> >>>          pass $gdb_test_multiple_name
> >>>        }
> >>>        -output "pattern without prompt" {
> >>>          fail $gdb_test_multiple_name
> >>>        }
> >>>      }
> >>>
> >>> So using '-re' and '-output' to specialise the behaviour of
> >>> gdb_test_multiple, and adding in the $gdb_test_multiple_name variable.
> >>>
> >>> When I get back to my desk I'll try to code this up.
> >>
> >> Hi,
> >>
> >> I took a stab at this. I'm not sure about the naming though.
> >>
> >> For the pattern flag I used the name -cooked. Perhaps -wrap is better?
> >> Any better suggestions?
> >>
> >> I used gdb_test_multiple_message (using the 'message' postfix because it
> >> matched the name of the gdb_test_multiple argument) for the convenience
> >> variable, but it's a tad long, perhaps we could abbreviate to
> >> 'gtm_message'?
> > 
> > This duplicates work in this patch:
> > 
> >   https://sourceware.org/ml/gdb-patches/2019-10/msg00023.html
> > 
> 
> I see, that's unfortunate. [ FWIW, it would have helped me if you would
> have submitted continuing this thread, or cc-ed me on the new thread, or
> mentioned the new thread at this thread. ]
> 
> > I was just about to merge that patch, but I'll leave it for a couple
> > of days in case you'd like to provide feedback.
> > 
> > Pedro's suggestion, which I was going to adopt was 'gdb_test_name'.
> 
> OK, I'll take a look and follow up at the new thread if I have any comments.

Apologies, I'll try to do better next time.

I have now pushed the gdb_test_name patch.

Thanks,
Andrew