Re: [PATCH][gdb/testsuite] Rewrite catch-follow-exec.exp

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

On 2018-10-23 6:38 p.m., Tom de Vries wrote:
> On 10/23/18 11:05 PM, Tom de Vries wrote:
>> On 10/23/18 11:04 PM, Simon Marchi wrote:
>>> On 2018-10-15 3:54 p.m., Tom de Vries wrote:
>>>>> Just wondering.  Would it make life easier if we fixed PR 23368, which
>>>>> is the reason we have to do the test in an unnatural way?
>>>>
>>>> Yes.
>>>
>>> Hi Tom,
>>>
>>> PR 23368 should be fixed now.  Do you plan on updating catch-follow-exec.exp
>>> to be written in a more standard way?
>>
>> Sure, will do.
> 
> How does this look?

Hi Tom,

Thanks for looking into this so quickly.  I have some superficial suggestions that
can help shorten the test a bit and make it more readable (some of them can be personal
preference though...).

When the test name is omitted, it defaults to the command.  So instead of

    gdb_test "catch exec" \
	{Catchpoint [0-9][0-9]* \(exec\)} \
	"catch exec"

You can write

    gdb_test "catch exec" {Catchpoint [0-9][0-9]* \(exec\)}

and the test name will be "catch exec".  Instead of [0-9][0-9]*, I am
pretty sure you can use [0-9]+, or $decimal, which is provided by DejaGnu
(/usr/share/dejagnu/runtest.exp):

  101:    set decimal "\[0-9\]+"

Except in the {} string, $decimal won't work, because it won't get
substituted.

For this:

    gdb_test "set follow-exec-mode new" \
	"" \
	"set follow-exec-mode new"

You can use

    gdb_test_no_output "set follow-exec-mode new"

(again, omitting the test name makes it default to the command)

I'd suggest replacing

    gdb_test_multiple "info prog" "info prog" {
	-i "$gdb_spawn_id" eof {
	    fail "info prog"
	}
	-i "$gdb_spawn_id" "No selected thread\."  {
	    pass "info prog"
	}
    }

with the simpler

    gdb_test "info prog" "No selected thread."

If GDB crashes as it did before your fix, the test will be unresolved, which is
treated the same as a FAIL.  If you decide to keep the gdb_test_multiple, I
think you don't need to specify -i "$gdb_spawn_id", it's the default.  Also, it's
common practice to factor out the test name, to make sure it's constant.  And
because the test name is the same as the command, you could do

set test "info prog"
gdb_test_multiple $test $test {
  eof {
    fail $test
  }
  -re "No selected thread\." {
    pass $test
  }
}

While at it, could you update the comment at the top of the file, which currently
says:

# Check whether finish respects the print pretty user setting when printing the
# function result.

Thanks!

Simon