[RFA] Suppress stap-trace.exp when compilation fails

[RFA] Suppress stap-trace.exp when compilation fails

[Keith Seitz]

[-- Attachment #1: Type: text/plain, Size: 604 bytes --]

Hi,

stap-trace.exp does not check the return value of its compile-a-testcase 
wrapper procedure, so when compilation of the test case fails, it 
attempts to run the tests anyway instead of nicely bailing out.

This patch addresses this by fixing the compile wrapper to always return 
a valid value and checking this procedure's return value before running 
the tests.

Okay?
Keith

testsuite/ChangeLog

2012-05-23  Keith Seitz  <keiths@redhat.com>

	* gdb.trace/stap-trace.exp: If compile_stap_bin fails,
	return -1 to suppress the rest of the tests.
	(compile_stap_bin): Return boolean success value.


[-- Attachment #2: suppress-stap-trace.patch --]
[-- Type: text/x-patch, Size: 835 bytes --]

diff --git a/gdb/testsuite/gdb.trace/stap-trace.exp b/gdb/testsuite/gdb.trace/stap-trace.exp
index 562eec4..088d14c 100644
--- a/gdb/testsuite/gdb.trace/stap-trace.exp
+++ b/gdb/testsuite/gdb.trace/stap-trace.exp
@@ -46,8 +46,10 @@ proc compile_stap_bin {exec_name {arg ""}} {
 	    "$binfile_dir/$exec_name" \
 	    executable [concat $arg debug nowarnings]] != "" } {
 	untested "Could not compile ${srcfile}"
-	return -1
+	return 0
     }
+
+    return 1
 }
 
 proc prepare_for_trace_test {} {
@@ -105,7 +107,10 @@ proc gdb_collect_probe_arg { msg probe val_arg0 } {
 	    "collect $msg: collected probe arg0"
 }
 
-compile_stap_bin "stap-probe-nosem"
+if {![compile_stap_bin "stap-probe-nosem"]} {
+    # An appropriate failure message has already been output
+    return -1
+}
 
 clean_restart $executable
 if { ![runto_main] } {

Re: [RFA] Suppress stap-trace.exp when compilation fails

[Sergio Durigan Junior]

On Wednesday, May 23 2012, Keith Seitz wrote:

> This patch addresses this by fixing the compile wrapper to always
> return a valid value and checking this procedure's return value before
> running the tests.
>
> Okay?

Thanks for catching this.

It's OK for me, but IANAM.

> Keith
>
> testsuite/ChangeLog
>
> 2012-05-23  Keith Seitz  <keiths@redhat.com>
>
> 	* gdb.trace/stap-trace.exp: If compile_stap_bin fails,
> 	return -1 to suppress the rest of the tests.
> 	(compile_stap_bin): Return boolean success value.
> diff --git a/gdb/testsuite/gdb.trace/stap-trace.exp b/gdb/testsuite/gdb.trace/stap-trace.exp
> index 562eec4..088d14c 100644
> --- a/gdb/testsuite/gdb.trace/stap-trace.exp
> +++ b/gdb/testsuite/gdb.trace/stap-trace.exp
> @@ -46,8 +46,10 @@ proc compile_stap_bin {exec_name {arg ""}} {
>  	    "$binfile_dir/$exec_name" \
>  	    executable [concat $arg debug nowarnings]] != "" } {
>  	untested "Could not compile ${srcfile}"
> -	return -1
> +	return 0
>      }
> +
> +    return 1
>  }
>  
>  proc prepare_for_trace_test {} {
> @@ -105,7 +107,10 @@ proc gdb_collect_probe_arg { msg probe val_arg0 } {
>  	    "collect $msg: collected probe arg0"
>  }
>  
> -compile_stap_bin "stap-probe-nosem"
> +if {![compile_stap_bin "stap-probe-nosem"]} {
> +    # An appropriate failure message has already been output
> +    return -1
> +}
>  
>  clean_restart $executable
>  if { ![runto_main] } {

-- 
Sergio

Re: [RFA] Suppress stap-trace.exp when compilation fails

[Pedro Alves]

On 05/23/2012 06:15 PM, Keith Seitz wrote:

> Hi,
> 
> stap-trace.exp does not check the return value of its compile-a-testcase wrapper procedure, so when compilation of the test case fails, it attempts to run the tests anyway instead of nicely bailing out.
> 
> This patch addresses this by fixing the compile wrapper to always return a valid value and checking this procedure's return value before running the tests.
> 
> Okay?


Sure, thanks.  There's a second compilation further below, but I'll assume that if
the first succeeds, the second should also.

-- 
Pedro Alves

Re: [RFA] Suppress stap-trace.exp when compilation fails

[Keith Seitz]

On 05/23/2012 10:37 AM, Pedro Alves wrote:
> Sure, thanks.  There's a second compilation further below, but I'll assume that if
> the first succeeds, the second should also.

I thought the same thing, too, but perhaps there is no reason /not/ to 
protect that, too.

I've committed the patch with that safeguard added. [I'm not reposting 
that, since it is trivial.]

Thank you for taking a look.

Keith