[RFA/testsuite/ada] Do not abort prematurely the test upon build failure

[RFA/testsuite/ada] Do not abort prematurely the test upon build failure

[Joel Brobecker]

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

I was triple checking that I fail gracefully if we don't find any
gnatmake program when running our new ada tescase, and that part seems
fine. But, on the contrary, I think we're being perhaps a little too
silent.

Right now, if you don't run the null_record.exp testcase with -v,
the entire test gets skipped without any message (ie no compilation
error message, no pass, no fail).

With -v, we get:
<<
Ada compilation failed: default_target_compile: Can't find gnatmake.
>>

That's because in null_record.exp, I have put:

        if {[gdb_compile_ada <bla bla bla>] != "" } {
 !! ->    return -1
        }

So indeed, when gdb_compile_ada fails, I simply abort the test...
A bit too extreme, maybe? :-).

Anyway, I looked at various tests in the gdb.base and gdb.cp
subdirectory, and found that the common thing to do something
like this:

        gdb_suppress_entire_file "Testcase compile failed, [...]"

Here is the output (not very pretty, but oh well) I get after using
this procedure, instead of returning prematurely:

<<
WARNING: Testcase compile failed, so all tests in this file will automatically fail.

ERROR: (timeout) GDB never initialized after 10 seconds.
WARNING: remote_expect statement without a default case?!
ERROR: couldn't load /home/brobecke/act/gdb-public/gdb/testsuite/gdb.ada/null_record into /home/brobecke/act/gdb-public/gdb/testsuite/../../gdb/gdb (end of file).
FAIL: gdb.ada/null_record.exp: ptype on null record
>>

Note that the logs still do not contain the reason for the failure.
The reason is only printed if I run the testcase with -v.

2004-03-31  Joel Brobecker  <brobecker@gnat.com>

        * gdb.ada/null_record.exp: Do not abort the test prematurely
        if the build of the example program failed. Report a warning
        and automatically fail the tests in that file.

Tested on x86-linux.

OK to apply?

Thanks,
-- 
Joel

[-- Attachment #2: null_record.exp.diff --]
[-- Type: text/plain, Size: 587 bytes --]

Index: null_record.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.ada/null_record.exp,v
retrieving revision 1.1
diff -u -r1.1 null_record.exp
--- null_record.exp	1 Apr 2004 00:57:59 -0000	1.1
+++ null_record.exp	1 Apr 2004 02:28:30 -0000
@@ -9,7 +9,7 @@
 set binfile ${objdir}/${subdir}/${testfile}
 
 if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug ]] != "" } {
-  return -1
+    gdb_suppress_entire_file "Testcase compile failed, so all tests in this file will automatically fail."
 }
 
 gdb_exit

Re: [RFA/testsuite/ada] Do not abort prematurely the test upon build failure

[Daniel Jacobowitz]

On Wed, Mar 31, 2004 at 06:30:07PM -0800, Joel Brobecker wrote:
> So indeed, when gdb_compile_ada fails, I simply abort the test...
> A bit too extreme, maybe? :-).
> 
> Anyway, I looked at various tests in the gdb.base and gdb.cp
> subdirectory, and found that the common thing to do something
> like this:
> 
>         gdb_suppress_entire_file "Testcase compile failed, [...]"
> 
> Here is the output (not very pretty, but oh well) I get after using
> this procedure, instead of returning prematurely:

Unfortunately you didn't pick the best example... please do something
like issuing one UNSUPPORTED and then returning. 
gdb_suppress_entire_file is evil.


-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA/testsuite/ada] Do not abort prematurely the test upon build failure

[Joel Brobecker]

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

> Unfortunately you didn't pick the best example... please do something
> like issuing one UNSUPPORTED and then returning. 
> gdb_suppress_entire_file is evil.

I knew it! :-) (there's something evil, and sure enough I pick it)

UNSUPPORTED sounds better indeed. Thanks for the suggestion.

I'd like to put the call to unsupported inside gdb_compile_ada in order
to help having a certain consistency between all Ada testcases. So I'd
like to suggest the attached patch instead. The code in null_record.exp
can thus remain the same (simply abort if gdb_compile_ada returns a
failure, nothing else to do since the unsupported has already been
emitted).

2004-03-31  Joel Brobecker  <brobecker@gnat.com>

        * lib/ada.exp (gdb_compile_ada): Emit UNSUPPORTED if we failed
        to build the application. Remove the message printed when in
        verbose mode, redundant with the UNSUPPORTED message above.

(not sure if I used the proper terminology in the ChangeLog entry)

Otherwise, the other approach would be to add the call to unsupported
in null_record.exp itself. In that case, it might be nice to have the
reason for the failure. For that, we would need to update the message
returned by gdb_compile_ada to include that reason, and then include
that reason in the UNSUPPORTED message.

Would either approach be ok?
-- 
Joel

[-- Attachment #2: ada.exp.diff --]
[-- Type: text/plain, Size: 639 bytes --]

Index: ada.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/lib/ada.exp,v
retrieving revision 1.1
diff -u -p -r1.1 ada.exp
--- ada.exp	1 Apr 2004 00:51:13 -0000	1.1
+++ ada.exp	1 Apr 2004 06:06:59 -0000
@@ -405,7 +405,7 @@ proc gdb_compile_ada {source dest type o
     # We therefore simply check whether the dest file has been created
     # or not. Unless not present, the build has succeeded.
     if ![file exists $dest] {
-        verbose "Ada compilation failed: $result"
+	 unsupported "Ada compilation failed: $result"
         return "Ada compilation failed."
     }
 }

Re: [RFA/testsuite/ada] Do not abort prematurely the test upon build failure

[Daniel Jacobowitz]

On Wed, Mar 31, 2004 at 10:18:28PM -0800, Joel Brobecker wrote:
> > Unfortunately you didn't pick the best example... please do something
> > like issuing one UNSUPPORTED and then returning. 
> > gdb_suppress_entire_file is evil.
> 
> I knew it! :-) (there's something evil, and sure enough I pick it)
> 
> UNSUPPORTED sounds better indeed. Thanks for the suggestion.
> 
> I'd like to put the call to unsupported inside gdb_compile_ada in order
> to help having a certain consistency between all Ada testcases. So I'd
> like to suggest the attached patch instead. The code in null_record.exp
> can thus remain the same (simply abort if gdb_compile_ada returns a
> failure, nothing else to do since the unsupported has already been
> emitted).
> 
> 2004-03-31  Joel Brobecker  <brobecker@gnat.com>
> 
>         * lib/ada.exp (gdb_compile_ada): Emit UNSUPPORTED if we failed
>         to build the application. Remove the message printed when in
>         verbose mode, redundant with the UNSUPPORTED message above.
> 
> (not sure if I used the proper terminology in the ChangeLog entry)

This patch is OK.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA/testsuite/ada] Do not abort prematurely the test upon build failure

[Joel Brobecker]

> > 2004-03-31  Joel Brobecker  <brobecker@gnat.com>
> > 
> >         * lib/ada.exp (gdb_compile_ada): Emit UNSUPPORTED if we failed
> >         to build the application. Remove the message printed when in
> >         verbose mode, redundant with the UNSUPPORTED message above.
> > 
> > (not sure if I used the proper terminology in the ChangeLog entry)
> 
> This patch is OK.

Cool, thank you.
Checked in.

-- 
Joel

Re: [RFA/testsuite/ada] Do not abort prematurely the test upon build failure

[Andrew Cagney]

>>>2004-03-31  Joel Brobecker  <brobecker@gnat.com>
>>>> > 
>>>> >         * lib/ada.exp (gdb_compile_ada): Emit UNSUPPORTED if we failed
>>>> >         to build the application. Remove the message printed when in
>>>> >         verbose mode, redundant with the UNSUPPORTED message above.
>>>> > 
>>>> > (not sure if I used the proper terminology in the ChangeLog entry)
>>
>>> 
>>> This patch is OK.
> 
> 
> Cool, thank you.
> Checked in.

Michael, shouldn't this be "UNTESTED"?  The feature works, we just 
failed to test it.

Andrew

Re: [RFA/testsuite/ada] Do not abort prematurely the test upon build failure

[Michael Elizabeth Chastain]

Joel> lib/ada.exp (gdb_compile_ada): Emit UNSUPPORTED if we failed
Joel> to build the application. Remove the message printed when in
Joel> verbose mode, redundant with the UNSUPPORTED message above.

ac> Michael, shouldn't this be "UNTESTED"?  The feature works, we just 
ac> failed to test it.

I favor UNSUPPORTED for this, because the Ada compiler failed to run
on that platform.  Thus Ada is unsupported on that specific machine.
It's a bit of an assumption to go from that specific machine to all
machines of that platform, though.

I won't object if someone gives a result of UNTESTED in a situation
like this, though.

Michael C