Re: Failure of expensive test "datamash-valgrind.sh" for git HEAD

[Erik Auerswald]

Hi,

how about the following test adjustments?

------------ 8< ------------
diff --git a/tests/datamash-valgrind.sh b/tests/datamash-valgrind.sh
index 6bb74af..d8a1f8f 100755
--- a/tests/datamash-valgrind.sh
+++ b/tests/datamash-valgrind.sh
@@ -188,9 +188,11 @@ cmp wide wide_orig ||
     fail=1 ; }

 ## Test large output formats
+## datamash exits with an error on x86, because some numbers in the 
file "wide"
+## are too big for "long double" 80-bit floating point
 cat wide | valgrind --track-origins=yes  --leak-check=full \
-                  --show-reachable=yes  --error-exitcode=1 \
+                  --show-reachable=yes  --error-exitcode=2 \
                   datamash --format "%05000.5000f" sum 1 > /dev/null ||
-  { warn_ "custom-format failed" ; fail=1 ; }
+  { test $? -eq 2 && { warn_ "custom-format failed" ; fail=1 ; } ; }

 Exit $fail
------------ >8 ------------

I can commit and push this (without Thunderbird's whitespace damage)
if it is OK.

Thanks,
Erik


On 01.06.22 13:28, Shawn Wagner wrote:
> Yeah, verified it hasn't passed for as long as it's existed (since 1.3).
>
> On Wed, Jun 1, 2022 at 2:42 AM Shawn Wagner <shawnw.mobile@gmail.com> wrote:
>
> > datamash-output-format.pl already has test cases with printing more
> > reasonable (And unreasonable) numbers, and tests/datamash-tests.pl and
> > datamash-tests-2.pl have some for input numbers. I don't think we
> > necessarily need to add more to the valgrind version (But maybe to the
> > normal tests?)
> >
> > Looking into this particular failing test some more, according to git
> > blame, it was added years after the relevant error checking code; I don't
> > think it could ever have passed as is.
> >
> > On Wed, Jun 1, 2022 at 12:58 AM Erik Auerswald <auerswal@unix-ag.uni-kl.de>
> > wrote:
> >
> > > Hi,
> > >
> > > On Wed, Jun 01, 2022 at 12:44:01AM -0700, Shawn Wagner wrote:
> > > > It's not /crashing/ in the test, it's exiting with an error. I suspect
> > > the
> > > > idea of this particular test is to look for buffer overflows with really
> > > > long input strings, and it should be using valgrind's --error-exitcode
> > > > option and checking for that instead of any failure status.
> > >
> > > We could create two tests from this, one with valid input for x86 80 bit
> > > extended precision floating point, the other with a different exit code
> > > for valgrind errors to accept a datamash error exit as well as datamash
> > > succeeding (the input could work on ARM64 and some PPC64 systems).
> > >
> > > > (Fixed all the small memory leaks I found with -fsanitize=address in the
> > > > normal test scripts, btw)
> > >
> > > Thanks!
> > >
> > > Cheers,
> > > Erik
> > >
> > > > On Tue, May 31, 2022 at 2:23 PM Tim Rice <trice@posteo.net> wrote:
> > > >
> > > > > Hey Erik,
> > > > >
> > > > > > the "expensive" test "datamash-valgrind.sh" fails on my Ubuntu 18.04
> > > > > > GNU/Linux x86-64 laptop with current git HEAD:
> > > > > >
> > > > > > ```
> > > > > > $ make check-expensive TESTS=tests/datamash-valgrind.sh
> > > > > > [...]
> > > > > > custom-format failed
> > > > > > FAIL: tests/datamash-valgrind.sh
> > > > > > [...]
> > > > > > ```
> > > > >
> > > > > Yeah, I was looking at this too. I want to make sure all expensive
> > > tests
> > > > > pass before opening 1.8 up for testing. This is not the only
> > > expensive test
> > > > > that fails for me, btw. The one for doing i/o on a full filesystem is
> > > also
> > > > > a bit wonky.
> > > > >
> > > > > I thought the --format test problem could be because of something
> > > weird on
> > > > > my local machine. The test was added at the same time as the --format
> > > flag.
> > > > > I figured, surely it must have been working when it was added?
> > > > >
> > > > > And if datamash is intended to fail for this test, then what is the
> > > point
> > > > > of including inputs that don't cause it to crash? It would be clearer
> > > to go
> > > > > straight to the input which is too big.
> > > > >
> > > > > Because it doesn't do that, I think datamash is intended to succeed.
> > > The
> > > > > conclusion is that the test inputs need to be pruned down.
> > > > >
> > > > > ~ Tim