[RFA,2/2] Modify gdb.base/commands.exp to test multi breakpoint command setting/clearing.

  gdb/testsuite/ChangeLog
2018-08-02  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* gdb.base/commands.exp: Test multi breakpoints setting
	and clearing.
---
 gdb/testsuite/gdb.base/commands.exp | 44 +++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)
  

Hi Philippe,

On 08/02/2018 10:26 PM, Philippe Waroquiers wrote:
> gdb/testsuite/ChangeLog
> 2018-08-02  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> 
> 	* gdb.base/commands.exp: Test multi breakpoints setting
> 	and clearing.
> ---
>  gdb/testsuite/gdb.base/commands.exp | 44 +++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/gdb/testsuite/gdb.base/commands.exp b/gdb/testsuite/gdb.base/commands.exp
> index 259b89b803..57d9348244 100644
> --- a/gdb/testsuite/gdb.base/commands.exp
> +++ b/gdb/testsuite/gdb.base/commands.exp
> @@ -281,6 +281,49 @@ proc_with_prefix breakpoint_command_test {} {
>      gdb_test "print value" " = 5"
>  }
>  
> +# Test clearing the commands of several breakpoints with one single "end".
> +# As this test uses breakpoint numbers, we better run it first to ensure
> +# the breakpoint numbers are not changing if other tests are added/changed
> +# so that breakpoint numbers are also changed.

The "are not changing" here gave me pause.  I think you meant "do not change".

I'd suggest:

 # Test clearing the commands of several breakpoints with one single "end".
 # As this test hardcodes breakpoint numbers, we better run it first to ensure
 # the breakpoint numbers remain stable, as other tests can also create
 # breakpoints.

But, I think you could simply remove this requirement.
You can for example get the first breakpoint number with:

    gdb_test "break factorial" "Breakpoint.*at.*"
    set bpnum [get_integer_valueof "\$bpnum" 0]

Or in a single shot:

        set bpnum 0
        gdb_test_multiple "break main" "set breakpoint on main" {
            -re "Breakpoint ($decimal) at .*$gdb_prompt $" {
                set bpnum $expect_out(1,string)
            }
        }

> +proc_with_prefix run_me_first_breakpoint_clear_command_test {} {
> +    # The below creates breakpoint nr 1.
> +    runto_or_return factorial
> +
> +    set any "\[^\r\n\]*"
> +    delete_breakpoints
> +    gdb_test "break factorial" "Breakpoint.*at.*"
> +    gdb_test "break main" "Breakpoint.*at.*"
> +    gdb_test \
> +	[multi_line_input \
> +	     {commands 2 3} \
> +	     {  print 1234321} \
> +	     {end}] \
> +	"End with.*" \
> +	"commands"
> +    gdb_test "info breakpoints" \
> +	[multi_line \
> +	     "${any}What${any}" \
> +	     "${any}in factorial${any}" \
> +	     "${any}print 1234321${any}" \
> +	     "${any}in main${any}" \
> +	     "${any}print 1234321${any}" \
> +	    ] \
> +	"check print 1234321 is there."

No period at end of test name.  While at it, "check" is redundant,
since all tests are checks.

> +    gdb_test \
> +	[multi_line_input \
> +	     {commands 2 3} \
> +	     {end}] \
> +	"End with.*" \
> +	"commands"
> +    gdb_test "info breakpoints" \
> +	[multi_line \
> +	     "${any}What${any}" \
> +	     "${any}in factorial${any}" \
> +	     "${any}in main${any}" \
> +	    ] \
> +	"check print 1234321 is not there anymore."

Ditto.

Also, you have a duplicated test name above: the "commands" tests:

 https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Make_sure_test_messages_are_unique

> +    }
> +
>  # Test a simple user defined command (with arguments)
>  proc_with_prefix user_defined_command_test {} {
>      global valnum_re
> @@ -1125,6 +1168,7 @@ proc_with_prefix backslash_in_multi_line_command_test {} {
>      gdb_test "print 1" "" "run command"
>  }
>  
> +run_me_first_breakpoint_clear_command_test
>  gdbvar_simple_if_test
>  gdbvar_simple_while_test
>  gdbvar_complex_if_while_test
> 

Thanks,
Pedro Alves