public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Problems with strub tests
@ 2023-12-20  4:40 Jeff Law
  2023-12-20  6:31 ` [PATCH] compare_tests: distinguish c-c++-common results by tool Alexandre Oliva
  2024-01-07  1:51 ` Problems with strub tests Hans-Peter Nilsson
  0 siblings, 2 replies; 4+ messages in thread
From: Jeff Law @ 2023-12-20  4:40 UTC (permalink / raw)
  To: Alexandre Oliva; +Cc: gcc-patches


So the strub tests in c-c++-common are problematical.  They get run 
twice, once for C, once for C++.  Yet the name of the test is the same 
in both runs. (by the name, I mean the name emitted into the dejagnu 
summary and log files).

Thus if you have a test in there which passes in one context, but fails 
in the other, comparison tools like contrib/compare_tests may 
erroneously report the tests as both a test which now fails, but passed 
before and a test which now passes but failed before.

It looks like some of the strub tests are currently known to fail with 
C++ and are triggering this problem


Ideally we'd include the c or c++ in the test name depending on which 
context its being run within.  That would be sufficient to resolve these 
problems and avoid them in the future.  It would also be sufficient to 
get all the tests to the point where their behavior is the same for both 
languages.

Not sure if the latter is reasonably in the cards or not.  If it's not 
likely to land soon, any change you could look at the framework for 
c-c++-common and get the names unique across the two times they're run?

A third option would be to change the compare_tests tool to somehow 
distinguish between the C and C++ tests.  Not sure how feasible that is.

Thanks,
Jeff

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH] compare_tests: distinguish c-c++-common results by tool
  2023-12-20  4:40 Problems with strub tests Jeff Law
@ 2023-12-20  6:31 ` Alexandre Oliva
  2023-12-20 17:17   ` Jeff Law
  2024-01-07  1:51 ` Problems with strub tests Hans-Peter Nilsson
  1 sibling, 1 reply; 4+ messages in thread
From: Alexandre Oliva @ 2023-12-20  6:31 UTC (permalink / raw)
  To: Jeff Law; +Cc: gcc-patches

On Dec 20, 2023, Jeff Law <jeffreyalaw@gmail.com> wrote:

> So the strub tests in c-c++-common are problematical.  They get run
> twice, once for C, once for C++.  Yet the name of the test is the same 
> in both runs. (by the name, I mean the name emitted into the dejagnu
> summary and log files).

> Thus if you have a test in there which passes in one context, but
> fails in the other, comparison tools like contrib/compare_tests may 
> erroneously report the tests as both a test which now fails, but
> passed before and a test which now passes but failed before.

> It looks like some of the strub tests are currently known to fail with
> C++ and are triggering this problem

Yeah, type warnings/errors are different between C and C++, and this is
noticeable with permissible conversions between strub types.


> A third option would be to change the compare_tests tool to somehow
> distinguish between the C and C++ tests.  Not sure how feasible that
> is.

Most feasible among the possibilities ;-)

I've tested the following by comparing my obj-x86_64-linux-gnu test tree
with itself.  Ok to install?


When compare_tests compares both C and C++ tests in c-c++-common, they
get the same identifier, so expected differences in results across
languages become undesirably noisy.

This patch adds tool identifiers to tests, so that runs by different
tools are not confused by the compare logic.

It also fixes a bug in reporting differences, that would attempt to
print an undefined fname (the definitions are in subshell loops), and
adjusts the target insertion to match tabs in addition to blanks after
colons.


for  contrib/ChangeLog

	* compare_tests: Add tool to test lines.  Match tabs besides
	blanks to insert tool and target.  Don't print undefined fname.
---
 contrib/compare_tests |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/contrib/compare_tests b/contrib/compare_tests
index 2dfa8640756a0..e09fc4f113a3f 100755
--- a/contrib/compare_tests
+++ b/contrib/compare_tests
@@ -96,7 +96,7 @@ if [ -d "$1" -a -d "$2" ] ; then
 	ret=$?
 	if [ $ret -ne 0 ]; then
 		exit_status=`expr $exit_status + 1`
-		echo "## Differences found: $fname"
+		echo "## Differences found"
 	fi
 	if [ $exit_status -ne 0 ]; then
 		echo "# $exit_status differences in $cmnsums common sum files found"
@@ -108,8 +108,8 @@ elif [ -d "$1" -o -d "$2" ] ; then
 	usage "Must specify either two directories or two files"
 fi
 
-sed 's/^XFAIL/FAIL/; s/^ERROR/FAIL/; s/^XPASS/PASS/' < "$1" | awk '/^Running target / {target = $3} { if (target != "unix") { sub(/: /, "&"target": " ); }; print $0; }' | cut -c1-2000 >$tmp1
-sed 's/^XFAIL/FAIL/; s/^ERROR/FAIL/; s/^XPASS/PASS/' < "$2" | awk '/^Running target / {target = $3} { if (target != "unix") { sub(/: /, "&"target": " ); }; print $0; }' | cut -c1-2000 >$tmp2
+sed 's/^XFAIL/FAIL/; s/^ERROR/FAIL/; s/^XPASS/PASS/' < "$1" | awk '/^[	 ]*=== [^ ]* tests ===$/ {tool = $2} /^Running target / {target = $3} { if (tool != "") { sub(/:[ 	]/, "&"tool": " ); }; if (target != "unix") { sub(/:[ 	]/, "&"target": " ); }; print $0; }' | cut -c1-2000 >$tmp1
+sed 's/^XFAIL/FAIL/; s/^ERROR/FAIL/; s/^XPASS/PASS/' < "$2" | awk '/^[	 ]*=== [^ ]* tests ===$/ {tool = $2} /^Running target / {target = $3} { if (tool != "") { sub(/:[ 	]/, "&"tool": " ); }; if (target != "unix") { sub(/:[ 	]/, "&"target": " ); }; print $0; }' | cut -c1-2000 >$tmp2
 
 before=$tmp1
 now=$tmp2


-- 
Alexandre Oliva, happy hacker                    https://FSFLA.org/blogs/lxo/
   Free Software Activist                           GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice but
very few check the facts.  Think Assange & Stallman.  The empires strike back

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] compare_tests: distinguish c-c++-common results by tool
  2023-12-20  6:31 ` [PATCH] compare_tests: distinguish c-c++-common results by tool Alexandre Oliva
@ 2023-12-20 17:17   ` Jeff Law
  0 siblings, 0 replies; 4+ messages in thread
From: Jeff Law @ 2023-12-20 17:17 UTC (permalink / raw)
  To: Alexandre Oliva; +Cc: gcc-patches



On 12/19/23 23:31, Alexandre Oliva wrote:
> On Dec 20, 2023, Jeff Law <jeffreyalaw@gmail.com> wrote:
> 
>> So the strub tests in c-c++-common are problematical.  They get run
>> twice, once for C, once for C++.  Yet the name of the test is the same
>> in both runs. (by the name, I mean the name emitted into the dejagnu
>> summary and log files).
> 
>> Thus if you have a test in there which passes in one context, but
>> fails in the other, comparison tools like contrib/compare_tests may
>> erroneously report the tests as both a test which now fails, but
>> passed before and a test which now passes but failed before.
> 
>> It looks like some of the strub tests are currently known to fail with
>> C++ and are triggering this problem
> 
> Yeah, type warnings/errors are different between C and C++, and this is
> noticeable with permissible conversions between strub types.
> 
> 
>> A third option would be to change the compare_tests tool to somehow
>> distinguish between the C and C++ tests.  Not sure how feasible that
>> is.
> 
> Most feasible among the possibilities ;-)
> 
> I've tested the following by comparing my obj-x86_64-linux-gnu test tree
> with itself.  Ok to install?
> 
> 
> When compare_tests compares both C and C++ tests in c-c++-common, they
> get the same identifier, so expected differences in results across
> languages become undesirably noisy.
> 
> This patch adds tool identifiers to tests, so that runs by different
> tools are not confused by the compare logic.
> 
> It also fixes a bug in reporting differences, that would attempt to
> print an undefined fname (the definitions are in subshell loops), and
> adjusts the target insertion to match tabs in addition to blanks after
> colons.
> 
> 
> for  contrib/ChangeLog
> 
> 	* compare_tests: Add tool to test lines.  Match tabs besides
> 	blanks to insert tool and target.  Don't print undefined fname
Go for it.  It's hard to know if there'll be fallout in the consumers, 
but I think if there is pain, it should be short term.

Jeff

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Problems with strub tests
  2023-12-20  4:40 Problems with strub tests Jeff Law
  2023-12-20  6:31 ` [PATCH] compare_tests: distinguish c-c++-common results by tool Alexandre Oliva
@ 2024-01-07  1:51 ` Hans-Peter Nilsson
  1 sibling, 0 replies; 4+ messages in thread
From: Hans-Peter Nilsson @ 2024-01-07  1:51 UTC (permalink / raw)
  To: Jeff Law; +Cc: Alexandre Oliva, gcc-patches

On Tue, 19 Dec 2023, Jeff Law wrote:
> 
> So the strub tests in c-c++-common are problematical.  They get run twice,
> once for C, once for C++.  Yet the name of the test is the same in both runs.
> (by the name, I mean the name emitted into the dejagnu summary and log files).
> 
> Thus if you have a test in there which passes in one context, but fails in the
> other, comparison tools like contrib/compare_tests may erroneously report the
> tests as both a test which now fails, but passed before and a test which now
> passes but failed before.
> 
> It looks like some of the strub tests are currently known to fail with C++ and
> are triggering this problem
> 
> 
> Ideally we'd include the c or c++ in the test name depending on which context
> its being run within.  That would be sufficient to resolve these problems and
> avoid them in the future.  It would also be sufficient to get all the tests to
> the point where their behavior is the same for both languages.
> 
> Not sure if the latter is reasonably in the cards or not.  If it's not likely
> to land soon, any change you could look at the framework for c-c++-common and
> get the names unique across the two times they're run?
> 
> A third option would be to change the compare_tests tool to somehow
> distinguish between the C and C++ tests.  Not sure how feasible that is.

How about including the name of the .sum file in the key?

(They're gcc.sum and g++.sum thus different.  This is what 
contrib/regression/btest-gcc.sh does.  On the other hand, that 
prunes the name of the test at the first space.  Don't copy 
that bit. :)

Also not sure how feasible that is.

brgds, H-P

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2024-01-07  1:51 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-20  4:40 Problems with strub tests Jeff Law
2023-12-20  6:31 ` [PATCH] compare_tests: distinguish c-c++-common results by tool Alexandre Oliva
2023-12-20 17:17   ` Jeff Law
2024-01-07  1:51 ` Problems with strub tests Hans-Peter Nilsson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).