RFC: Unique testnames

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

RFC: Unique testnames

Nick Clifton
Hi Guys,

  I am planning on committing the attached patch, but I thought that it
  would be a good idea to check with the community first to see if there
  are any comments.

  The patch fixes an issue with the binutils testsuites where some tests
  have identical names.  This is a problem when in comes to reviewing
  failures and trying to identify exactly which test has failed, and it
  is an issue for automatic testing frameworks which need unique names
  for each test.

  In most cases I have just added a number or string to the end of tests
  which have identical names, but in one case I made a bigger change:
  The "objcopy zlib-gnu compress debug sections 3" test in the binutils'
  compress.exp file appears twice, and the two tests appear to be
  completely identical.  I have assumed that the intent was that the
  second version of the test would check the copying of an object file
  containing sections compressed using the zlib-gabi method.  So I have
  tweaked the test to do this.

  One other change that might also affect people who run regression
  tests is that there was a bug in ld-size/size.exp where the size-3e
  test was not properly escaped, meaning that it was not run...  So this
  patch fixes that too.

  Any comments before I check this patch in ?

Cheers
  Nick

binutils/ChangeLog
2018-09-13  Nick Clifton  <[hidden email]>

        * testsuite/binutils-all/compress.exp: Rename second "objcopy
        zlib-gnu compress debug sections 3" test to "objcopy zlib-gabi
        compress debug sections 3" and use gabi object files instead
        of gnu object files.
        * testsuite/binutils-all/objcopy.exp: Add suffix to the names
        of the "ELF group" tests.
        * testsuite/binutils-all/readelf.exp (proc readelf_find_size):
        Add an iteration parameter and include it in the name of the
        test.  Update callers to include an iteration count.
       
gas/ChangeLog
2018-09-13  Nick Clifton  <[hidden email]>

        * testuite/gas/elf/group0a.d: Add extra details to the test
        name.
        * testuite/gas/elf/group0b.d: Likewise.
        * testuite/gas/elf/group1a.d: Likewise.
        * testuite/gas/elf/group1b.d: Likewise.
        * testuite/gas/elf/group0b.d: Likewise.
        * testuite/gas/elf/section9.d: Likewise.
        * testuite/gas/i386/ilp32/lns/lns-common-1.d: Likewise.
        * testuite/gas/i386/ilp32/lns/lns-duplicate-1.d: Likewise.

gas/ChangeLog
2018-09-13  Nick Clifton  <[hidden email]>

        * testuite/ld/ld-elf/audit.exp: Differentiate the names of the
        two "Run with shared with --audit" tests.
        * testuite/ld/ld-elf/compress.exp: Differentiate the zlib
        compressed debug output test names.
        * testuite/ld/ld-i386/tlspie1.d: Add extra details to the test
        name.
        * testuite/ld/ld-i386/tlspie2.d: Likewise.
        * testuite/ld/ld-size/size.exp: Add missing escapes to the end
        of lines in the size-3e test.
        * testuite/ld/ld-unique/unique.exp: Differentiate the names of
        the two "Checking unique PIC object" tests.
        * testuite/ld/ld-x86-64/tlspie1.d: Add extra details to the test
        name.
       

diff --git a/binutils/testsuite/binutils-all/compress.exp b/binutils/testsuite/binutils-all/compress.exp
index 948d20a451..3dcfa180b2 100644
--- a/binutils/testsuite/binutils-all/compress.exp
+++ b/binutils/testsuite/binutils-all/compress.exp
@@ -505,16 +505,16 @@ if ![string match "" $exec_output] then {
     pass "objcopy ($testname)"
 }
 
-set testname "objcopy zlib-gnu compress debug sections 3"
-set got [binutils_run $OBJCOPY "${compressedfile3}gnu.o ${copyfile}gnu.o"]
+set testname "objcopy zlib-gabi compress debug sections 3"
+set got [binutils_run $OBJCOPY "${compressedfile3}gabi.o ${copyfile}gabi.o"]
 if ![string match "" $got] then {
     fail "objcopy ($testname)"
     return
 }
-send_log "cmp ${compressedfile3}gnu.o ${copyfile}gnu.o\n"
-verbose "cmp ${compressedfile3}gnu.o ${copyfile}gnu.o"
-set src1 ${compressedfile3}gnu.o
-set src2 ${copyfile}gnu.o
+send_log "cmp ${compressedfile3}gabi.o ${copyfile}gabi.o\n"
+verbose "cmp ${compressedfile3}gabi.o ${copyfile}gabi.o"
+set src1 ${compressedfile3}gabi.o
+set src2 ${copyfile}gabi.o
 set status [remote_exec build cmp "${src1} ${src2}"]
 set exec_output [lindex $status 1]
 set exec_output [prune_warnings $exec_output]
diff --git a/binutils/testsuite/binutils-all/objcopy.exp b/binutils/testsuite/binutils-all/objcopy.exp
index 61793d98f5..a94c48c35f 100644
--- a/binutils/testsuite/binutils-all/objcopy.exp
+++ b/binutils/testsuite/binutils-all/objcopy.exp
@@ -1045,10 +1045,10 @@ if [is_elf_format] {
     objcopy_test_symbol_manipulation
     objcopy_test_elf_common_symbols
     objcopy_test "ELF unknown section type" unknown.s
-    objcopy_test_readelf "ELF group" group.s
-    objcopy_test_readelf "ELF group" group-2.s
-    objcopy_test_readelf "ELF group" group-3.s
-    objcopy_test_readelf "ELF group" group-4.s
+    objcopy_test_readelf "ELF group 1" group.s
+    objcopy_test_readelf "ELF group 2" group-2.s
+    objcopy_test_readelf "ELF group 3" group-3.s
+    objcopy_test_readelf "ELF group 4" group-4.s
     objcopy_test_readelf "GNU_MBIND section" mbind1.s
     run_dump_test "group-5"
     run_dump_test "group-6"
diff --git a/binutils/testsuite/binutils-all/readelf.exp b/binutils/testsuite/binutils-all/readelf.exp
index 45a022a8b6..7a1edd6d13 100644
--- a/binutils/testsuite/binutils-all/readelf.exp
+++ b/binutils/testsuite/binutils-all/readelf.exp
@@ -40,13 +40,13 @@ proc file_contents { filename } {
 # Find out the size by reading the output of the EI_CLASS field.
 # Similar to the test for readelf -h, but we're just looking for the
 # EI_CLASS line here.
-proc readelf_find_size { binary_file } {
+proc readelf_find_size { binary_file test_iteration } {
     global READELF
     global READELFFLAGS
     global readelf_size
 
     set readelf_size ""
-    set testname "finding out ELF size with readelf -h"
+    set testname "finding out ELF size with readelf -h ($test_iteration)"
     set got [remote_exec host "$READELF $READELFFLAGS -h $binary_file" "" "/dev/null" "readelf.out"]
     if [is_remote host] then {
         remote_upload host "readelf.out"
@@ -335,7 +335,7 @@ if {![binutils_assemble $srcdir/$subdir/bintest.s tmpdir/bintest.o]} then {
     }
 
     # First, determine the size, so specific output matchers can be used.
-    readelf_find_size $tempfile
+    readelf_find_size $tempfile 1
 
     # Run the tests.
     readelf_test -h $tempfile readelf.h  {}
@@ -444,7 +444,7 @@ if ![istarget "riscv*-*-*"] then {
  }
 
  # First, determine the size, so specific output matchers can be used.
- readelf_find_size $tempfile
+ readelf_find_size $tempfile 2
 
  # Make sure that readelf can decode the contents.
  readelf_test -wiaoRlL $tempfile dw5.W { nds32*-elf }
@@ -463,7 +463,7 @@ if {![binutils_assemble_flags $srcdir/$subdir/dwarf-attributes.S tmpdir/dwarf-at
     }
 
     # First, determine the size, so specific output matchers can be used.
-    readelf_find_size $tempfile
+    readelf_find_size $tempfile 3
 
     # Make sure that readelf can decode the contents.
     readelf_test -wi $tempfile dwarf-attributes.W {}
diff --git a/gas/testsuite/gas/elf/group0a.d b/gas/testsuite/gas/elf/group0a.d
index e6b9366c8b..be231603c7 100644
--- a/gas/testsuite/gas/elf/group0a.d
+++ b/gas/testsuite/gas/elf/group0a.d
@@ -1,5 +1,5 @@
 #readelf: -SW
-#name: group section
+#name: group section (using readelf -SW)
 #source: group0.s
 
 #...
diff --git a/gas/testsuite/gas/elf/group0b.d b/gas/testsuite/gas/elf/group0b.d
index fc74ea6398..c2b5f58890 100644
--- a/gas/testsuite/gas/elf/group0b.d
+++ b/gas/testsuite/gas/elf/group0b.d
@@ -1,5 +1,5 @@
 #readelf: -g
-#name: group section
+#name: group section (using readelf -g)
 #source: group0.s
 
 #...
diff --git a/gas/testsuite/gas/elf/group1a.d b/gas/testsuite/gas/elf/group1a.d
index d264766f7b..6be70a75f3 100644
--- a/gas/testsuite/gas/elf/group1a.d
+++ b/gas/testsuite/gas/elf/group1a.d
@@ -1,5 +1,5 @@
 #readelf: -SW
-#name: group section with multiple sections of same name
+#name: group section with multiple sections of same name (using readelf -SW)
 #source: group1.s
 # The RX port uses non-standard section names.
 #not-target: rx-*
diff --git a/gas/testsuite/gas/elf/group1b.d b/gas/testsuite/gas/elf/group1b.d
index 405e2036f7..9c592edaaa 100644
--- a/gas/testsuite/gas/elf/group1b.d
+++ b/gas/testsuite/gas/elf/group1b.d
@@ -1,5 +1,5 @@
 #readelf: -g
-#name: group section with multiple sections of same name
+#name: group section with multiple sections of same name (using readelf -g)
 #source: group1.s
 
 #...
diff --git a/gas/testsuite/gas/elf/section9.d b/gas/testsuite/gas/elf/section9.d
index 1acf63e819..63152fe4b4 100644
--- a/gas/testsuite/gas/elf/section9.d
+++ b/gas/testsuite/gas/elf/section9.d
@@ -1,5 +1,5 @@
 #readelf: -S --wide
-#name: section flags
+#name: section flags (for GNU lto sections)
 
 #...
 [ ]*\[.*\][ ]+\.gnu\.lto_main[ ]+PROGBITS.*[ ]+E[   ]+.*
diff --git a/gas/testsuite/gas/i386/ilp32/lns/lns-common-1.d b/gas/testsuite/gas/i386/ilp32/lns/lns-common-1.d
index 7b1d5c2132..0f6e690c30 100644
--- a/gas/testsuite/gas/i386/ilp32/lns/lns-common-1.d
+++ b/gas/testsuite/gas/i386/ilp32/lns/lns-common-1.d
@@ -1,6 +1,6 @@
 #source: ../../../lns/lns-common-1.s
 #readelf: -wl
-#name: lns-common-1
+#name: lns-common-1 (ILP32)
 Raw dump of debug contents of section \.z?debug_line:
 #...
   Initial value of 'is_stmt':  1
diff --git a/gas/testsuite/gas/i386/ilp32/lns/lns-duplicate.d b/gas/testsuite/gas/i386/ilp32/lns/lns-duplicate.d
index fc0f861eb7..ffea47ab94 100644
--- a/gas/testsuite/gas/i386/ilp32/lns/lns-duplicate.d
+++ b/gas/testsuite/gas/i386/ilp32/lns/lns-duplicate.d
@@ -1,6 +1,6 @@
 #source: ../../../lns/lns-duplicate.s
 #readelf: -wl
-#name: lns-duplicate
+#name: lns-duplicate (ILP32)
 Raw dump of debug contents of section \.z?debug_line:
 #...
  Line Number Statements:
diff --git a/ld/testsuite/ld-elf/audit.exp b/ld/testsuite/ld-elf/audit.exp
index 618f425add..45b5c4602f 100644
--- a/ld/testsuite/ld-elf/audit.exp
+++ b/ld/testsuite/ld-elf/audit.exp
@@ -49,7 +49,7 @@ set build_tests {
   {"Run with shared with --audit"
      "-shared -Wl,--audit=tmpdir/audit.so" "-fPIC"
      {main.c} {} "libusesaudit.so"}
-  {"Run with shared with --audit"
+  {"Run with shared with three --audit"
      "-shared -Wl,--audit=tmpdir/audit.so -Wl,--audit=tmpdir/audit2.so \
      -Wl,--audit=tmpdir/audit3.so"
      "-fPIC"
diff --git a/ld/testsuite/ld-elf/compress.exp b/ld/testsuite/ld-elf/compress.exp
index 0020e9941f..040bb52ad2 100644
--- a/ld/testsuite/ld-elf/compress.exp
+++ b/ld/testsuite/ld-elf/compress.exp
@@ -168,7 +168,7 @@ if { [regexp_diff tmpdir/$test.out $srcdir/$subdir/$test.rt] } then {
     pass "$test_name"
 }
 
-set test_name "Link with zlib compressed debug output"
+set test_name "Link with zlib compressed debug output 1"
 set test normal
 send_log "$READELF -w tmpdir/$test > tmpdir/$test.out\n"
 set got [remote_exec host [concat sh -c [list "$READELF -w tmpdir/$test > tmpdir/$test.out"]] "" "/dev/null"]
@@ -177,7 +177,7 @@ if { [lindex $got 0] != 0 || ![string match "" [lindex $got 1]] } then {
     unresolved "$test_name"
 }
 
-set test_name "Link with zlib compressed debug output"
+set test_name "Link with zlib compressed debug output 2"
 set test zlibnormal
 send_log "$READELF -w tmpdir/$test | sed -e \"s/.zdebug_/.debug_/\" > tmpdir/$test.out\n"
 set got [remote_exec host [concat sh -c [list "$READELF -w tmpdir/$test | sed -e \"s/.zdebug_/.debug_/\" > tmpdir/$test.out"]] "" "/dev/null"]
@@ -191,6 +191,7 @@ if { [catch {exec cmp tmpdir/normal.out tmpdir/$test.out}] } then {
 } else {
     pass "$test_name"
 }
+set test_name "Link with zlib compressed debug output 3"
 send_log "$READELF -S -W tmpdir/$test' > tmpdir/$test.out\n"
 set got [remote_exec host "$READELF -S -W tmpdir/$test" "" "/dev/null" "tmpdir/$test.out"]
 if { [lindex $got 0] != 0 || ![string match "" [lindex $got 1]] } then {
@@ -203,7 +204,7 @@ if { [regexp_diff tmpdir/$test.out $srcdir/$subdir/$test.rS] } then {
     pass "$test_name"
 }
 
-set test_name "Link with zlib-gnu compressed debug output"
+set test_name "Link with zlib-gnu compressed debug output 1"
 set test gnunormal
 send_log "$READELF -w tmpdir/$test | sed -e \"s/.zdebug_/.debug_/\" > tmpdir/$test.out\n"
 set got [remote_exec host [concat sh -c [list "$READELF -w tmpdir/$test | sed -e \"s/.zdebug_/.debug_/\" > tmpdir/$test.out"]] "" "/dev/null"]
@@ -217,6 +218,7 @@ if { [catch {exec cmp tmpdir/normal.out tmpdir/$test.out}] } then {
 } else {
     pass "$test_name"
 }
+set test_name "Link with zlib-gnu compressed debug output 2"
 send_log "$READELF -S -W tmpdir/$test' > tmpdir/$test.out\n"
 set got [remote_exec host "$READELF -S -W tmpdir/$test" "" "/dev/null" "tmpdir/$test.out"]
 if { [lindex $got 0] != 0 || ![string match "" [lindex $got 1]] } then {
@@ -230,7 +232,7 @@ if { [regexp_diff tmpdir/$test.out $srcdir/$subdir/$test.rS] } then {
 }
 
 set test gabinormal
-set test_name "Link with zlib-gabi compressed debug output"
+set test_name "Link with zlib-gabi compressed debug output 1"
 send_log "$READELF -w tmpdir/$test > tmpdir/$test.out\n"
 set got [remote_exec host [concat sh -c [list "$READELF -w tmpdir/$test > tmpdir/$test.out"]] "" "/dev/null"]
 if { [lindex $got 0] != 0 || ![string match "" [lindex $got 1]] } then {
@@ -243,6 +245,7 @@ if { [catch {exec cmp tmpdir/normal.out tmpdir/$test.out}] } then {
 } else {
     pass "$test_name"
 }
+set test_name "Link with zlib-gabi compressed debug output 2"
 send_log "$READELF -t -W tmpdir/$test > tmpdir/$test.out\n"
 set got [remote_exec host "$READELF -t -W tmpdir/$test" "" "/dev/null" "tmpdir/$test.out"]
 if { [lindex $got 0] != 0 || ![string match "" [lindex $got 1]] } then {
diff --git a/ld/testsuite/ld-i386/tlspie1.d b/ld/testsuite/ld-i386/tlspie1.d
index bac5dc67de..7ad385569d 100644
--- a/ld/testsuite/ld-i386/tlspie1.d
+++ b/ld/testsuite/ld-i386/tlspie1.d
@@ -1,4 +1,4 @@
-#name: TLS with PIE
+#name: TLS with PIE (1 x86)
 #as: --32
 #ld: -melf_i386 -pie
 #readelf: -r
diff --git a/ld/testsuite/ld-i386/tlspie2.d b/ld/testsuite/ld-i386/tlspie2.d
index 799d646c62..202e4fbdde 100644
--- a/ld/testsuite/ld-i386/tlspie2.d
+++ b/ld/testsuite/ld-i386/tlspie2.d
@@ -1,4 +1,4 @@
-#name: TLS with PIE
+#name: TLS with PIE (2 x86)
 #as: --32
 #ld: -melf_i386 -pie
 #objdump: -dw
diff --git a/ld/testsuite/ld-size/size.exp b/ld/testsuite/ld-size/size.exp
index 2ff92c1674..e193e04c04 100644
--- a/ld/testsuite/ld-size/size.exp
+++ b/ld/testsuite/ld-size/size.exp
@@ -226,9 +226,9 @@ run_ld_link_exec_tests [list \
  "size-3.out" \
     ] \
     [list \
-    {"Run size-3e"
-     "-Wl,--no-as-needed tmpdir/libsize-3c.so" ""
-     {size-3.c} "size-3e" "size-3.out"}
+    {"Run size-3e" \
+     "-Wl,--no-as-needed tmpdir/libsize-3c.so" "" \
+     {size-3.c} "size-3e" "size-3.out"} \
     ] \
     [list \
  "Run size-4a" \
diff --git a/ld/testsuite/ld-unique/unique.exp b/ld/testsuite/ld-unique/unique.exp
index cc3bf08d5f..9a704dd7ed 100644
--- a/ld/testsuite/ld-unique/unique.exp
+++ b/ld/testsuite/ld-unique/unique.exp
@@ -229,7 +229,7 @@ if {[contains_unique_symbol tmpdir/unique_shared.o] != 1} {
 }
 
 if { $fails == 0 } {
-  pass "Checking unique PIC object"
+  pass "Checking unique PIC object 1"
 }
 
 # Check the unique shared library.
@@ -255,7 +255,7 @@ if {[contains_unique_symbol tmpdir/libunique_shared_ref.so] != 1} {
 }
 
 if { $fails == 0 } {
-  pass "Checking unique PIC object"
+  pass "Checking unique PIC object 2"
 }
 
 # Check the empty executable linked against unique shared library.
diff --git a/ld/testsuite/ld-x86-64/tlspie1.d b/ld/testsuite/ld-x86-64/tlspie1.d
index e06e8b1afb..44f73b6d18 100644
--- a/ld/testsuite/ld-x86-64/tlspie1.d
+++ b/ld/testsuite/ld-x86-64/tlspie1.d
@@ -1,4 +1,4 @@
-#name: TLS with PIE
+#name: TLS with PIE (1 x86_64)
 #as: --64
 #ld: -melf_x86_64 -pie
 #readelf: -r
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Unique testnames

Jeff Law
On 9/13/18 5:15 AM, Nick Clifton wrote:

> Hi Guys,
>
>   I am planning on committing the attached patch, but I thought that it
>   would be a good idea to check with the community first to see if there
>   are any comments.
>
>   The patch fixes an issue with the binutils testsuites where some tests
>   have identical names.  This is a problem when in comes to reviewing
>   failures and trying to identify exactly which test has failed, and it
>   is an issue for automatic testing frameworks which need unique names
>   for each test.
>
>   In most cases I have just added a number or string to the end of tests
>   which have identical names, but in one case I made a bigger change:
>   The "objcopy zlib-gnu compress debug sections 3" test in the binutils'
>   compress.exp file appears twice, and the two tests appear to be
>   completely identical.  I have assumed that the intent was that the
>   second version of the test would check the copying of an object file
>   containing sections compressed using the zlib-gabi method.  So I have
>   tweaked the test to do this.
>
>   One other change that might also affect people who run regression
>   tests is that there was a bug in ld-size/size.exp where the size-3e
>   test was not properly escaped, meaning that it was not run...  So this
>   patch fixes that too.
>
>   Any comments before I check this patch in ?
So to give wider background to the community.  It's the .sum file
comparison tool provided by GCC (contrib/compare_tests) that doesn't
handle this case gracefully.

If the tests have different PASS/FAIL states, the tool will report that
there were regressions/errors in the new testrun.  Uniqueness of the
testnames avoids this problem.

The irony is that the Codesourcery guys were raising the uniqueness of
testnames issues for GCC eons ago (in the QMtest discussions).  I
largely ignored it at the time since I've always done my comparisons by
hand (thankfully others took on the task of addressing this issue for
GCC).  Of course this stuff matters with automatic regression testers :-)

jeff

Reply | Threaded
Open this post in threaded view
|

Re: RFC: Unique testnames

Pedro Alves-7
On 09/13/2018 03:29 PM, Jeff Law wrote:

> So to give wider background to the community.  It's the .sum file
> comparison tool provided by GCC (contrib/compare_tests) that doesn't
> handle this case gracefully.
>
> If the tests have different PASS/FAIL states, the tool will report that
> there were regressions/errors in the new testrun.  Uniqueness of the
> testnames avoids this problem.
>
> The irony is that the Codesourcery guys were raising the uniqueness of
> testnames issues for GCC eons ago (in the QMtest discussions).  I
> largely ignored it at the time since I've always done my comparisons by
> hand (thankfully others took on the task of addressing this issue for
> GCC).  Of course this stuff matters with automatic regression testers :-)
>

We're pretty anal about this in GDB too, for the same reasons.  :-)  See:

 https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Make_sure_test_messages_are_unique

Note GDB avoids using " (foo)" at end of names as a way to make test
names unique.  Not sure whether that's relevant for binutils:

  https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Do_not_use_.22tail_parentheses.22_on_test_messages

Thanks,
Pedro Alves
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Unique testnames

Nick Clifton
Hi Pedro,

> We're pretty anal about this in GDB too, for the same reasons.  :-)  See:
>
>  https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Make_sure_test_messages_are_unique

Thanks for the reference.

> Note GDB avoids using " (foo)" at end of names as a way to make test
> names unique.  Not sure whether that's relevant for binutils:
>
>   https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Do_not_use_.22tail_parentheses.22_on_test_messages

Well the use of parenthesised comments at the end of test names is
failry well established in the binutils now, so I am not going to
change it with this patch.  Maybe one day in the future.


Anyway - since there have been no objections, I am going to check
the patch in.

Cheers
  Nick