diff mbox

[RFA,2/2] Make some test names invariant

Message ID 1473893962-12420-2-git-send-email-tom@tromey.com
State New, archived
Headers

Commit Message

Tom Tromey Sept. 14, 2016, 10:59 p.m. UTC 
  While working on the previous patch, I noticed that the test names in
process-dies-while-detaching include the PID of some test process,
making it so that the test names change between runs.  This patch
fixes the problem.

2016-09-14  Tom Tromey  <tom@tromey.com>

	* gdb.threads/process-dies-while-detaching.exp
	(test_detach_killed_outside): Pass test name to
	get_integer_valueof.
	* lib/gdb.exp (get_integer_valueof): Add "test" argument.
---
 gdb/testsuite/ChangeLog                                    | 7 +++++++
 gdb/testsuite/gdb.threads/process-dies-while-detaching.exp | 2 +-
 gdb/testsuite/lib/gdb.exp                                  | 6 ++++--
 3 files changed, 12 insertions(+), 3 deletions(-)

Comments

Pedro Alves Sept. 15, 2016, 3:16 p.m. UTC | #1 
On 09/14/2016 11:59 PM, Tom Tromey wrote:
> While working on the previous patch, I noticed that the test names in
> process-dies-while-detaching include the PID of some test process,
> making it so that the test names change between runs.  This patch
> fixes the problem.

LGTM.

However, FYI, with the current "definition" of test names, it 
doesn't actually change, because tools should be ignoring the
trailing " (...)" bit.  See:

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

We've always had to consider this, given "$test (timeout)", etc.

Note that the wiki doesn't say it, and I don't think we've ever
discussed it, but but the space before the open parens should matter
when deciding whether to ignore the trailing bit.  If there's no space,
it shouldn't be ignored.  This in order to make it possible to write
tests that call functions without coming up with odd contortions,
like, e.g., "print foo(1)".

Thanks,
Pedro Alves
Tom Tromey Sept. 15, 2016, 4:46 p.m. UTC | #2 
Pedro> However, FYI, with the current "definition" of test names, it 
Pedro> doesn't actually change, because tools should be ignoring the
Pedro> trailing " (...)" bit.

Aha, I didn't realize this, and the old test-comparison script I have
been using doesn't do this.

I think I'll just drop this patch and fix my script.

Tom
diff mbox

Patch

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index e34ae67..b397364 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,10 @@ 
+2016-09-14  Tom Tromey  <tom@tromey.com>
+
+	* gdb.threads/process-dies-while-detaching.exp
+	(test_detach_killed_outside): Pass test name to
+	get_integer_valueof.
+	* lib/gdb.exp (get_integer_valueof): Add "test" argument.
+
 2016-09-13  Tom Tromey  <tom@tromey.com>
 
 	PR gdb/20604:
diff --git a/gdb/testsuite/gdb.threads/process-dies-while-detaching.exp b/gdb/testsuite/gdb.threads/process-dies-while-detaching.exp
index 52dc8dd..734effc 100644
--- a/gdb/testsuite/gdb.threads/process-dies-while-detaching.exp
+++ b/gdb/testsuite/gdb.threads/process-dies-while-detaching.exp
@@ -268,7 +268,7 @@  proc test_detach_killed_outside {multi_process cmd} {
 	# Run to _exit in the child.
 	continue_to_exit_bp
 
-	set childpid [get_integer_valueof "mypid" -1]
+	set childpid [get_integer_valueof "mypid" -1 "get value of mypid"]
 	if { $childpid == -1 } {
 	    untested "failed to extract child pid"
 	    return -1
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index e538812..9abe3cd 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -5524,10 +5524,12 @@  proc get_valueof { fmt exp default } {
     return ${val}
 }
 
-proc get_integer_valueof { exp default } {
+proc get_integer_valueof { exp default {test ""} } {
     global gdb_prompt
 
-    set test "get integer valueof \"${exp}\""
+    if {$test == ""} {
+	set test "get integer valueof \"${exp}\""
+    }
     set val ${default}
     gdb_test_multiple "print /d ${exp}" "$test" {
 	-re "\\$\[0-9\]* = (\[-\]*\[0-9\]*).*$gdb_prompt $" {