Re: [Quilt-dev] Race in test suite (faildiff.test)

[Jean Delvare]

Hallo Andreas,

Thanks for the quick answer.

On Thu, 25 Jan 2018 15:32:45 +0100, Andreas Grünbacher wrote:
> 2018-01-25 13:43 GMT+01:00 Jean Delvare <address@hidden>:
> > Hi Andreas,
> >
> > Once in a while, the quilt test suite fails with:
> >
> > [faildiff.test]
> > [1] $ mkdir patches -- ok
> > [3] $ quilt new test.diff -- ok
> > [6] $ cat > test.txt -- ok
> > [8] $ quilt add test.txt -- ok
> > [13] $ chmod -r test.txt -- ok
> > [15] $ quilt refresh -- ok
> > [18] $ echo 1 -- ok
> > [21] $ chmod +r test.txt -- ok
> > [25] $ printf "\\002\\000\\001" > test.bin -- ok
> > [26] $ quilt add test.bin -- ok
> > [29] $ printf "\\003\\000\\001" > test.bin -- ok
> > [30] $ quilt diff -pab --no-index -- failed
> > Diff failed on file 'test.bin', aborting !~ (Files|Binary files) 
> > a/test\.bin and b/test\.bin differ
> > Binary files a/test.bin and b/test.bin differ != Diff failed on file 
> > 'test.bin', aborting
> > [33] $ echo 1 -- ok
> > [36] $ quilt refresh -- ok
> > [38] $ echo 1 -- ok
> > 15 commands (14 passed, 1 failed)
> > Makefile:410: recipe for target 'test/.faildiff.ok' failed
> > make: *** [test/.faildiff.ok] Error 1
> >
> > The reason is that the 2 messages are not printed in the same order as
> > expected. Looking at the code, I can't explain how this can possibly
> > happen. "Binary files ..." is printed first in the code, and the
> > subprocess which prints it is already gone by the time we reach the
> > line which prints "Diff failed ...". It has to be, because we check its
> > status code before printing "Diff failed ...".
> >
> > The inversion is very rare, the test can succeed hundred or even
> > thousand times before if fails again. I'm using the following command
> > to reproduce the error:
> >
> > $ n=0 ; while make check-faildiff ; do let n=n+1 ; echo $n ; rm 
> > test/.faildiff.ok ; done
> >
> > Do you have any idea how this can happen? Am I missing something, or
> > could this be a bug in bash? I tried a few tricks (stdbuf, wait...) but
> > nothing seems to solve the problem. If you (or anyone reading this)
> > have any suggestion, that would be very welcome.  
> 
> The command "quilt diff -pab --no-index" writes to stdout as well as
> stderr, so it's undefined which of the two lines the run script will
> see first.

That was my first attempt at an explanation too, but I'm not longer
sure this is the problem.

> Try appending a "2>&1" to direct all output to a single file.

Something like:

--- a/test/faildiff.test
+++ b/test/faildiff.test
@@ -27,7 +27,7 @@ What happens on binary files?
        > File test.bin added to patch %{P}test.diff
 
        $ printf "\\003\\000\\001" > test.bin
-       $ quilt diff -pab --no-index
+       $ quilt diff -pab --no-index 2>&1
        >~ (Files|Binary files) a/test\.bin and b/test\.bin differ
        > Diff failed on file 'test.bin', aborting
        $ echo %{?}

I tried that already, but it doesn't help. Which doesn't really
surprise me: while the code in test/run is a bit beyond my own perl-fu,
I seem to understand that the script itself is treating stdin and stdout
all the same, so it would make no difference if we merge them in the
test case itself.

That, on the other hand, works around the problem:

--- a/test/faildiff.test
+++ b/test/faildiff.test
@@ -27,8 +27,9 @@ What happens on binary files?
        > File test.bin added to patch %{P}test.diff
 
        $ printf "\\003\\000\\001" > test.bin
-       $ quilt diff -pab --no-index
+       $ quilt diff -pab --no-index 2>/dev/null
        >~ (Files|Binary files) a/test\.bin and b/test\.bin differ
+       $ quilt diff -pab --no-index 1>/dev/null
        > Diff failed on file 'test.bin', aborting
        $ echo %{?}
        > 1

But isn't it more of a hack than an actual fix? Aren't stdout and
stderr supposed to be line-buffered? And when pointing to the same file
or tty, shouldn't the messages reach said file or tty in the same order
as printed in the code?

From a UI perspective, it doesn't seem right that the messages can
reach the user in apparently random order (even though nothing really
bad will result from that in this specific case.)

-- 
Jean Delvare
SUSE L3 Support