valgrind-developers

[Valgrind-developers] extending vg_regtest to also produce automake-flavoured result files

[Frank C. E. <fc...@re...>]

Hi -

I'd like to help make valgrind test results more archivable and
machine-readable, so that we can automate builds & testing better.  We
operate a system called bunsen over on sourceware.org (= valgrind.org)
which supports ingesting result text files after a "make check" (or
whatever).  However, valgrind's vg_regtest doesn't leave apprx. any of
that around when it runs.

So I'd like to propose to change vg_regtest so that, in addition to
the things you're used to it doing, it ALSO create a collection of log
files that impersonate the automake-flavoured testsuite.  Namely: one
'.trs' and one '.log' file per vgtest case, and one test-suite.log per
vgtest directory (all 58ish of them?).  bunsen can already read &
parse this type of output.

The '.log' file is customarily free-form, so I'm thinking of letting
it contain the actual stdout and stderr streams, plus any judgements /
diffs from the .exp expected files.  The '.trs' is the main
machine-readable file that has the pass/fail judgement recorded.

What do you think?  Any concerns if this behaviour were made a quiet
default?

- FChE

Re: [Valgrind-developers] extending vg_regtest to also produce automake-flavoured result files

[Mark W. <ma...@kl...>]

Hi Frank,

On Tue, 2024-07-16 at 12:58 -0400, Frank Ch. Eigler wrote:
> I'd like to help make valgrind test results more archivable and
> machine-readable, so that we can automate builds & testing better.  We
> operate a system called bunsen over on sourceware.org (= valgrind.org)
> which supports ingesting result text files after a "make check" (or
> whatever).  However, valgrind's vg_regtest doesn't leave apprx. any of
> that around when it runs.

For reference the bunsen project is documented here:
https://sourceware.org/bunsen/
And an instance collecting test results from various sourceware
projects can be queried here:
https://builder.sourceware.org/testruns/

> So I'd like to propose to change vg_regtest so that, in addition to
> the things you're used to it doing, it ALSO create a collection of log
> files that impersonate the automake-flavoured testsuite.  Namely: one
> '.trs' and one '.log' file per vgtest case, and one test-suite.log per
> vgtest directory (all 58ish of them?).  bunsen can already read &
> parse this type of output.

That sounds nice. Having a (standard) machine readable/parsable log of
the vgtest runs would be really useful even without bunsen. Currently I
am often just looking with find . -name \*std\*diff to find out which
tests failed.

I would create just one test-suite.log file per vgtest run though (in
the dir where vgtest is being run) that should include all the tests it
is being asked to run.

In practice it is invoked from the top-level builddir as either
make regtest:
@PERL@ tests/vg_regtest gdbserver_tests $(TOOLS) $(EXP_TOOLS)
or make nonexp-regtest:
@PERL@ tests/vg_regtest $(TOOLS)
Or by hand to test a subset. e.g.
perl tests/vg_regtest none/tests/amd64-linux

In each case I would expect just one test-suite.log file with the
subset of tests that were being run. Just like in case you run a "real"
automake testsuite with make check TESTS="xxx yyy".

> The '.log' file is customarily free-form, so I'm thinking of letting
> it contain the actual stdout and stderr streams, plus any judgements /
> diffs from the .exp expected files.  The '.trs' is the main
> machine-readable file that has the pass/fail judgement recorded.
> 
> What do you think?  Any concerns if this behaviour were made a quiet
> default?

Sounds good to me. If it comes with some make clean and .gitignore
additions.

Cheers,

Mark

Re: [Valgrind-developers] extending vg_regtest to also produce automake-flavoured result files

[Frank C. E. <fc...@re...>]

Hi -

> [...]
> That sounds nice. Having a (standard) machine readable/parsable log of
> the vgtest runs would be really useful even without bunsen. Currently I
> am often just looking with find . -name \*std\*diff to find out which
> tests failed.

OK, I have a functional proposal over on the valgrind.git repo
users/fche/try-vgregtest branch.

https://sourceware.org/git/?p=valgrind.git;a=shortlog;h=refs/heads/users/fche/try-vgregtest

It creates individual .log & baby .trs files for each subtest, e.g.:

./callgrind/tests/simwork2.vgtest.log:

Fri Jul 19 12:51:33 2024 executing  VALGRIND_LIB=/home/fche/Private/DEVEL/DEVEL-sourceware/valgrind/.in_place VALGRIND_LIB_INNER=/home/fche/Private/DEVEL/DEVEL-sourceware/valgrind/.in_place /home/fche/Private/DEVEL/DEVEL-sourceware/valgrind/./coregrind/valgrind --command-line-only=yes --memcheck:leak-check=no --tool=callgrind   --simulate-wb=yes --simulate-hwpref=yes ./simwork  > simwork2.stdout.out 2> simwork2.stderr.out
Fri Jul 19 12:51:33 2024 rc 256
Fri Jul 19 12:51:33 2024 stdout:
Sum: 1000000
Fri Jul 19 12:51:33 2024 stderr:
==3216305== Callgrind, a call-graph generating cache profiler
==3216305== Copyright (C) 2002-2017, and GNU GPL'd, by Josef Weidendorfer et al.
==3216305== Using Valgrind-3.23.0.GIT and LibVEX; rerun with -h for copyright info
==3216305== Command: ./simwork
==3216305== 
--3216305-- warning: L3 cache found, using its data for the LL simulation.
==3216305== For interactive control, run 'callgrind_control -h'.
==3216305== 
==3216305== Events    : Ir Dr Dw I1mr D1mr D1mw ILmr DLmr DLmw ILdmr DLdmr DLdmw
==3216305== Collected : 18506076 7701686 1901122 320 125164 75061 289 63476 37521 1 31728 10594
==3216305== 
==3216305== I   refs:      18,506,076
==3216305== I1  misses:           320
==3216305== LLi misses:           289
==3216305== I1  miss rate:       0.00%
==3216305== LLi miss rate:       0.00%
==3216305== 
==3216305== D   refs:       9,602,808  (7,701,686 rd + 1,901,122 wr)
==3216305== D1  misses:       200,225  (  125,164 rd +    75,061 wr)
==3216305== LLd misses:       100,997  (   63,476 rd +    37,521 wr)
==3216305== D1  miss rate:        2.1% (      1.6%   +       3.9%  )
==3216305== LLd miss rate:        1.1% (      0.8%   +       2.0%  )
==3216305== 
==3216305== LL refs:          200,545  (  125,484 rd +    75,061 wr)
==3216305== LL misses:        101,286  (   63,765 rd +    37,521 wr)
==3216305== LL miss rate:         0.4% (      0.2%   +       2.0%  )
Fri Jul 19 12:51:33 2024 executing diff -u simwork2.stdout.exp simwork2.stdout.out > simwork2.stdout.diff
Fri Jul 19 12:51:33 2024 rc 0
Fri Jul 19 12:51:33 2024 executing ./filter_stderr simwork2 < simwork2.stderr.out > vg_regtest.tmp.3190086
Fri Jul 19 12:51:33 2024 rc 0
Fri Jul 19 12:51:33 2024 executing diff -u simwork2.stderr.exp simwork2.stderr.out > simwork2.stderr.diff
Fri Jul 19 12:51:33 2024 rc 0
Fri Jul 19 12:51:33 2024 executing rm callgrind.out.*
Fri Jul 19 12:51:33 2024 rc 0

./callgrind/tests/simwork2.vgtest.trs:

:test-result: PASS
:global-test-result: PASS
:recheck: no
:copy-in-global-log: no


> I would create just one test-suite.log file per vgtest run though (in
> the dir where vgtest is being run) that should include all the tests it
> is being asked to run.
> In each case I would expect just one test-suite.log file with the
> subset of tests that were being run. Just like in case you run a "real"
> automake testsuite with make check TESTS="xxx yyy".

Unfortunately, that's not quite how bunsen expects automake tests to
be laid out.  There needs to be -a- test-suite.log file in each
subdirectory where .trs/.log files come to be.  So the new code puts
an empty one in each spot, plus a "tests-suite-overall.log" file
functioning as an index:

[...]
Fri Jul 19 13:00:08 2024 processing memcheck/tests/amd64/more_x87_fp.vgtest
Fri Jul 19 13:00:08 2024 result: PASS memcheck/tests/amd64/more_x87_fp.vgtest.log
Fri Jul 19 13:00:08 2024 processing memcheck/tests/amd64/pcmpgt.vgtest
Fri Jul 19 13:00:09 2024 result: PASS memcheck/tests/amd64/pcmpgt.vgtest.log
[...]
Fri Jul 19 13:01:15 2024 processing memcheck/tests/origin6-fp.vgtest
Fri Jul 19 13:01:16 2024 result: PASS memcheck/tests/origin6-fp.vgtest.log
Fri Jul 19 13:01:16 2024 processing memcheck/tests/overlap.vgtest
Fri Jul 19 13:01:16 2024 result: SKIP memcheck/tests/overlap.vgtest.log
[...]


> Sounds good to me. If it comes with some make clean and .gitignore
> additions.

Yup, included.  Tested with a local copy of bunsen, and it accepts all
this data (plus auxcheck if run) perfectly.

- FChE

Re: [Valgrind-developers] extending vg_regtest to also produce automake-flavoured result files

[Mark W. <ma...@kl...>]

Hi Frank,

On Wed, 2024-08-21 at 18:43 -0400, Frank Ch. Eigler wrote:
> > One last request. Could you add a little paragraph under "Running the
> > regression tests" in README_DEVELOPERS about the test-suite*.log and
> > *.vgtest.[trs|log] files?
> 
> Done, it says:
> 
> +The details of each vgtest run are logged to individual "vgtest.log"
> +files.  These are listed, and non-passing tests detailed, in the
> +test-suite-overall.log file.  (Token *.trs and test-suite.log files
> +are also created, for emulating automake-style testsuites, as expected
> +by tools such as bunsen.)
> 
> Code in the users/fche/try-vgregtest branch updated.  Thanks for the review!

Thanks, I think the patch is OK to land now.

Cheers,

Mark

Re: [Valgrind-developers] extending vg_regtest to also produce automake-flavoured result files

[Frank C. E. <fc...@re...>]

Hi -

> Thanks, I think the patch is OK to land now.

Thanks a lot!  Pushed to master.

BTW, I tried to push a gpg-signed git commit, but the githooks
erroneously rejected that with.  I think there's a git-show option
that it should be using to suppress signature messages when checking
the commit text.

remote: *** Invalid revision history for commit 91115a0195e440d0f4b10c30d223827b05ea99d9:
remote: *** The first line should be the subject of the commit,
remote: *** followed by an empty line.
remote: *** 
remote: *** Below are the first few lines of the revision history:
remote: *** | gpg: Signature made Thu Aug 22 15:19:12 2024 GMT
remote: *** | gpg:                using RSA key 4DD136490411C0A42B28844F258B6EFA0F209D24
remote: *** | gpg: Good signature from "Frank Ch. Eigler <fc...@el...>" [unknown]
remote: *** | gpg: WARNING: This key is not certified with a trusted signature!
remote: *** | gpg:          There is no indication that the signature belongs to the owner.
remote: *** 
remote: *** Please amend the commit's revision history and try again.
remote: error: hook declined to update refs/heads/master
To ssh://sourceware.org/git/valgrind.git

- FChE

Re: [Valgrind-developers] extending vg_regtest to also produce automake-flavoured result files

[Mark W. <ma...@kl...>]

Hi Frank,

On Fri, Jul 19, 2024 at 01:05:15PM -0400, Frank Ch. Eigler wrote:
> > That sounds nice. Having a (standard) machine readable/parsable log of
> > the vgtest runs would be really useful even without bunsen. Currently I
> > am often just looking with find . -name \*std\*diff to find out which
> > tests failed.
> 
> OK, I have a functional proposal over on the valgrind.git repo
> users/fche/try-vgregtest branch.
> 
> https://sourceware.org/git/?p=valgrind.git;a=shortlog;h=refs/heads/users/fche/try-vgregtest
> 
> It creates individual .log & baby .trs files for each subtest, e.g.:
> 
> ./callgrind/tests/simwork2.vgtest.log:
> 
> Fri Jul 19 12:51:33 2024 executing  VALGRIND_LIB=/home/fche/Private/DEVEL/DEVEL-sourceware/valgrind/.in_place VALGRIND_LIB_INNER=/home/fche/Private/DEVEL/DEVEL-sourceware/valgrind/.in_place /home/fche/Private/DEVEL/DEVEL-sourceware/valgrind/./coregrind/valgrind --command-line-only=yes --memcheck:leak-check=no --tool=callgrind   --simulate-wb=yes --simulate-hwpref=yes ./simwork  > simwork2.stdout.out 2> simwork2.stderr.out
> Fri Jul 19 12:51:33 2024 rc 256
> Fri Jul 19 12:51:33 2024 stdout:
> Sum: 1000000
> Fri Jul 19 12:51:33 2024 stderr:
> ==3216305== Callgrind, a call-graph generating cache profiler
> ==3216305== Copyright (C) 2002-2017, and GNU GPL'd, by Josef Weidendorfer et al.
> ==3216305== Using Valgrind-3.23.0.GIT and LibVEX; rerun with -h for copyright info
> ==3216305== Command: ./simwork
> ==3216305== 
> --3216305-- warning: L3 cache found, using its data for the LL simulation.
> ==3216305== For interactive control, run 'callgrind_control -h'.
> ==3216305== 
> ==3216305== Events    : Ir Dr Dw I1mr D1mr D1mw ILmr DLmr DLmw ILdmr DLdmr DLdmw
> ==3216305== Collected : 18506076 7701686 1901122 320 125164 75061 289 63476 37521 1 31728 10594
> ==3216305== 
> ==3216305== I   refs:      18,506,076
> ==3216305== I1  misses:           320
> ==3216305== LLi misses:           289
> ==3216305== I1  miss rate:       0.00%
> ==3216305== LLi miss rate:       0.00%
> ==3216305== 
> ==3216305== D   refs:       9,602,808  (7,701,686 rd + 1,901,122 wr)
> ==3216305== D1  misses:       200,225  (  125,164 rd +    75,061 wr)
> ==3216305== LLd misses:       100,997  (   63,476 rd +    37,521 wr)
> ==3216305== D1  miss rate:        2.1% (      1.6%   +       3.9%  )
> ==3216305== LLd miss rate:        1.1% (      0.8%   +       2.0%  )
> ==3216305== 
> ==3216305== LL refs:          200,545  (  125,484 rd +    75,061 wr)
> ==3216305== LL misses:        101,286  (   63,765 rd +    37,521 wr)
> ==3216305== LL miss rate:         0.4% (      0.2%   +       2.0%  )
> Fri Jul 19 12:51:33 2024 executing diff -u simwork2.stdout.exp simwork2.stdout.out > simwork2.stdout.diff
> Fri Jul 19 12:51:33 2024 rc 0
> Fri Jul 19 12:51:33 2024 executing ./filter_stderr simwork2 < simwork2.stderr.out > vg_regtest.tmp.3190086
> Fri Jul 19 12:51:33 2024 rc 0
> Fri Jul 19 12:51:33 2024 executing diff -u simwork2.stderr.exp simwork2.stderr.out > simwork2.stderr.diff
> Fri Jul 19 12:51:33 2024 rc 0
> Fri Jul 19 12:51:33 2024 executing rm callgrind.out.*
> Fri Jul 19 12:51:33 2024 rc 0
> 
> ./callgrind/tests/simwork2.vgtest.trs:
> 
> :test-result: PASS
> :global-test-result: PASS
> :recheck: no
> :copy-in-global-log: no

This is useful and I saw you already got the bunsen instance on
sourceware to ingest these.

BTW. Some of the discrepancies I noticed on irc seem to come from the
fact that your branch is based on somewhat old trunk code from
February. You might want to rebase it.

> > I would create just one test-suite.log file per vgtest run though (in
> > the dir where vgtest is being run) that should include all the tests it
> > is being asked to run.
> > In each case I would expect just one test-suite.log file with the
> > subset of tests that were being run. Just like in case you run a "real"
> > automake testsuite with make check TESTS="xxx yyy".
> 
> Unfortunately, that's not quite how bunsen expects automake tests to
> be laid out.  There needs to be -a- test-suite.log file in each
> subdirectory where .trs/.log files come to be.

Aha, ok. So the test-suite.log file is just a marker for where the
(machine readable) .log and .trs files are.

> So the new code puts
> an empty one in each spot, plus a "tests-suite-overall.log" file
> functioning as an index:
> 
> [...]
> Fri Jul 19 13:00:08 2024 processing memcheck/tests/amd64/more_x87_fp.vgtest
> Fri Jul 19 13:00:08 2024 result: PASS memcheck/tests/amd64/more_x87_fp.vgtest.log
> Fri Jul 19 13:00:08 2024 processing memcheck/tests/amd64/pcmpgt.vgtest
> Fri Jul 19 13:00:09 2024 result: PASS memcheck/tests/amd64/pcmpgt.vgtest.log
> [...]
> Fri Jul 19 13:01:15 2024 processing memcheck/tests/origin6-fp.vgtest
> Fri Jul 19 13:01:16 2024 result: PASS memcheck/tests/origin6-fp.vgtest.log
> Fri Jul 19 13:01:16 2024 processing memcheck/tests/overlap.vgtest
> Fri Jul 19 13:01:16 2024 result: SKIP memcheck/tests/overlap.vgtest.log
> [...]

Sorry for changing my mind, but if we are going to have test-suite.log
files in every directory anyway, then I would prefer them to not be
empty. Also if possible I would prefer to have the format be like the
automake style test-suite.log output, which I find more human readable
and useful. The files aren't parsed by bunsen are they? And we have
the .log files if we need the timestamped full output.

So then the test-suite.log in each dir would just contain the summary
of test results plus any logs that tell why some tests FAILed or were
SKIPed.

If that isn't too much work of course. But I don't think the empty
files are super useful. And the time-stammed processing
tests-suite-overall.log isn't as useful since it doesn't really give a
quick human-readable overview like a "normal" test-suite.log file
does.

Thanks,

Mark

Re: [Valgrind-developers] extending vg_regtest to also produce automake-flavoured result files

[Frank C. E. <fc...@re...>]

Hi -


> Sorry for changing my mind, but if we are going to have test-suite.log
> files in every directory anyway, then I would prefer them to not be
> empty. [...]

Are you sure?  There would be rather a lot of them in valgrind (21).

> And the time-stammed processing tests-suite-overall.log isn't as
> useful since it doesn't really give a quick human-readable overview
> like a "normal" test-suite.log file does. [...]

OK, can try: basically an overall test state count, and a
transcription of the FAIL- and SKIP-state subtest .log files?

- FChE

Re: [Valgrind-developers] extending vg_regtest to also produce automake-flavoured result files

[Mark W. <ma...@kl...>]

Hi Frank,

On Sat, Jul 20, 2024 at 08:38:48PM -0400, Frank Ch. Eigler wrote:
> > Sorry for changing my mind, but if we are going to have test-suite.log
> > files in every directory anyway, then I would prefer them to not be
> > empty. [...]
> 
> Are you sure?  There would be rather a lot of them in valgrind (21).

It would be better imho than having 21 empty files. But maybe others
have a different opinion?

> > And the time-stammed processing tests-suite-overall.log isn't as
> > useful since it doesn't really give a quick human-readable overview
> > like a "normal" test-suite.log file does. [...]
> 
> OK, can try: basically an overall test state count, and a
> transcription of the FAIL- and SKIP-state subtest .log files?

Yes, if that is easy to generate.

Thanks,

Mark

Re: [Valgrind-developers] extending vg_regtest to also produce automake-flavoured result files

[Frank C. E. <fc...@re...>]

Hi -

> [...]
> > OK, can try: basically an overall test state count, and a
> > transcription of the FAIL- and SKIP-state subtest .log files?
> 
> Yes, if that is easy to generate.

OK, see what you think about this recent buildbot try-run.  It still
leaves the 21 test-suite.log files empty (you don't ever have to look
at them), but the single central test-suite-overall.log file now has
all the non-PASS .log files transcribed into it.

https://builder.sourceware.org/testrun/e208899f958817d1c15e09f3544c5c6972210190?focus=filelist&sortColumn=0&sortDirection=desc

https://builder.sourceware.org/testrun/e208899f958817d1c15e09f3544c5c6972210190?filename=test-suite-overall.log

Code is in the users/fche/try-vgregtest branch.

- FChE

Re: [Valgrind-developers] extending vg_regtest to also produce automake-flavoured result files

[Mark W. <ma...@kl...>]

On Thu, 2024-08-08 at 14:26 -0400, Frank Ch. Eigler wrote:
> Hi -
> 
> > [...]
> > > OK, can try: basically an overall test state count, and a
> > > transcription of the FAIL- and SKIP-state subtest .log files?
> > 
> > Yes, if that is easy to generate.
> 
> OK, see what you think about this recent buildbot try-run.  It still
> leaves the 21 test-suite.log files empty (you don't ever have to look
> at them)

You don't have to look at them, but they are there. So I am sure people
will look at them and wonder what/why they are empty. I think it would
be good if they weren't totally empty but contained at least some hint
of where to look for the "real" results. Maybe just some static text
telling the user to look for the top-level test-suite-overall.log
and/or the .log/trs files in the current directory?

> , but the single central test-suite-overall.log file now has
> all the non-PASS .log files transcribed into it.
> 
> https://builder.sourceware.org/testrun/e208899f958817d1c15e09f3544c5c6972210190?focus=filelist&sortColumn=0&sortDirection=desc
> 
> https://builder.sourceware.org/testrun/e208899f958817d1c15e09f3544c5c6972210190?filename=test-suite-overall.log

Very nice.

> Code is in the users/fche/try-vgregtest branch.

There are 4 commits on that branch. I assume you want to squash those?

Some random comments:

> diff --git a/.gitignore b/.gitignore
> index 08598edbff70..664b37020f38 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -2445,3 +2445,7 @@
>  
>  # /VEX/switchback/
>  /VEX/switchback/switchback
> +
> +*.vgtest*.trs
> +*.vgtest*.log
> +test-suite*.log
> 

Might be better to be precise and add test-suite.log and /test-suite-
overall.log separately?

+/test-suite-overall.log
+test-suite.log

> diff --git a/Makefile.am b/Makefile.am
> index 7749442e9f6a..bb2e90335bf8 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -137,6 +137,10 @@ all-local: default.supp include/vgversion.h
>  
>  clean-local:
>         rm -rf $(inplacedir) include/vgversion.h
> +       find . -name '*.vgtest.log' -print0 | xargs -r -0 rm
> +       find . -name '*.vgtest.trs' -print0 | xargs -r -0 rm
> +       find . -name test-suite.log -print0 | xargs -r -0 rm
> +       find . -name test-suite-overall.log -print0 | xargs -r -0 rm
>  
>  # Need config.h in the installed tree, since some files depend on it
>  pkginclude_HEADERS = config.h
> 

Kind of same question here. Why search for the test-suite-overall.log?
Don't we know exactly where it is? Just remove it directly?

> -sub mysystem($) 
> +sub mysystem($@) 
                   ^ extra whitespace.

>  {
> -    my $exit_code = system($_[0]);
> +    my $tslog = shift(@_);
> +    print $tslog scalar localtime, " executing @_\n";
> +    my $exit_code = system(@_);
> +    print $tslog scalar localtime, " rc $exit_code\n";
>      ($exit_code == 2) and exit 1;      # 2 is SIGINT
>      return $exit_code;
>  } 

I don't know precisely of tslog works, it obviously adds a timestamp.
But in this case it also indents, but before instead of after the
timestamp, which seems odd, it results in the following:

Thu Aug  8 13:40:25 2024 result: SKIP gdbserver_tests/nlself_invalidate.vgtest.log
	Thu Aug  8 13:40:25 2024 executing /bin/sh -c test -e gdb && ( ../tests/arch_test amd64 || ../tests/arch_test mips32 || ../tests/arch_test mips64 )
	Thu Aug  8 13:40:25 2024 rc 256

I understand why you want to indent, but would it be possible to indent
after the timestamp?

> +    my $rc = "PASS";
> +    # $vgtest.trs is opened by caller, so as to absorb $rc reliably, even from early returns
> +    open(VGTESTLOG,">","$vgtest.log") or die $!;    
                                                   ^ extra whitespace
> +    
    ^ extra whitespace

>      if (defined $prereq) {
> -        my $prereq_res = system("/bin/sh", "-c", $prereq);
> +        my $prereq_res = mysystem(*VGTESTLOG, "/bin/sh", "-c", $prereq);
>          if (0 == $prereq_res) {
>              # Do nothing (ie. continue with the test)
>          } elsif (256 == $prereq_res) {
>              # Nb: weird Perl-ism -- exit code of '1' is seen by Perl as 256...
>              # Prereq failed, skip.
>              printf("%-16s (skipping, prereq failed: $prereq)\n", "$name:");
> -            return;
> +            print VGTESTLOG scalar localtime, " skipping, prereq failed";
							  ^ missing
\n.
> +            close(VGTESTLOG);
> +            return "SKIP";
>          } else {
> 

Same as above, this creates indenting before instead of after the
timestamp. Combined with the missing \n it results in:

Thu Aug  8 13:40:25 2024 result: SKIP gdbserver_tests/nlself_invalidate.vgtest.log
	Thu Aug  8 13:40:25 2024 executing /bin/sh -c test -e gdb && ( ../tests/arch_test amd64 || ../tests/arch_test mips32 || ../tests/arch_test mips64 )
	Thu Aug  8 13:40:25 2024 rc 256
	Thu Aug  8 13:40:25 2024 skipping, prereq failedThu Aug  8 13:40:25 2024 processing gdbserver_tests/nlsigvgdb.vgtest

So all my comments are really whitespace comments :)

Thanks,

Mark

Re: [Valgrind-developers] extending vg_regtest to also produce automake-flavoured result files

[Frank C. E. <fc...@re...>]

Hi -

> You don't have to look at them, but they are there. So I am sure people
> will look at them and wonder what/why they are empty. I think it would
> be good if they weren't totally empty but contained at least some hint
> of where to look for the "real" results. [...]

Good idea, added.

> > Code is in the users/fche/try-vgregtest branch.
> There are 4 commits on that branch. I assume you want to squash those?

Yes, done now.

> Might be better to be precise and add test-suite.log and /test-suite-
> overall.log separately?
> 
> +/test-suite-overall.log
> +test-suite.log

Sure, done.

> [...]
> Kind of same question here. Why search for the test-suite-overall.log?
> Don't we know exactly where it is? Just remove it directly?

Sure, done.

> > -sub mysystem($) 
> > +sub mysystem($@) 
>                    ^ extra whitespace.

OK, trimmed off all line-ending whitespace throughout file.


> >  {
> > -    my $exit_code = system($_[0]);
> > +    my $tslog = shift(@_);
> > +    print $tslog scalar localtime, " executing @_\n";
> > +    my $exit_code = system(@_);
> > +    print $tslog scalar localtime, " rc $exit_code\n";
> >      ($exit_code == 2) and exit 1;      # 2 is SIGINT
> >      return $exit_code;
> >  } 
> 
> I don't know precisely of tslog works, it obviously adds a timestamp.
> But in this case it also indents, but before instead of after the
> timestamp, which seems odd, it results in the following:

Not quite.  What you're seeing is the indented transcription of the
non-PASSing individual subtest .log in the test-suite-overall.log
file.  New code prefixes all such lines with

| <tab>

so it's a little more clear what's what.

> [...]
> So all my comments are really whitespace comments :)

OK, final (?) testing underway on the users/fche/try-vgregtest branch.


- FChE

Re: [Valgrind-developers] extending vg_regtest to also produce automake-flavoured result files

[Paul F. <pj...@wa...>]

On 16-07-24 16:58, Frank Ch. Eigler wrote:
> Hi -
> 
> I'd like to help make valgrind test results more archivable and
> machine-readable, so that we can automate builds & testing better.  We
> operate a system called bunsen over on sourceware.org (= valgrind.org)
> which supports ingesting result text files after a "make check" (or
> whatever).  However, valgrind's vg_regtest doesn't leave apprx. any of
> that around when it runs.

Does bunsen run on any non-Linux systems?

If not then as long as it doesn't break anything I don't mind.

A+
Paul

Re: [Valgrind-developers] extending vg_regtest to also produce automake-flavoured result files

[Frank C. E. <fc...@re...>]

Hi, Paul -

> Does bunsen run on any non-Linux systems?
> If not then as long as it doesn't break anything I don't mind.

While bunsen should run fine on python3 platforms generally, it's not
really a prerequisite for this work.  Here the idea is to make the
complete testsuite output be retained in a standardish human- and
machine-readable form ... and not be in your way if you don't care. :-)

- FChE

Re: [Valgrind-developers] extending vg_regtest to also produce automake-flavoured result files

[Mark W. <ma...@kl...>]

Hi Frank,

On Wed, Aug 14, 2024 at 03:28:27PM -0400, Frank Ch. Eigler wrote:
> > You don't have to look at them, but they are there. So I am sure people
> > will look at them and wonder what/why they are empty. I think it would
> > be good if they weren't totally empty but contained at least some hint
> > of where to look for the "real" results. [...]
> 
> Good idea, added.

  See *.log files for details on each test in this directory.
  See /home/mark/src/valgrind/test-suite-overall.log

Nice. Thanks.

> > > Code is in the users/fche/try-vgregtest branch.
> > There are 4 commits on that branch. I assume you want to squash those?
> 
> Yes, done now.
>
> > Might be better to be precise and add test-suite.log and /test-suite-
> > overall.log separately?
> > 
> > +/test-suite-overall.log
> > +test-suite.log
> 
> Sure, done.

Thanks.
 
> > [...]
> > Kind of same question here. Why search for the test-suite-overall.log?
> > Don't we know exactly where it is? Just remove it directly?
> 
> Sure, done.
> 
> > > -sub mysystem($) 
> > > +sub mysystem($@) 
> >                    ^ extra whitespace.
> 
> OK, trimmed off all line-ending whitespace throughout file.

Thanks. Although I am not a fan of doing it for the whole file at the
same time as the rest of the patch (now it is slightly more difficult
to see what the real change is without using git show -w -b).

> > >  {
> > > -    my $exit_code = system($_[0]);
> > > +    my $tslog = shift(@_);
> > > +    print $tslog scalar localtime, " executing @_\n";
> > > +    my $exit_code = system(@_);
> > > +    print $tslog scalar localtime, " rc $exit_code\n";
> > >      ($exit_code == 2) and exit 1;      # 2 is SIGINT
> > >      return $exit_code;
> > >  } 
> > 
> > I don't know precisely of tslog works, it obviously adds a timestamp.
> > But in this case it also indents, but before instead of after the
> > timestamp, which seems odd, it results in the following:
> 
> Not quite.  What you're seeing is the indented transcription of the
> non-PASSing individual subtest .log in the test-suite-overall.log
> file.  New code prefixes all such lines with
> 
> | <tab>
>
> so it's a little more clear what's what.

Ah, nice. That makes things much clearer.
 
> > [...]
> > So all my comments are really whitespace comments :)
> 
> OK, final (?) testing underway on the users/fche/try-vgregtest branch.

I looked over the diff -w -b and as far as my perl goes, it looks
good. Also the output files look ok. And the .gitignore and make clean
parts seem to work as expected.

I haven't looked at the bunsen import, I assume you have and that it
looks fine?

One last request. Could you add a little paragraph under "Running the
regression tests" in README_DEVELOPERS about the test-suite*.log and
*.vgtest.[trs|log] files?

Thanks,

Mark

Re: [Valgrind-developers] extending vg_regtest to also produce automake-flavoured result files

[Frank C. E. <fc...@re...>]

Hi -

> > OK, trimmed off all line-ending whitespace throughout file.
> 
> Thanks. Although I am not a fan of doing it for the whole file at the
> same time as the rest of the patch (now it is slightly more difficult
> to see what the real change is without using git show -w -b).

Yeah, sorry about that, but then again stuff like this rarely gets done
otherwise. ;-)

> [...]
> I haven't looked at the bunsen import, I assume you have and that it
> looks fine?

Definitely (can see them on sourceware).

> One last request. Could you add a little paragraph under "Running the
> regression tests" in README_DEVELOPERS about the test-suite*.log and
> *.vgtest.[trs|log] files?

Done, it says:

+The details of each vgtest run are logged to individual "vgtest.log"
+files.  These are listed, and non-passing tests detailed, in the
+test-suite-overall.log file.  (Token *.trs and test-suite.log files
+are also created, for emulating automake-style testsuites, as expected
+by tools such as bunsen.)

Code in the users/fche/try-vgregtest branch updated.  Thanks for the review!

- FChE