[RFA 2/4] Test prefix-define.

[Simon Marchi]

The following test name is duplicated:

2 PASS: gdb.base/prefix-define.exp: An empty user defined prefix command cannot be executed

On 2019-09-29 4:54 p.m., Philippe Waroquiers wrote:
> Adds a test testing the new prefix-define command.
> 
> 2019-08-03  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> 
> 	* gdb.base/prefix-define.exp: New file.
> ---
>  gdb/testsuite/gdb.base/prefix-define.exp | 144 +++++++++++++++++++++++
>  1 file changed, 144 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.base/prefix-define.exp
> 
> diff --git a/gdb/testsuite/gdb.base/prefix-define.exp b/gdb/testsuite/gdb.base/prefix-define.exp
> new file mode 100644
> index 0000000000..81719204b4
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/prefix-define.exp
> @@ -0,0 +1,144 @@
> +# This testcase is part of GDB, the GNU debugger.
> +
> +# Copyright 2019 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +
> +# This test verifies the "prefix-define" command.
> +
> +gdb_start
> +
> +# disable pagination
> +gdb_test_no_output "set height 0" "disable pagination"

I think this is already done in default_gdb_start, so it might be unnecessary.

> +
> +####################
> +# Verify the definition of user defined prefix commands.
> +# Test the definition of a command using these prefixes.
> +
> +# Define an empty prefix command.
> +gdb_test_no_output "prefix-define abc-prefix"
> +
> +# Verify an empty prefix command cannot be used as a command.
> +gdb_test "abc-prefix" \
> +    "\"abc-prefix\" must be followed by the name of an abc-prefix command.*" \
> +    "An empty user defined prefix command cannot be executed"

Here I would suggest the simpler test name "execute empty prefix command".

The test names are generally not sentences, and therefore do not start with a capital letter:

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

> +
> +# Define a sub-prefix command.
> +gdb_test_no_output "prefix-define abc-prefix def-prefix"
> +
> +# Define ghi-cmd using the prefixes.
> +gdb_test_multiple "define abc-prefix def-prefix ghi-cmd" \
> +    "define user command: abc-prefix def-prefix ghi-cmd" {
> +	-re "Type commands for definition of \"abc-prefix def-prefix ghi-cmd\".\r\nEnd with a line saying just \"end\".\r\n>$" {
> +	    gdb_test "echo command ghi-cmd\\n\nend" "" \
> +		"define user command: ghi-cmd"
> +	}
> +    }
> +# Verify ghi-cmd works.
> +gdb_test "abc-prefix def-prefix ghi-cmd" \
> +    "command ghi-cmd" \
> +    "use user command: ghi-cmd"
> +
> +
> +####################
> +# Verify an existing (empty) command can be marked as a prefix command.
> +# Then verify an empty prefix command can be transformed into an executable command.
> +
> +# Define ghi-prefix-cmd as an empty command.
> +gdb_test_multiple "define abc-prefix def-prefix ghi-prefix-cmd" \
> +    "define user command: abc-prefix def-prefix ghi-prefix-cmd" {
> +	-re "Type commands for definition of \"abc-prefix def-prefix ghi-prefix-cmd\".\r\nEnd with a line saying just \"end\".\r\n>$" {
> +	    gdb_test "end" "" \
> +		"define user command: ghi-prefix-cmd"
> +	}
> +    }
> +gdb_test_no_output "prefix-define abc-prefix def-prefix ghi-prefix-cmd"
> +
> +# Verify an empty prefix command cannot be used as a command.
> +gdb_test "abc-prefix def-prefix ghi-prefix-cmd" \
> +    "\"abc-prefix def-prefix ghi-prefix-cmd\" must be followed by the name of an ghi-prefix-cmd command.*" \
> +    "An empty user defined prefix command cannot be executed"
> +
> +# Define jkl-cmd command.
> +gdb_test_multiple "define abc-prefix def-prefix ghi-prefix-cmd jkl-cmd" \
> +    "define user command: abc-prefix def-prefix ghi-prefix-cmd jkl-cmd" {
> +	-re "Type commands for definition of \"abc-prefix def-prefix ghi-prefix-cmd jkl-cmd\".\r\nEnd with a line saying just \"end\".\r\n>$" {
> +	    gdb_test "echo command jkl-cmd\\n\nend" "" \
> +		"define user command: jkl-cmd"
> +	}
> +    }
> +# Verify jkl-cmd command works.
> +gdb_test "abc-prefix def-prefix ghi-prefix-cmd jkl-cmd" \
> +    "command jkl-cmd" \
> +    "use user command: jkl-cmd"
> +
> +# Define alternate-jkl-cmd and check it works.
> +# Define alternate-jkl-cmd command.
> +gdb_test_multiple "define abc-prefix def-prefix ghi-prefix-cmd alternate-jkl-cmd" \
> +    "define user command: abc-prefix def-prefix ghi-prefix-cmd alternate-jkl-cmd" {
> +	-re "Type commands for definition of \"abc-prefix def-prefix ghi-prefix-cmd alternate-jkl-cmd\".\r\nEnd with a line saying just \"end\".\r\n>$" {
> +	    gdb_test "echo command alternate-jkl-cmd\\n\nend" "" \
> +		"define user command: alternate-jkl-cmd"
> +	}
> +    }
> +# Verify alternate-jkl-cmd command works.
> +gdb_test "abc-prefix def-prefix ghi-prefix-cmd alternate-jkl-cmd" \
> +    "command alternate-jkl-cmd" \
> +    "use user command: alternate-jkl-cmd"
> +
> +
> +
> +# Now redefine ghi-prefix-cmd as a real command, and check it is working.
> +send_gdb "define abc-prefix def-prefix ghi-prefix-cmd\n"
> +gdb_expect {
> +  -re "Redefine command \"ghi-prefix-cmd\".*y or n. $"\
> +          {send_gdb "y\n"
> +           gdb_expect {
> +             -re "Type commands for definition of \"abc-prefix def-prefix ghi-prefix-cmd\".\r\nEnd with a line saying just \"end\".\r\n>$"\
> +                     {send_gdb "echo redefined command ghi-prefix-cmd\\n\nend\n"
> +                      gdb_expect {
> +                        -re "$gdb_prompt $"\
> +                                {pass "redefine user command: ghi-prefix-cmd"}
> +                        timeout {fail "(timeout) redefine user command: ghi-prefix-cmd"}
> +                      }
> +                     }
> +             timeout {fail "(timeout) redefine user command: ghi-prefix-cmd"}
> +           }
> +          }
> +  -re "$gdb_prompt $"\
> +          {fail "redefine user command: ghi-prefix-cmd"}
> +  timeout {fail "(timeout) redefine user command: ghi-prefix-cmd"}
> +}

That reminds me something I forgot to mention in review of patch 1, so I'll mention
it here.  If found it a bit surprising that when you do:

- prefix-define foo
- define foo

it asks you "Redefine command "foo"?".  I think it's misleading to ask whether I want
to "redefine" the command, when it was never really define to begin with.  As a user,
I would be a bit scared that it would scrap all the subcommands I have created.

>From what I understand, defining "foo" at this point will actually not overwrite
anything.

If there are already some commands associated to foo (it has already been "define"'d),
then I think it makes sense to warn the user about the redefinition, as usual.

About the formatting of the code above, it is a bit inconsistent.  Make sure to indent
by 4 columns and use tabs to replace leading groups of 8 spaces.

Also, we usually put the { on the same line as the "-re", like this:

gdb_expect {
    -re "Redefine command \"ghi-prefix-cmd\".*y or n. $" {
	send_gdb "y\n"

and not like this:

gdb_expect {
  -re "Redefine command \"ghi-prefix-cmd\".*y or n. $"\
          {send_gdb "y\n"

> +
> +# Verify ghi-prefix-cmd command works.
> +gdb_test "abc-prefix def-prefix ghi-prefix-cmd" \
> +    "redefined command ghi-prefix-cmd" \
> +    "use redefined user command: ghi-prefix-cmd"
> +
> +# Check jkl-cmd still works.
> +gdb_test "abc-prefix def-prefix ghi-prefix-cmd jkl-cmd" \
> +    "command jkl-cmd" \
> +    "use user command after redefining ghi-prefix-cmd: jkl-cmd"
> +
> +# Check alternate-jkl-cmd still works.
> +gdb_test "abc-prefix def-prefix ghi-prefix-cmd alternate-jkl-cmd" \
> +    "command alternate-jkl-cmd" \
> +    "use user command after redefining ghi-prefix-cmd: alternate-jkl-cmd"
> +
> +####################
> +# Check error behaviour.
> +
> +gdb_test "prefix-define print" ".* is built-in.*"
> +

An error behavior to test here would be to try to define a prefix command with
a parent that does not yet exist, like:

(gdb) prefix-define foo bar baz
Undefined command: "foo".  Try "help".

Simon