[v2,4/8] Fix test names starting with uppercase using gdb_test_multiple
Commit Message
This fixes offender testcases that have test names starting with uppercase
when using gdb_test_multiple in a single-line construct.
gdb/testsuite/ChangeLog
2016-11-25 Luis Machado <lgustavo@codesourcery.com>
Fix test names starting with uppercase throughout the files.
* gdb/testsuite/gdb.arch/i386-bp_permanent.exp
* gdb/testsuite/gdb.arch/i386-gnu-cfi.exp
* gdb/testsuite/gdb.base/disasm-end-cu.exp
* gdb/testsuite/gdb.base/macscp.exp
* gdb/testsuite/gdb.base/pending.exp
* gdb/testsuite/gdb.base/watch_thread_num.exp
* gdb/testsuite/gdb.cp/exception.exp
* gdb/testsuite/gdb.cp/gdb2495.exp
* gdb/testsuite/gdb.cp/local.exp
* gdb/testsuite/gdb.python/py-evsignal.exp
* gdb/testsuite/gdb.python/python.exp
* gdb/testsuite/gdb.trace/tracecmd.exp
---
gdb/testsuite/gdb.arch/i386-bp_permanent.exp | 2 +-
gdb/testsuite/gdb.arch/i386-gnu-cfi.exp | 2 +-
gdb/testsuite/gdb.base/disasm-end-cu.exp | 2 +-
gdb/testsuite/gdb.base/macscp.exp | 2 +-
gdb/testsuite/gdb.base/pending.exp | 4 ++--
gdb/testsuite/gdb.base/watch_thread_num.exp | 2 +-
gdb/testsuite/gdb.cp/exception.exp | 2 +-
gdb/testsuite/gdb.cp/gdb2495.exp | 4 ++--
gdb/testsuite/gdb.cp/local.exp | 2 +-
gdb/testsuite/gdb.python/py-evsignal.exp | 2 +-
gdb/testsuite/gdb.python/python.exp | 2 +-
gdb/testsuite/gdb.trace/tracecmd.exp | 2 +-
12 files changed, 14 insertions(+), 14 deletions(-)
Comments
On Fri, Nov 25, 2016 at 02:54:00PM -0600, Luis Machado wrote:
> This fixes offender testcases that have test names starting with uppercase
> when using gdb_test_multiple in a single-line construct.
>
> gdb/testsuite/ChangeLog
> 2016-11-25 Luis Machado <lgustavo@codesourcery.com>
>
> Fix test names starting with uppercase throughout the files.
>
> * gdb/testsuite/gdb.arch/i386-bp_permanent.exp
> * gdb/testsuite/gdb.arch/i386-gnu-cfi.exp
> * gdb/testsuite/gdb.base/disasm-end-cu.exp
> * gdb/testsuite/gdb.base/macscp.exp
> * gdb/testsuite/gdb.base/pending.exp
> * gdb/testsuite/gdb.base/watch_thread_num.exp
> * gdb/testsuite/gdb.cp/exception.exp
> * gdb/testsuite/gdb.cp/gdb2495.exp
> * gdb/testsuite/gdb.cp/local.exp
> * gdb/testsuite/gdb.python/py-evsignal.exp
> * gdb/testsuite/gdb.python/python.exp
> * gdb/testsuite/gdb.trace/tracecmd.exp
Drop "gdb/testsuite/". Patch is good to me, but I think we need to
enforce a rule in "proc gdb_test_multiple" that test message should
start with a lower case letter, emit an ERROR if it starts with
uppercase.
On 11/27/2016 11:09 AM, Yao Qi wrote:
> On Fri, Nov 25, 2016 at 02:54:00PM -0600, Luis Machado wrote:
>> This fixes offender testcases that have test names starting with uppercase
>> when using gdb_test_multiple in a single-line construct.
>>
>> gdb/testsuite/ChangeLog
>> 2016-11-25 Luis Machado <lgustavo@codesourcery.com>
>>
>> Fix test names starting with uppercase throughout the files.
>>
>> * gdb/testsuite/gdb.arch/i386-bp_permanent.exp
>> * gdb/testsuite/gdb.arch/i386-gnu-cfi.exp
>> * gdb/testsuite/gdb.base/disasm-end-cu.exp
>> * gdb/testsuite/gdb.base/macscp.exp
>> * gdb/testsuite/gdb.base/pending.exp
>> * gdb/testsuite/gdb.base/watch_thread_num.exp
>> * gdb/testsuite/gdb.cp/exception.exp
>> * gdb/testsuite/gdb.cp/gdb2495.exp
>> * gdb/testsuite/gdb.cp/local.exp
>> * gdb/testsuite/gdb.python/py-evsignal.exp
>> * gdb/testsuite/gdb.python/python.exp
>> * gdb/testsuite/gdb.trace/tracecmd.exp
>
> Drop "gdb/testsuite/". Patch is good to me, but I think we need to
> enforce a rule in "proc gdb_test_multiple" that test message should
> start with a lower case letter, emit an ERROR if it starts with
> uppercase.
>
Do you mean just for gdb_test_multiple?
It can be tricky, since one is still free to start the sentences with
something like "ARM ..." or some other technology name. So unfortunately
we can't be too strict. I wish we could.
On Mon, Nov 28, 2016 at 10:06:28AM -0600, Luis Machado wrote:
> On 11/27/2016 11:09 AM, Yao Qi wrote:
> >On Fri, Nov 25, 2016 at 02:54:00PM -0600, Luis Machado wrote:
> >>This fixes offender testcases that have test names starting with uppercase
> >>when using gdb_test_multiple in a single-line construct.
> >>
> >>gdb/testsuite/ChangeLog
> >>2016-11-25 Luis Machado <lgustavo@codesourcery.com>
> >>
> >> Fix test names starting with uppercase throughout the files.
> >>
> >> * gdb/testsuite/gdb.arch/i386-bp_permanent.exp
> >> * gdb/testsuite/gdb.arch/i386-gnu-cfi.exp
> >> * gdb/testsuite/gdb.base/disasm-end-cu.exp
> >> * gdb/testsuite/gdb.base/macscp.exp
> >> * gdb/testsuite/gdb.base/pending.exp
> >> * gdb/testsuite/gdb.base/watch_thread_num.exp
> >> * gdb/testsuite/gdb.cp/exception.exp
> >> * gdb/testsuite/gdb.cp/gdb2495.exp
> >> * gdb/testsuite/gdb.cp/local.exp
> >> * gdb/testsuite/gdb.python/py-evsignal.exp
> >> * gdb/testsuite/gdb.python/python.exp
> >> * gdb/testsuite/gdb.trace/tracecmd.exp
> >
> >Drop "gdb/testsuite/". Patch is good to me, but I think we need to
> >enforce a rule in "proc gdb_test_multiple" that test message should
> >start with a lower case letter, emit an ERROR if it starts with
> >uppercase.
> >
>
> Do you mean just for gdb_test_multiple?
Yes,
>
> It can be tricky, since one is still free to start the sentences
> with something like "ARM ..." or some other technology name. So
> unfortunately we can't be too strict. I wish we could.
Do we have some many technology names? We have a white list of these
technology names which can be capitalized in test message.
On 11/29/2016 08:48 AM, Yao Qi wrote:
> On Mon, Nov 28, 2016 at 10:06:28AM -0600, Luis Machado wrote:
>> On 11/27/2016 11:09 AM, Yao Qi wrote:
>>> On Fri, Nov 25, 2016 at 02:54:00PM -0600, Luis Machado wrote:
>>>> This fixes offender testcases that have test names starting with uppercase
>>>> when using gdb_test_multiple in a single-line construct.
>>>>
>>>> gdb/testsuite/ChangeLog
>>>> 2016-11-25 Luis Machado <lgustavo@codesourcery.com>
>>>>
>>>> Fix test names starting with uppercase throughout the files.
>>>>
>>>> * gdb/testsuite/gdb.arch/i386-bp_permanent.exp
>>>> * gdb/testsuite/gdb.arch/i386-gnu-cfi.exp
>>>> * gdb/testsuite/gdb.base/disasm-end-cu.exp
>>>> * gdb/testsuite/gdb.base/macscp.exp
>>>> * gdb/testsuite/gdb.base/pending.exp
>>>> * gdb/testsuite/gdb.base/watch_thread_num.exp
>>>> * gdb/testsuite/gdb.cp/exception.exp
>>>> * gdb/testsuite/gdb.cp/gdb2495.exp
>>>> * gdb/testsuite/gdb.cp/local.exp
>>>> * gdb/testsuite/gdb.python/py-evsignal.exp
>>>> * gdb/testsuite/gdb.python/python.exp
>>>> * gdb/testsuite/gdb.trace/tracecmd.exp
>>>
>>> Drop "gdb/testsuite/". Patch is good to me, but I think we need to
>>> enforce a rule in "proc gdb_test_multiple" that test message should
>>> start with a lower case letter, emit an ERROR if it starts with
>>> uppercase.
>>>
>>
>> Do you mean just for gdb_test_multiple?
>
> Yes,
>
Why just that one? Shouldn't we attempt to enforce this for all the
other proc's, so gdb_test, gdb_test_multiple, gdb_test_no_output and
mi_gdb_test?
>>
>> It can be tricky, since one is still free to start the sentences
>> with something like "ARM ..." or some other technology name. So
>> unfortunately we can't be too strict. I wish we could.
>
> Do we have some many technology names? We have a white list of these
> technology names which can be capitalized in test message.
>
It is certainly possible, but do we want to add one more layer of
maintenance? We could enforce a rule from now on to require test names
to always start with lowercase or to even be all lowercase.
I don't think we're coherent with our use of lower/uppercase anyway. For
example, GDBserver gets called gdbserver in gdbserver's own help text
and the testsuite is (less so now with this series) a mixed bag.
I'm open to ideas.
On Tue, Nov 29, 2016 at 09:06:54AM -0600, Luis Machado wrote:
>
> Why just that one? Shouldn't we attempt to enforce this for all the other
> proc's, so gdb_test, gdb_test_multiple, gdb_test_no_output and mi_gdb_test?
>
because all of them except mi_gdb_test call gdb_test_multiple.
> > >
> > > It can be tricky, since one is still free to start the sentences
> > > with something like "ARM ..." or some other technology name. So
> > > unfortunately we can't be too strict. I wish we could.
> >
> > Do we have some many technology names? We have a white list of these
> > technology names which can be capitalized in test message.
> >
>
> It is certainly possible, but do we want to add one more layer of
> maintenance? We could enforce a rule from now on to require test names to
> always start with lowercase or to even be all lowercase.
>
This is done by manual inspection in the patch review each time. The
automatic checking is much better than manual inspection.
> I don't think we're coherent with our use of lower/uppercase anyway. For
> example, GDBserver gets called gdbserver in gdbserver's own help text and
> the testsuite is (less so now with this series) a mixed bag.
>
> I'm open to ideas.
We've spent some effort to convert GDB tests to comply to this rule, so
I am wondering if we can find some efficient way to enforce this rule,
otherwise, the violation to this rules may show up in the code some time
later.
On 11/29/2016 02:55 PM, Yao Qi wrote:
> On Tue, Nov 29, 2016 at 09:06:54AM -0600, Luis Machado wrote:
>>
>> Why just that one? Shouldn't we attempt to enforce this for all the other
>> proc's, so gdb_test, gdb_test_multiple, gdb_test_no_output and mi_gdb_test?
>>
>
> because all of them except mi_gdb_test call gdb_test_multiple.
>
Ah, that makes sense. I thought i was missing something.
>>>>
>>>> It can be tricky, since one is still free to start the sentences
>>>> with something like "ARM ..." or some other technology name. So
>>>> unfortunately we can't be too strict. I wish we could.
>>>
>>> Do we have some many technology names? We have a white list of these
>>> technology names which can be capitalized in test message.
>>>
>>
>> It is certainly possible, but do we want to add one more layer of
>> maintenance? We could enforce a rule from now on to require test names to
>> always start with lowercase or to even be all lowercase.
>>
>
> This is done by manual inspection in the patch review each time. The
> automatic checking is much better than manual inspection.
>
Right. I agree that we don't want to have to keep inspecting things
manually. So if we decide to go all lowercase even for technology, ISA
or arch names, then i think we can enforce this better via the change to
gdb_test_multiple (and mi_gdb_test). Then we don't have to maintain a
whitelist at all.
Does that sound reasonable?
>> I don't think we're coherent with our use of lower/uppercase anyway. For
>> example, GDBserver gets called gdbserver in gdbserver's own help text and
>> the testsuite is (less so now with this series) a mixed bag.
>>
>> I'm open to ideas.
>
> We've spent some effort to convert GDB tests to comply to this rule, so
> I am wondering if we can find some efficient way to enforce this rule,
> otherwise, the violation to this rules may show up in the code some time
> later.
>
I'd like that too.
On 11/29/2016 09:14 PM, Luis Machado wrote:
> Right. I agree that we don't want to have to keep inspecting things
> manually. So if we decide to go all lowercase even for technology, ISA
> or arch names, then i think we can enforce this better via the change to
> gdb_test_multiple (and mi_gdb_test). Then we don't have to maintain a
> whitelist at all.
>
> Does that sound reasonable?
Is that going a bit too far? There's no real harm in
uppercase test messages, other than inconsistency. I suspect that the
main reason we see patches with uppercase test names is people
starting from an existing test and applying the old time-proven
copy/paste test-writing methodology. So if we fix most violations, then
we should see much fewer attempts to reintroduce more. And we can't add
such enforcement until we've fixed all existing ones.
Thanks,
Pedro Alves
@@ -51,7 +51,7 @@ if ![runto_main] then {
set function "standard"
-set retcode [gdb_test_multiple "disassemble $function" "Disassemble function '$function'" {
+set retcode [gdb_test_multiple "disassemble $function" "disassemble function '$function'" {
-re "($hex) <\\+0>.*($hex) <\\+$decimal>.*int3.*($hex) <\\+$decimal>.*leave.*$gdb_prompt $" {
set address_bp $expect_out(2,string)
set address_after_bp $expect_out(3,string)
@@ -71,7 +71,7 @@ gdb_test "up 3" \
"gate \\(\[^()\]*\\) at .*gate.c.*" \
"shift up to the modified frame"
-gdb_test_multiple "info frame" "Existence of the CFI inserted register" {
+gdb_test_multiple "info frame" "existence of the CFI inserted register" {
-re "Stack level 3, frame at (0x\[0-9a-f\]+):.*Saved registers:.* ecx at (0x\[0-9a-f\]+),.*" {
pass "existence of the CFI inserted register"
if { [string compare $expect_out(1,string) $expect_out(2,string)] } then {
@@ -33,7 +33,7 @@ if {$main_addr == 0 || $dummy_3_addr == 0 || $dummy_3_addr <= $main_addr} {
return -1
}
-gdb_test_multiple "disassemble /m ${main_addr},${dummy_3_addr}" "Disassemble address range with source" {
+gdb_test_multiple "disassemble /m ${main_addr},${dummy_3_addr}" "disassemble address range with source" {
-re "Dump of assembler code from ${main_addr} to ${dummy_3_addr}:\r\nEnd of assembler dump\." {
fail "no output from the disassemble command"
}
@@ -195,7 +195,7 @@ proc list_and_check_macro {func macro expected} {
gdb_test "list main" ".*main.*" "list main for support check"
set macro_support "unknown"
-gdb_test_multiple "info source" "Test macro information" {
+gdb_test_multiple "info source" "test macro information" {
-re "Includes preprocessor macro info\..*$gdb_prompt $" {
set macro_support 1
verbose "Source has macro information"
@@ -167,7 +167,7 @@ gdb_test "info break" \
#
set bp2_loc [gdb_get_line_number "y = x + 4" ${libfile}.c]
-gdb_test_multiple "break pendshr.c:$bp2_loc if x > 3" "Set pending breakpoint 2" {
+gdb_test_multiple "break pendshr.c:$bp2_loc if x > 3" "set pending breakpoint 2" {
-re ".*Make breakpoint pending.*y or \\\[n\\\]. $" {
gdb_test "y" "Breakpoint.*pendshr.c:$bp2_loc.*pending." \
"Set pending breakpoint 2"
@@ -189,7 +189,7 @@ gdb_test "info break" \
#
set bp3_loc [gdb_get_line_number "printf" ${libfile}.c]
-gdb_test_multiple "break pendshr.c:$bp3_loc" "Set pending breakpoint 3" {
+gdb_test_multiple "break pendshr.c:$bp3_loc" "set pending breakpoint 3" {
-re ".*Make breakpoint pending.*y or \\\[n\\\]. $" {
gdb_test "y" "Breakpoint.*pendshr.c:$bp3_loc.*pending." \
"Set pending breakpoint 3"
@@ -58,7 +58,7 @@ gdb_test "break loop" "Breakpoint \[0-9\].*" \
gdb_test "continue" ".*Breakpoint .*loop.*" "stopped in loop"
-gdb_test_multiple "thread" "Thread command" {
+gdb_test_multiple "thread" "thread command" {
-re ".*Current thread is (\[0-9\]*).*$gdb_prompt $" {
pass "thread command"
}
@@ -89,7 +89,7 @@ gdb_test "tbreak main" "Temporary breakpoint 4.*" \
set ok 0
gdb_run_cmd
-gdb_test_multiple "" "Run to main" {
+gdb_test_multiple "" "run to main" {
-re "Temporary breakpoint 4,.*$gdb_prompt $" {
pass "run to main"
set ok 1
@@ -121,7 +121,7 @@ if ![runto_main] then {
# behaviour; it should not. Test both on and off states.
# Turn on unwind on signal behaviour.
-gdb_test_multiple "set unwindonsignal on" "Turn unwindonsignal on" {
+gdb_test_multiple "set unwindonsignal on" "turn unwindonsignal on" {
-re "$gdb_prompt $" {pass "set unwindonsignal on"}
timeout {fail "(timeout) set unwindonsignal on"}
}
@@ -137,7 +137,7 @@ gdb_test "p exceptions.raise_signal(1)" \
"To change this behavior use \"set unwindonsignal off\".*"
# And reverse - turn off again.
-gdb_test_multiple "set unwindonsignal off" "Turn unwindonsignal off" {
+gdb_test_multiple "set unwindonsignal off" "turn unwindonsignal off" {
-re "$gdb_prompt $" {pass "set unwindonsignal off"}
timeout {fail "(timeout) set unwindonsignal off"}
}
@@ -164,7 +164,7 @@ gdb_test "up" ".*main.*" "up from marker2"
# setup_kfail "gdb/825"
set eol "\[\t \]*\[\r\n\]+\[\t \]*"
-gdb_test_multiple "ptype Local" "Local out of scope" {
+gdb_test_multiple "ptype Local" "local out of scope" {
-re "No symbol \"Local\" in current context.*${gdb_prompt} $" {
pass "local out of scope"
}
@@ -37,7 +37,7 @@ gdb_test "test-events" "Event testers registered."
gdb_test_no_output "set non-stop on"
gdb_run_cmd
-gdb_test_multiple "" "Signal Thread 3" {
+gdb_test_multiple "" "signal Thread 3" {
-re "event type: stop\r\nstop reason: signal\r\nstop signal: SIGUSR1\r\nthread num: 3\r\nevent type: stop\r\n.*$gdb_prompt $" {
pass "thread 3 was signaled"
}
@@ -322,7 +322,7 @@ gdb_test_multiple "set prompt blah " "set blah in GDB" {
}
}
-gdb_test_multiple "python gdb.prompt_hook = None" "Delete hook" {
+gdb_test_multiple "python gdb.prompt_hook = None" "delete hook" {
-re "\[\r\n\]$newprompt2 $" {
pass "delete old hook"
}
@@ -161,7 +161,7 @@ gdb_test "help trace" "Set a tracepoint at .*" "1.14: help trace"
gdb_delete_tracepoints
# Acceptance vs rejection of a location are target-specific, so allow both.
-gdb_test_multiple "ftrace gdb_recursion_test" "Set a fast tracepoint" {
+gdb_test_multiple "ftrace gdb_recursion_test" "set a fast tracepoint" {
-re "Fast tracepoint $decimal at $hex: file.*$srcfile, line $testline1.*$gdb_prompt $" {
pass "set a fast tracepoint"
}