Re: [RFAv3 5/6] Test the | (pipe) command.

Pedro Alves
2019-05-27T17:49:42Z

On 5/4/19 5:17 PM, Philippe Waroquiers wrote:
> gdb/testsuite/ChangeLog
> 2019-05-04 Philippe Waroquiers <philippe.waroquiers@skynet.be>
>
> * gdb.base/shell.exp: Test pipe command, $_shell_exitcode,
> $_shell_exitsignal.
> * gdb.base/default.exp: Update for new convenience variables.
>
> Update default.exp

^^^^^^^^^^^^^^^^^^ spurious.

> ---
> gdb/testsuite/gdb.base/default.exp | 2 +
> gdb/testsuite/gdb.base/shell.exp | 68 +++++++++++++++++++++++++++++-
> 2 files changed, 69 insertions(+), 1 deletion(-)
>
> diff --git a/gdb/testsuite/gdb.base/default.exp b/gdb/testsuite/gdb.base/default.exp
> index 56ec917aa3..0325b8045d 100644
> --- a/gdb/testsuite/gdb.base/default.exp
> +++ b/gdb/testsuite/gdb.base/default.exp
> @@ -606,6 +606,8 @@ set show_conv_list \
> {$_isvoid = <internal function _isvoid>} \
> {$_gdb_major = 8} \
> {$_gdb_minor = 4} \
> + {$_shell_exitsignal = void} \
> + {$_shell_exitcode = 0} \
> }
> if ![skip_python_tests] {
> append show_conv_list \
> diff --git a/gdb/testsuite/gdb.base/shell.exp b/gdb/testsuite/gdb.base/shell.exp
> index 60d6e31e4f..9b85e4988b 100644
> --- a/gdb/testsuite/gdb.base/shell.exp
> +++ b/gdb/testsuite/gdb.base/shell.exp
> @@ -13,7 +13,7 @@
> # You should have received a copy of the GNU General Public License
> # along with this program. If not, see <http://www.gnu.org/licenses/>.
>
> -# Test that the shell and ! commands work.
> +# Test that the shell and ! and | and pipe commands work.

Too many ands. I'd suggest:

# Test that the "shell", "!", "|" and "pipe" commands work.

>
> gdb_exit
> gdb_start
> @@ -22,3 +22,69 @@ gdb_test "shell echo foo" "foo"
>
> gdb_test "! echo foo" "foo"
> gdb_test "!echo foo" "foo"
> +
> +# Convenience variables with shell command.
> +gdb_test "! exit 0" ""

This always passes regardless of output. Is that intended?
If not, use gdb_test_no_output. Ditto for all other similar
cases below.

> +gdb_test "p \$_shell_exitcode" " = 0" "shell success exitcode"
> +gdb_test "p \$_shell_exitsignal" " = void" "shell success exitsignal"
> +
> +gdb_test "! exit 1" ""
> +gdb_test "p \$_shell_exitcode" " = 1" "shell fail exitcode"
> +gdb_test "p \$_shell_exitsignal" " = void" "shell fail exitsignal"
> +
> +gdb_test "! kill -2 $$" ""
> +gdb_test "p \$_shell_exitcode" " = void" "shell interrupt exitcode"
> +gdb_test "p \$_shell_exitsignal" " = 2" "shell interrupt exitsignal"
> +
> +
> +gdb_test "pipe help pipe | wc -l" "10" "check simple pipe"
> +gdb_test "pipe help pipe | grep Usage: | wc -l" "4" "check double pipe"

That looks a bit brittle. We'll have to update the tests if we change the
help output. Wouldn't something a bit more stable be better? E.g.,
gdb has output/printf/echo commands. You could use those to precisely control
the output in the testcase. To avoid cluttering the test logs, you could
make the testcase define a command that uses echo, and then use that command:

define foo
echo "hello world\n"
...
end

> +
> +gdb_test "| help pipe | grep Usage: | wc -l" "4" "check double pipe, pipe char"
> +gdb_test "|help pipe|grep Usage:|wc -l" "4" "no space around pipe char"
> +
> +gdb_test "echo coucou\\n" "coucou" "echo coucou"
> +gdb_test "||wc -l" "1" "Check repeat previous command"

Lowercase "Check". Or better, remove "Check".

> +
> +gdb_test "| -d ! echo this contains a | character\\n ! sed -e 's/|/PIPE/'" \
> + "this contains a PIPE character" "verify alternate 1char sep"
> +
> +gdb_test "|-d ! echo this contains a | character\\n!sed -e 's/|/PIPE/'" \
> + "this contains a PIPE character" "verify alternate 1char sep, no space"
> +
> +gdb_test "| -d !!! echo this contains a | character\\n !!! sed -e 's/|/PIPE/'" \
> + "this contains a PIPE character" "verify alternate 3char sep"
> +
> +gdb_test "|-d !!! echo this contains a | character\\n!!!sed -e 's/|/PIPE/'" \
> + "this contains a PIPE character" "verify alternate 3char sep, no space"
> +
> +# Convenience variables with pipe command.
> +gdb_test "|p 123| exit 0" ""
> +gdb_test "p \$_shell_exitcode" " = 0" "pipe success exitcode"
> +gdb_test "p \$_shell_exitsignal" " = void" "pipe success exitsignal"
> +
> +gdb_test "|p 123| exit 1" ""
> +gdb_test "p \$_shell_exitcode" " = 1" "pipe fail exitcode"
> +gdb_test "p \$_shell_exitsignal" " = void" "pipe fail exitsignal"
> +
> +gdb_test "|p 123| kill -2 $$" ""
> +gdb_test "p \$_shell_exitcode" " = void" "pipe interrupt exitcode"
> +gdb_test "p \$_shell_exitsignal" " = 2" "pipe interrupt exitsignal"
> +
> +# Error handling verifications.
> +gdb_test "|" "Missing COMMAND" "all missing"
> +gdb_test "|-d" "Missing separator SEP after -d" "-d value missing"
> +gdb_test "|-d " "Missing separator SEP after -d" "-d spaces value missing"
> +gdb_test "| echo coucou" \
> + "Missing separator before SHELL_COMMAND" \
> + "| separator missing"
> +gdb_test "|-d SEP echo coucou" \
> + "Missing separator before SHELL_COMMAND" \
> + "SEP separator missing"
> +gdb_test "|echo coucou|" \
> + "Missing SHELL_COMMAND" \
> + "SHELL_COMMAND missing"
> +gdb_test "|-d ! echo coucou !" \
> + "Missing SHELL_COMMAND" \
> + "SHELL_COMMAND missing"
> +

At least the last two above have duplicate test names.

I think you're missing a test for "-dSEP", no space.

Thanks,
Pedro Alves