Re: [PATCH v4 1/2] gdb: testsuite: print explicit test result in can_spawn_for_attach()

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2022-02-22 07:42, Tiezhu Yang wrote:
> In the current code, there is no test result when execute the following
> commands:
> 
>   $ make check-gdb TESTS="gdb.base/attach-pie-noexec.exp" RUNTESTFLAGS="--target_board=remote-gdbserver-on-localhost"
>   $ make check-gdb TESTS="gdb.base/attach-pie-noexec.exp" RUNTESTFLAGS="--target_board=native-gdbserver"
> 
> It is better to print explicit test result in can_spawn_for_attach().
> 
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>

It could be that some test uses this procedure not to skip some tests,
but to choose between two alternatives.  In that case, we wouldn't want
to show an "UNTESTED".  On the other hand, if you can confirm that all
current uses of can_spawn_for_attach are for simply skipping tests, then
it's probably fine.

To make it non-ambiguous, we could rename can_spawn_for_attach to
skip_attach_tests, very similar to the skip_python_tests procedure.
Then it would be clear what it is for (skipping attach tests if attach
is not supported by the target).  And skip_python_tests does show an
"UNSUPPORTED" if Python support is disabled, so it would makes sense to
do the same for attach.

I think that "unsupported" is more accurate than "untested" in this
case:

https://www.gnu.org/software/dejagnu/manual/untested-procedure.html
https://www.gnu.org/software/dejagnu/manual/unsupported-procedure.html

> ---
>  gdb/testsuite/lib/gdb.exp | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index a3717a4..d15ab90 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -5063,6 +5063,7 @@ proc can_spawn_for_attach { } {
>      # back the pid of the program.  On remote boards, that would give
>      # us instead the PID of e.g., the ssh client, etc.
>      if [is_remote target] then {
> +	untested "skipping tests due to is_remote target"
>  	return 0
>      }
>  
> @@ -5070,6 +5071,7 @@ proc can_spawn_for_attach { } {
>      # stub-like, where GDB finds the program already started on
>      # initial connection.
>      if {[target_info exists use_gdb_stub]} {
> +	untested "skipping tests due to use_gdb_stub"
>  	return 0
>      }

So I would suggest using:

  unsupported "skip attach tests (target is remote)"
  unsupported "skip attach tests (target is stub)"

That makes "skipping attach tests" the name of the test (identical in
both cases), and the part in parenthesis is additional details.  See:

  https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Do_not_use_.22tail_parentheses.22_on_test_messages

Simon