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

[Jim Meyering]

On Sun, Nov 13, 2016 at 11:05 AM, Jim Meyering <address@hidden> wrote:
> 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.

Here's the patch I expect to push:

cu-returns_-v3.diff
Description: Text document