Re: [RFA PATCH 3/3] Testcase for pascal/17815

[Pedro Alves <palves@redhat.com>]

Thanks for adding the test case.

On 01/09/2015 04:18 PM, Pierre Muller wrote:
>   This adds the testcase for pascal/17815 bug report.
> 
> 
> gdb/testsuite/Changelog entry:
> 
> 2015-01-09  Pierre Muller  <muller@sourceware.org>
> 
> 	gdb.pascal/gdb17815.pas: New file.
> 	gdb.pascal/gdb17815.exp: New file.

Missing '*'.

Can you please pick a more descriptive name for the file?
One that suggests what's being tested, rather than a bug
number is ideal.  E.g., gdb.pascal/case-insensitive.{exp|pas}.

> +load_lib "pascal.exp"
> +
> +standard_testfile .pas
> +
> +if {[gdb_compile_pascal "${srcdir}/${subdir}/${srcfile}" "${binfile}"
> executable [list debug class]] != "" } {
> +    untested $testfile.exp
> +    return -1
> +}
> +
> +clean_restart ${testfile}
> +set bp_location [gdb_get_line_number "set breakpoint here"]
> +
> +if { [gdb_breakpoint ${srcfile}:${bp_location}] } {
> +    pass "setting breakpoint"
> +}
> +
> +# Verify that "start" lands inside the right procedure.
> +if { [gdb_start_cmd] < 0 } {
> +    untested start
> +    return -1
> +}
> +
> +gdb_test "" ".* at .*${srcfile}.*" "start"
> +gdb_test "continue" ""

I won't object to this, given that it seems like all
pascal tests do it, but I'll note that it's best to avoid
gdb_start_cmd unless we're really testing the "start" command,
as otherwise the test won't run against gdbserver
(try --target_board=native-gdbserver).  Doesn't runto_main
work?

> +# We are now inside CHECK method

Could you add a period at end of sentences?  Here and throughout.

> +gdb_test "p X" " = 67"
> +gdb_test "p B.X" " = -1"
> +# As A is global, we can also check it global value
                                       ^^
Should be "its".

Or, "check its value"?  What's a _global_ value?

> +gdb_test "p A.X" " = 67"
> +gdb_test "p Y" " = 33"
> +gdb_test "p Y" " = 33"

Duplicate test here.  Please try the recipe at:

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

Thanks,
Pedro Alves