[testsuite] Include gdc.test prefix in test names (PR testsuite/88041)

Message ID ydda7l9iy79.fsf@CeBiTec.Uni-Bielefeld.DE
State New
Headers show
Series
  • [testsuite] Include gdc.test prefix in test names (PR testsuite/88041)
Related show

Commit Message

Rainer Orth Dec. 13, 2018, 9:16 a.m.
As reported in the PR, the test names of DejaGnu tests should be the
path relative to the respective testsuite directory, a convention the
gdc.test tests currently don't follow.

The patch below fixes this.  After a few false starts (some described in
the PR and several more) I came up with the following minimally
intrusive solution.  This way, the gdc.test prefix is included in the
test names but stripped out again in gdc-dg-test so all path names in
the upstream testsuite (which don't include the prefix) continue to
work.  The link from gdc.test to . in the gdc* dirs is still needed
because e.g. scanning the sources for dg- directives happens in
dg-runtest before entering dg-test and thus still needs the prefixed
form.

I'm omitting the changes (also mentioned in the PR) to
compilable/ddoc9676a.d and compilable/depsOutput9948.d which contain
absolute path names in EXTRA_SOURCES because those need to go upstream
first.

Bootstrapped without regressions on i386-pc-solaris2.11 and
x86_64-pc-linux-gnu.  Ok for mainline?

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

	PR testsuite/88041
	* lib/gdc-dg.exp (gdc-dg-test): Strip gdc.test prefix.
	* gdc.test/gdc-test.exp (gdc-do-test): Create $subdir link.
	Include $subdir in filename.
	Cleanup generated source.

Comments

Iain Buclaw Dec. 13, 2018, 9:56 a.m. | #1
On Thu, 13 Dec 2018 at 10:16, Rainer Orth <ro@cebitec.uni-bielefeld.de> wrote:
>

> As reported in the PR, the test names of DejaGnu tests should be the

> path relative to the respective testsuite directory, a convention the

> gdc.test tests currently don't follow.

>

> The patch below fixes this.  After a few false starts (some described in

> the PR and several more) I came up with the following minimally

> intrusive solution.  This way, the gdc.test prefix is included in the

> test names but stripped out again in gdc-dg-test so all path names in

> the upstream testsuite (which don't include the prefix) continue to

> work.  The link from gdc.test to . in the gdc* dirs is still needed

> because e.g. scanning the sources for dg- directives happens in

> dg-runtest before entering dg-test and thus still needs the prefixed

> form.

>

> I'm omitting the changes (also mentioned in the PR) to

> compilable/ddoc9676a.d and compilable/depsOutput9948.d which contain

> absolute path names in EXTRA_SOURCES because those need to go upstream

> first.

>


This has been committed upstream
(https://github.com/dlang/dmd/pull/9051) - I just need to trickle it
down, among other patches that are in my queue.

I have no problem however if you go ahead commit the changes with this patch.

> Bootstrapped without regressions on i386-pc-solaris2.11 and

> x86_64-pc-linux-gnu.  Ok for mainline?

>


OK on my side, just one mental note.

> @@ -360,6 +361,9 @@ proc gdc-do-test { } {

>      # Initialize `dg'.

>      dg-init

>

> +    # Create gdc.test link so test names include that subdir.

> +    catch { file link $subdir . }

> +


I guess the proper fix would be to split this into three files, right?

  - compilable/compilable.exp
  - fail_compilation/fail_compilation.exp
  - runnable/runnable.exp

Then move the common bits into lib/gdc-dmd.exp

-- 
Iain
Rainer Orth Dec. 13, 2018, 3:01 p.m. | #2
Hi Iain,

>> I'm omitting the changes (also mentioned in the PR) to

>> compilable/ddoc9676a.d and compilable/depsOutput9948.d which contain

>> absolute path names in EXTRA_SOURCES because those need to go upstream

>> first.

>>

>

> This has been committed upstream

> (https://github.com/dlang/dmd/pull/9051) - I just need to trickle it

> down, among other patches that are in my queue.


great, thanks.

> I have no problem however if you go ahead commit the changes with this patch.


Good: that's what I did.

>> Bootstrapped without regressions on i386-pc-solaris2.11 and

>> x86_64-pc-linux-gnu.  Ok for mainline?

>

> OK on my side, just one mental note.

>

>> @@ -360,6 +361,9 @@ proc gdc-do-test { } {

>>      # Initialize `dg'.

>>      dg-init

>>

>> +    # Create gdc.test link so test names include that subdir.

>> +    catch { file link $subdir . }

>> +

>

> I guess the proper fix would be to split this into three files, right?

>

>   - compilable/compilable.exp

>   - fail_compilation/fail_compilation.exp

>   - runnable/runnable.exp

>

> Then move the common bits into lib/gdc-dmd.exp


While that's possible, we usually only have per-subdir .exp files when
e.g. there are massive/systematic differences in the set of compilation
flags used.  In the case of those three subdirs, you could indeed split
into per-subdir files and a common one, but it's also fine as is.

The $subdir link above (where subdir is gdc.test, the directory below
.../testsuite) only achieves that both (say)
gdc.test/compilable/99bottles.d as passed to dg-runtest and
imports.jsonimport1 (without the prefixe and referenced in the sources
and -I flags) work just the same.  So this will remain necessary even if
you decide to split gdc-test.exp into three .exp files.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Patch

# HG changeset patch
# Parent  10a25e7dfa1774247c9accc8f36c21dabdf72c44
Fix various gdc.test issues

diff --git a/gcc/testsuite/gdc.test/gdc-test.exp b/gcc/testsuite/gdc.test/gdc-test.exp
--- a/gcc/testsuite/gdc.test/gdc-test.exp
+++ b/gcc/testsuite/gdc.test/gdc-test.exp
@@ -180,6 +180,7 @@  proc dmd2dg { base test } {
     global DEFAULT_DFLAGS
     global PERMUTE_ARGS
     global GDC_EXECUTE_ARGS
+    global subdir
 
     set PERMUTE_ARGS $DEFAULT_DFLAGS
     set GDC_EXECUTE_ARGS ""
@@ -360,6 +361,9 @@  proc gdc-do-test { } {
     # Initialize `dg'.
     dg-init
 
+    # Create gdc.test link so test names include that subdir.
+    catch { file link $subdir . }
+
     # Main loop.
 
     # set verbose 1
@@ -380,7 +384,8 @@  proc gdc-do-test { } {
 
         # Convert to DG test.
         set imports [format "-I%s/%s" $base $dir]
-        set filename [dmd2dg $base $dir/$name.$ext]
+        # Include $subdir prefix so test names follow DejaGnu conventions.
+        set filename "$subdir/[dmd2dg $base $dir/$name.$ext]"
 
         if { $dir == "runnable" } {
             append PERMUTE_ARGS " $SHARED_OPTION"
@@ -423,7 +428,7 @@  proc gdc-do-test { } {
         }
 
         # Cleanup
-        #file delete $filename
+        file delete $filename
     }
 
     # All done.
diff --git a/gcc/testsuite/lib/gdc-dg.exp b/gcc/testsuite/lib/gdc-dg.exp
--- a/gcc/testsuite/lib/gdc-dg.exp
+++ b/gcc/testsuite/lib/gdc-dg.exp
@@ -32,6 +32,10 @@  proc gdc-dg-test { prog do_what extra_to
 	}
     }
 
+    # Strip gdc.test prefix off test names to avoid pathname failures in
+    # some tests.
+    set prog [dg-trim-dirname gdc.test $prog]
+
     set result \
         [gcc-dg-test-1 gdc_target_compile $prog $do_what $extra_tool_flags]