Re: [testsuite, ada] Convert ACATS to DejaGnu (PR testsuite/37703)

[Mike Stump <mikestump at comcast dot net>]

On Jan 31, 2011, at 8:24 AM, Rainer Orth wrote:
> As I've mentioned before, I've long meant to convert the ACATS testsuite
> to DejaGnu to provide all dg features (timeouts, xfails, multilib
> support, don't copy all sources into the build dir).  After the gnat.dg
> cleanup patch (and requiring it), here's the first version of a patch
> that does this.  It is by no means ready or polished, but I'm asking for
> general comments, suggestions for fixes or improvements, and testers.

So, there is a good chance this just doesn't work with canadian crosses...

So, lines like:

> +    file mkdir tests
> +    file mkdir support

tend to be wrong.  The build machine, typically doesn't have much on it, and so there are few filesystem operations that apply to it.  Typically, most go to the host or the target.  See remote_file from testsuite/*.exp for ideas.

Now, that said, if people have never tested that way for this testssuite, well, you can skip that as a feature before putting it in, but... at some point, it would be nice to set up a cross and test it, and a canadian cross and test it.


+    # FIXME: How to properly locate host CC?
+    # Place into support subdir.
+    remote_exec host [which gcc] "-c $srcdir/cd/cd300051.c -o support/cd300051.o"

This is non-sensical.  You want the path on the local machine, which is unrelated to the host machine and unrelated to the target machine, and then you want to use that on an unrelated machine...  Oops.  You do this systematically though out the patch.

if ![is_remote host] you can do this, this way, otherwise, this won't work.  See default_gcc_version in gcc.exp for example.  Now, that said, this will work 99% of the time, so you can do that to start as expedient, but it is still wrong.  Using just a simple tool name would be better, then, just document that the testing environment must contain that command in the path for the testing to work.


+# FIXME: CFLAGS seems wrong for Ada, cf. gcc/ada/Make*.

No, CFLAGS is fine, the C stands for compilation, don't you know...  :-)  I think this is right, if you want to retain the functionality of the rest of the infrastructure that can set DEFAULT_CFLAGS.


+    # FIXME: Better use set_board_info, unset_board_info from targetdb.exp?
+    # Or global var or uplevel N info proc ...?
+    set board_info(gnat,acats) 1

+    # FIXME: What's right here, target_info or board_info?
+    if [info exists board_info(gnat,acats)] {
+	# Pass -Isupport to locate support files.
+	# FIXME: Why -gnatws?
+	lappend options "additional_flags=-Isupport -gnatws"
+    }

Kinda a nasty way to do this...  Others have code like:

    if { [istarget "i?86-*-*"]
         || [istarget "m68k-*-*"] } then {
      uplevel 1 lappend additional_flags "-ffloat-store"

or

    if { [string match "*-*-darwin*" $target_triplet] } {
        lappend ALWAYS_CXXFLAGS "ldflags=-multiply_defined suppress"
       }

to set up specific flags for the compiler or for ld.  I'd think you could just set a normal variable and use that variable down below...  Don't see the need to drop into board_info, as this data isn't about the board.  It is about the testcase.


+	switch -glob $basename {
+	    "cxb30*" {
+		# FIXME: Handle via
+		# -- { dg-additional-files "cxb30040.o cxb30060.o cxb30130.o cxb30131.o" }
+		# but already passes without!?

This is nasty.  Generally we put in .x files, that if present, they are run, and they can do whatever you need to.


+    switch $target_triplet {
+	"ia64*-*-*" -
+	"s390*-*-*" {
+	    set target_insn "nop 0"
+	}
+	"mmix-*-*" {
+	    set target_insn "swym 0"
+	}
+	default {
+	    set target_insn "nop"
+	}
+    }

This is nasty...  I'm sure you know why...  I think breaking this out to a target.exp, and then running it here, would be better.


You can have both infrastructures side by side and leave intact the old method.  You can wire up testing to run both methods, and report differences.  Twice the testing time, but trivial to spot unintentional divergences, and possibly a whole lot more testing.  If you do this, you can have a single flag you can flip to use the old scheme, the new scheme, or both schemes.  With that, you can push the changes into the repo with the new scheme off (old on), and not worry about impacting people.  People that want to help test but are two lazy to download your patch, can just flip it on.  Also, if we weren't in stage4, you could flip it on for a week with both running, watch the results mailing lists, and see if there were any faults.

I think the most progress will be made the fastest, when you just start putting it in the tree with it on, or both on.  The only problem with that right now, is the poor timing wrt the release.  If you can build consensus with the Ada people and possibly the RMs on timing issues, you could run experiments in trunk, but...  I think I'd say don't even try.

I would encourage you, if there is a way to do the changes to be acceptable to the Ada folks in terms of safety (can be mitigated by interested parties trying it out), on put all the base infrastructure in and off (for now) and encourage people to turn it on in their environments.  If it prove safe enough to the Ada folks, you could then seek to flip the switch.


Also, be sure to do a make -j16 check to ensure it works (or have someone else do this)...  easy for it not to, and by the time you flip it on, you'd want that to be a known to work feature.


The Ada people will have to weigh in on the addition of -O2 to the files in the testsuite.

Otherwise, looks like a reasonable start.