diff mbox series

[ping^3] Make sure that we get unique test names if several DejaGnu directives refer to the same line [PR102735]

Message ID 87mtmfx6x3.fsf@euler.schwinge.homeip.net
State New
Headers show
Series [ping^3] Make sure that we get unique test names if several DejaGnu directives refer to the same line [PR102735] | expand

Commit Message

Thomas Schwinge Nov. 8, 2021, 10:45 a.m. UTC
Hi!

Ping, once more.


Grüße
 Thomas


On 2021-10-14T12:12:41+0200, I wrote:
> Hi!
>
> Ping, again.
>
> Commit log updated for <https://gcc.gnu.org/PR102735>
> "privatization-1-compute.c results in both XFAIL and PASS".
>
>
> Grüße
>  Thomas
>
>
> On 2021-09-30T08:42:25+0200, I wrote:
>> Hi!
>>
>> Ping.
>>
>> On 2021-09-22T13:03:46+0200, I wrote:
>>> On 2021-09-19T11:35:00-0600, Jeff Law via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>>>> A couple of goacc tests do not have unique names.
>>>
>>> Thanks for fixing this up, and sorry, largely my "fault", I suppose.  ;-|
>>>
>>>> This causes problems
>>>> for the test comparison script when one of the test passes and the other
>>>> fails -- in this scenario the test comparison script claims there is a
>>>> regression.
>>>
>>> So I understand correctly that this is a problem not just for actual
>>> mixed PASS vs. FAIL (which we'd like you to report anyway!) that appear
>>> for the same line, but also for mixed PASS vs. XFAIL?  (Because, the
>>> latter appears to be what you're addressing with your commit here.)
>>>
>>>> This slipped through for a while because I had turned off x86_64 testing
>>>> (others test it regularly and I was revamping the tester's hardware
>>>> requirements).  Now that I've acquired more x86_64 resources and turned
>>>> on native x86 testing again, it's been flagged.
>>>
>>> (I don't follow that argument -- these test cases should be all generic?
>>> Anyway, not important, I guess.)
>>>
>>>> This patch just adds a numeric suffix to the TODO string to disambiguate
>>>> them.
>>>
>>> So, instead of doing this manually (always error-prone!), like you've...
>>>
>>>> Committed to the trunk,
>>>
>>>> commit f75b237254f32d5be32c9d9610983b777abea633
>>>> Author: Jeff Law <jeffreyalaw@gmail.com>
>>>> Date:   Sun Sep 19 13:31:32 2021 -0400
>>>>
>>>>     [committed] Make test names unique for a couple of goacc tests
>>>
>>>> --- a/gcc/testsuite/gfortran.dg/goacc/privatization-1-compute.f90
>>>> +++ b/gcc/testsuite/gfortran.dg/goacc/privatization-1-compute.f90
>>>> @@ -39,9 +39,9 @@ contains
>>>>            !$acc atomic write ! ... to force 'TREE_ADDRESSABLE'.
>>>>            y = a
>>>>      !$acc end parallel
>>>> -    ! { dg-note {variable 'i' in 'private' clause potentially has improper OpenACC privatization level: 'parm_decl'} "TODO" { xfail *-*-* } l_compute$c_compute }
>>>> -    ! { dg-note {variable 'j' in 'private' clause potentially has improper OpenACC privatization level: 'parm_decl'} "TODO" { xfail *-*-* } l_compute$c_compute }
>>>> -    ! { dg-note {variable 'a' in 'private' clause potentially has improper OpenACC privatization level: 'parm_decl'} "TODO" { xfail *-*-* } l_compute$c_compute }
>>>> +    ! { dg-note {variable 'i' in 'private' clause potentially has improper OpenACC privatization level: 'parm_decl'} "TODO2" { xfail *-*-* } l_compute$c_compute }
>>>> +    ! { dg-note {variable 'j' in 'private' clause potentially has improper OpenACC privatization level: 'parm_decl'} "TODO3" { xfail *-*-* } l_compute$c_compute }
>>>> +    ! { dg-note {variable 'a' in 'private' clause potentially has improper OpenACC privatization level: 'parm_decl'} "TODO4" { xfail *-*-* } l_compute$c_compute }
>>>
>>> ... etc. (also similarly in a handful of earlier commits, if I remember
>>> correctly), why don't we do that programmatically, like in the attached
>>> "Make sure that we get unique test names if several DejaGnu directives
>>> refer to the same line", once and for all?  OK to push after proper
>>> testing?
>>
>> Attached again, for easy reference.
>>
>> I figure it may help if I showed an example of how this changes things;
>> for the test case cited above (word-diff):
>>
>>     PASS: gfortran.dg/goacc/privatization-1-compute.f90   -O   {+at line 40+} (test for warnings, line 39)
>>     PASS: gfortran.dg/goacc/privatization-1-compute.f90   -O   {+at line 41+} (test for warnings, line 22)
>>     PASS: gfortran.dg/goacc/privatization-1-compute.f90   -O   {+at line 42+} (test for warnings, line 39)
>>     PASS: gfortran.dg/goacc/privatization-1-compute.f90   -O   {+at line 43+} (test for warnings, line 22)
>>     PASS: gfortran.dg/goacc/privatization-1-compute.f90   -O   {+at line 44+} (test for warnings, line 39)
>>     PASS: gfortran.dg/goacc/privatization-1-compute.f90   -O   {+at line 45+} (test for warnings, line 22)
>>     XFAIL: gfortran.dg/goacc/privatization-1-compute.f90   -O  TODO2 {+at line 50+} (test for warnings, line 29)
>>     XFAIL: gfortran.dg/goacc/privatization-1-compute.f90   -O  TODO3 {+at line 51+} (test for warnings, line 29)
>>     XFAIL: gfortran.dg/goacc/privatization-1-compute.f90   -O  TODO4 {+at line 52+} (test for warnings, line 29)
>>     PASS: gfortran.dg/goacc/privatization-1-compute.f90   -O  TODO {+at line 53+} (test for warnings, line 29)
>>     PASS: gfortran.dg/goacc/privatization-1-compute.f90   -O   {+at line 54+} (test for warnings, line 29)
>>     PASS: gfortran.dg/goacc/privatization-1-compute.f90   -O  (test for excess errors)
>>
>> Given that we now amend the 'comment' by 'at line $useline"', and given
>> that only ever one DejaGnu directive may appear on one source line, all
>> these output lines now must be unique.  (If we wanted to, we again could
>> 's%TODO[0-9]+%TODO%', reverting your change cited above.)
>>
>>
>> Grüße
>>  Thomas


-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

Comments

Thomas Schwinge Nov. 15, 2021, 2:50 p.m. UTC | #1
Hi!

..., and here is another ping.


Grüße
 Thomas


On 2021-11-08T11:45:12+0100, I wrote:
> Hi!
>
> Ping, once more.
>
>
> Grüße
>  Thomas
>
>
> On 2021-10-14T12:12:41+0200, I wrote:
>> Hi!
>>
>> Ping, again.
>>
>> Commit log updated for <https://gcc.gnu.org/PR102735>
>> "privatization-1-compute.c results in both XFAIL and PASS".
>>
>>
>> Grüße
>>  Thomas
>>
>>
>> On 2021-09-30T08:42:25+0200, I wrote:
>>> Hi!
>>>
>>> Ping.
>>>
>>> On 2021-09-22T13:03:46+0200, I wrote:
>>>> On 2021-09-19T11:35:00-0600, Jeff Law via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>>>>> A couple of goacc tests do not have unique names.
>>>>
>>>> Thanks for fixing this up, and sorry, largely my "fault", I suppose.  ;-|
>>>>
>>>>> This causes problems
>>>>> for the test comparison script when one of the test passes and the other
>>>>> fails -- in this scenario the test comparison script claims there is a
>>>>> regression.
>>>>
>>>> So I understand correctly that this is a problem not just for actual
>>>> mixed PASS vs. FAIL (which we'd like you to report anyway!) that appear
>>>> for the same line, but also for mixed PASS vs. XFAIL?  (Because, the
>>>> latter appears to be what you're addressing with your commit here.)
>>>>
>>>>> This slipped through for a while because I had turned off x86_64 testing
>>>>> (others test it regularly and I was revamping the tester's hardware
>>>>> requirements).  Now that I've acquired more x86_64 resources and turned
>>>>> on native x86 testing again, it's been flagged.
>>>>
>>>> (I don't follow that argument -- these test cases should be all generic?
>>>> Anyway, not important, I guess.)
>>>>
>>>>> This patch just adds a numeric suffix to the TODO string to disambiguate
>>>>> them.
>>>>
>>>> So, instead of doing this manually (always error-prone!), like you've...
>>>>
>>>>> Committed to the trunk,
>>>>
>>>>> commit f75b237254f32d5be32c9d9610983b777abea633
>>>>> Author: Jeff Law <jeffreyalaw@gmail.com>
>>>>> Date:   Sun Sep 19 13:31:32 2021 -0400
>>>>>
>>>>>     [committed] Make test names unique for a couple of goacc tests
>>>>
>>>>> --- a/gcc/testsuite/gfortran.dg/goacc/privatization-1-compute.f90
>>>>> +++ b/gcc/testsuite/gfortran.dg/goacc/privatization-1-compute.f90
>>>>> @@ -39,9 +39,9 @@ contains
>>>>>            !$acc atomic write ! ... to force 'TREE_ADDRESSABLE'.
>>>>>            y = a
>>>>>      !$acc end parallel
>>>>> -    ! { dg-note {variable 'i' in 'private' clause potentially has improper OpenACC privatization level: 'parm_decl'} "TODO" { xfail *-*-* } l_compute$c_compute }
>>>>> -    ! { dg-note {variable 'j' in 'private' clause potentially has improper OpenACC privatization level: 'parm_decl'} "TODO" { xfail *-*-* } l_compute$c_compute }
>>>>> -    ! { dg-note {variable 'a' in 'private' clause potentially has improper OpenACC privatization level: 'parm_decl'} "TODO" { xfail *-*-* } l_compute$c_compute }
>>>>> +    ! { dg-note {variable 'i' in 'private' clause potentially has improper OpenACC privatization level: 'parm_decl'} "TODO2" { xfail *-*-* } l_compute$c_compute }
>>>>> +    ! { dg-note {variable 'j' in 'private' clause potentially has improper OpenACC privatization level: 'parm_decl'} "TODO3" { xfail *-*-* } l_compute$c_compute }
>>>>> +    ! { dg-note {variable 'a' in 'private' clause potentially has improper OpenACC privatization level: 'parm_decl'} "TODO4" { xfail *-*-* } l_compute$c_compute }
>>>>
>>>> ... etc. (also similarly in a handful of earlier commits, if I remember
>>>> correctly), why don't we do that programmatically, like in the attached
>>>> "Make sure that we get unique test names if several DejaGnu directives
>>>> refer to the same line", once and for all?  OK to push after proper
>>>> testing?
>>>
>>> Attached again, for easy reference.
>>>
>>> I figure it may help if I showed an example of how this changes things;
>>> for the test case cited above (word-diff):
>>>
>>>     PASS: gfortran.dg/goacc/privatization-1-compute.f90   -O   {+at line 40+} (test for warnings, line 39)
>>>     PASS: gfortran.dg/goacc/privatization-1-compute.f90   -O   {+at line 41+} (test for warnings, line 22)
>>>     PASS: gfortran.dg/goacc/privatization-1-compute.f90   -O   {+at line 42+} (test for warnings, line 39)
>>>     PASS: gfortran.dg/goacc/privatization-1-compute.f90   -O   {+at line 43+} (test for warnings, line 22)
>>>     PASS: gfortran.dg/goacc/privatization-1-compute.f90   -O   {+at line 44+} (test for warnings, line 39)
>>>     PASS: gfortran.dg/goacc/privatization-1-compute.f90   -O   {+at line 45+} (test for warnings, line 22)
>>>     XFAIL: gfortran.dg/goacc/privatization-1-compute.f90   -O  TODO2 {+at line 50+} (test for warnings, line 29)
>>>     XFAIL: gfortran.dg/goacc/privatization-1-compute.f90   -O  TODO3 {+at line 51+} (test for warnings, line 29)
>>>     XFAIL: gfortran.dg/goacc/privatization-1-compute.f90   -O  TODO4 {+at line 52+} (test for warnings, line 29)
>>>     PASS: gfortran.dg/goacc/privatization-1-compute.f90   -O  TODO {+at line 53+} (test for warnings, line 29)
>>>     PASS: gfortran.dg/goacc/privatization-1-compute.f90   -O   {+at line 54+} (test for warnings, line 29)
>>>     PASS: gfortran.dg/goacc/privatization-1-compute.f90   -O  (test for excess errors)
>>>
>>> Given that we now amend the 'comment' by 'at line $useline"', and given
>>> that only ever one DejaGnu directive may appear on one source line, all
>>> these output lines now must be unique.  (If we wanted to, we again could
>>> 's%TODO[0-9]+%TODO%', reverting your change cited above.)
>>>
>>>
>>> Grüße
>>>  Thomas


-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
diff mbox series

Patch

From 347cce092ebd954d91046804c1d2b51b24eef68b Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <thomas@codesourcery.com>
Date: Wed, 22 Sep 2021 12:42:41 +0200
Subject: [PATCH] Make sure that we get unique test names if several DejaGnu
 directives refer to the same line [PR102735]

	gcc/testsuite/
	PR testsuite/102735
	* lib/gcc-dg.exp (process-message): Make sure that we get unique
	test names.
---
 gcc/testsuite/lib/gcc-dg.exp | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/gcc/testsuite/lib/gcc-dg.exp b/gcc/testsuite/lib/gcc-dg.exp
index 7edd070d71d..78a6c3651ef 100644
--- a/gcc/testsuite/lib/gcc-dg.exp
+++ b/gcc/testsuite/lib/gcc-dg.exp
@@ -1191,8 +1191,18 @@  proc process-message { msgproc msgprefix dgargs } {
     upvar dg-messages dg-messages
 
     if { [llength $dgargs] == 5 } {
-	set num [get-absolute-line [lindex $dgargs 0] [lindex $dgargs 4]]
-	set dgargs [lreplace $dgargs 4 4 $num]
+	set useline [lindex $dgargs 0]
+
+	# Resolve absolute line number.
+	set line [get-absolute-line $useline [lindex $dgargs 4]]
+	set dgargs [lreplace $dgargs 4 4 $line]
+
+	if { $line != $useline } {
+	    # Make sure that we get unique test names if different USELINEs
+	    # refer to the same LINE.
+	    set comment "[lindex $dgargs 2] at line $useline"
+	    set dgargs [lreplace $dgargs 2 2 $comment]
+	}
     }
 
     # Process the dg- directive, including adding the regular expression
-- 
2.33.0