diff mbox

get_integer_valueof: Don't output value in test name

Message ID 1507661462-32717-1-git-send-email-simon.marchi@ericsson.com
State New, archived
Headers

Commit Message

Simon Marchi Oct. 10, 2017, 6:51 p.m. UTC 
  The get_integer_valueof outputs the value it has read as part of the
test name.  This causes test names to vary from run to run, and adds
some noise when diffing test results.  e.g.:

-PASS: gdb.threads/process-dies-while-detaching.exp: multi-process: continue: killed outside: get integer valueof "mypid" (28770)
+PASS: gdb.threads/process-dies-while-detaching.exp: multi-process: continue: killed outside: get integer valueof "mypid" (32238)

This patch removes that, since it's probably not very useful.

gdb/testsuite/ChangeLog:

	* lib/gdb.exp (get_integer_valueof): Don't output read value in test name.
---
 gdb/testsuite/lib/gdb.exp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Pedro Alves Oct. 10, 2017, 7:02 p.m. UTC | #1 
On 10/10/2017 07:51 PM, Simon Marchi wrote:
> The get_integer_valueof outputs the value it has read as part of the
> test name.  This causes test names to vary from run to run, and adds
> some noise when diffing test results.  e.g.:
> 
> -PASS: gdb.threads/process-dies-while-detaching.exp: multi-process: continue: killed outside: get integer valueof "mypid" (28770)
> +PASS: gdb.threads/process-dies-while-detaching.exp: multi-process: continue: killed outside: get integer valueof "mypid" (32238)

Ideally, test result diffing tools would be ignoring
" (foo)" at the end test names, because that part is
not considered a part of the test name itself.  It's
extra info.  That's where we write "(timeout)", "(eof)",
etc.  So for instance:

 -PASS: gdb.threads/process-dies-while-detaching.exp: multi-process: continue: killed outside: get integer valueof "mypid"
 +FAIL: gdb.threads/process-dies-while-detaching.exp: multi-process: continue: killed outside: get integer valueof "mypid" (timeout)

... this should be considered a regression, instead of a new FAIL and
a missing PASS.

I think the buildbot diffing script is correctly ignoring that
tail end "(foo)"s.

That said, I think it's still a good idea to make the gdb.sum output
more stable across passing runs, so that plain /usr/bin/diff also
works reasonable well.

> 
> This patch removes that, since it's probably not very useful.

Since we can find the number in gdb.log if we need it, this
is fine with me.

Thanks,
Pedro Alves
Simon Marchi Oct. 10, 2017, 7:09 p.m. UTC | #2 
On 2017-10-10 15:02, Pedro Alves wrote:
> On 10/10/2017 07:51 PM, Simon Marchi wrote:
>> The get_integer_valueof outputs the value it has read as part of the
>> test name.  This causes test names to vary from run to run, and adds
>> some noise when diffing test results.  e.g.:
>> 
>> -PASS: gdb.threads/process-dies-while-detaching.exp: multi-process: 
>> continue: killed outside: get integer valueof "mypid" (28770)
>> +PASS: gdb.threads/process-dies-while-detaching.exp: multi-process: 
>> continue: killed outside: get integer valueof "mypid" (32238)
> 
> Ideally, test result diffing tools would be ignoring
> " (foo)" at the end test names, because that part is
> not considered a part of the test name itself.  It's
> extra info.  That's where we write "(timeout)", "(eof)",
> etc.  So for instance:
> 
>  -PASS: gdb.threads/process-dies-while-detaching.exp: multi-process:
> continue: killed outside: get integer valueof "mypid"
>  +FAIL: gdb.threads/process-dies-while-detaching.exp: multi-process:
> continue: killed outside: get integer valueof "mypid" (timeout)
> 
> ... this should be considered a regression, instead of a new FAIL and
> a missing PASS.
> 
> I think the buildbot diffing script is correctly ignoring that
> tail end "(foo)"s.
> 
> That said, I think it's still a good idea to make the gdb.sum output
> more stable across passing runs, so that plain /usr/bin/diff also
> works reasonable well.

Right, that's the idea.

>> 
>> This patch removes that, since it's probably not very useful.
> 
> Since we can find the number in gdb.log if we need it, this
> is fine with me.

Thanks, I'm pushing it.

Simon
diff mbox

Patch

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 48fec2f..9d8296a 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -5634,7 +5634,7 @@  proc get_integer_valueof { exp default {test ""} } {
     gdb_test_multiple "print /d ${exp}" "$test" {
 	-re "\\$\[0-9\]* = (\[-\]*\[0-9\]*).*$gdb_prompt $" {
 	    set val $expect_out(1,string)
-	    pass "$test ($val)"
+	    pass "$test"
 	}
 	timeout {
 	    fail "$test (timeout)"