Re: [PATCH] tests: use "returns_" rather than explicit comparison with "$?"

[Jim Meyering]

On Sun, Nov 13, 2016 at 9:24 AM, Bernhard Voelker
<address@hidden> wrote:
> On 11/13/2016 03:38 AM, Jim Meyering wrote:
>> I noticed many tests that compare directly with "$?". However, we now
>> have the "returns_" function (from init.sh) that can be used to make
>> the resulting code a little less error-prone: all on one line/stmt, it
>> is harder to accidentally insert code that accidentally clobbers "$?".
>>
>> Here's an example of what most of these changes look like:
>>
>>   -ls -l --time-style=XX > out 2> err
>>   -test $? = 2 || fail=1
>>   +returns_ 2 ls -l --time-style=XX > out 2> err || fail=1
>
> Nice work, thanks!
>
> BTW: what was your search pattern?  I mean, is there a reason
> you left places like the following alone?

Good catch. Thank you. Amended patch below.
I did the conversion in two steps. Initially I searched only for
single-digit values on the RHS, then realized my error and
found/converted the multi-digit ones, too, initially in a separate
commit. Could have sworn I merged them, but reflog shows no trace.

However, I did deliberately leave two unconverted, e.g.,

tests/ls/readdir-mountpoint-inode.sh-while read dir; do
tests/ls/readdir-mountpoint-inode.sh-  readdir_inode=$(inode_via_readdir "$dir")
tests/ls/readdir-mountpoint-inode.sh:  test $? = 77 && continue
tests/ls/readdir-mountpoint-inode.sh-  stat_inode=$(timeout 1 stat
--format=%i "$dir")
tests/ls/readdir-mountpoint-inode.sh-  # If stat fails or says the
inode is 0, skip $dir.
--
tests/misc/chroot-fail.sh-  returns_ 127 chroot / no_such || fail=1 #
no such command
tests/misc/chroot-fail.sh-else
tests/misc/chroot-fail.sh:  test $? = 125 || fail=1
tests/misc/chroot-fail.sh-  can_chroot_root=0
tests/misc/chroot-fail.sh-fi

We may want to convert those, too (if it can be done cleanly), so that
we can add an exception-free syntax-check. Or just add an exception
for each.

>   $ git grep -B1 -F '$? = ' -- tests
>   ..
>   tests/misc/stdbuf.sh-stdbuf -ol true # Capital 'L' required
>   tests/misc/stdbuf.sh:test $? = 125 || fail=1 # Internal error is a 
> particular status
>   ..
>
>   $ git grep -B1 -F '$? != ' -- tests
>   tests/du/8gb.sh-dd bs=1 seek=8G of=big < /dev/null 2> /dev/null
>   tests/du/8gb.sh:if test $? != 0; then
>   ..
>
> Furthermore, we need to tweak a syntax-check, see attached.

Good catch. Please push.

Thanks again for catching those.
I've attached both the delta and the merged V2.

cu-returns_-delta.diff
Description: Text document

cu-returns_-v2.diff
Description: Text document