Web lists-archives.com

Re: [PATCH] t2027: avoid using pipes




On Thu, Mar 9, 2017 at 1:38 PM, Christian Couder
<christian.couder@xxxxxxxxx> wrote:
> On Wed, Mar 8, 2017 at 4:13 PM, Prathamesh Chavan <pc44800@xxxxxxxxx> wrote:
>> The exit code of the upstream of a pipe is ignored thus we should avoid
>> using it.
>
> You might want to say more specifically that we should avoid piping a
> git command into another one as this could mask a failure of the git
> command.

Yes. I will add be specific, and update my patch.

>
>> By writing out the output of the git command to a file, we
>> can test the exit codes of both the commands.
>>
>> Signed-off-by: Prathamesh <pc44800@xxxxxxxxx>
>> ---
>>  t/t2027-worktree-list.sh | 14 +++++++-------
>>  1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh
>> index 848da5f..daa7a04 100755
>> --- a/t/t2027-worktree-list.sh
>> +++ b/t/t2027-worktree-list.sh
>> @@ -31,7 +31,7 @@ test_expect_success '"list" all worktrees from main' '
>>         test_when_finished "rm -rf here && git worktree prune" &&
>>         git worktree add --detach here master &&
>>         echo "$(git -C here rev-parse --show-toplevel) $(git rev-parse --short HEAD) (detached HEAD)" >>expect &&
>> -       git worktree list | sed "s/  */ /g" >actual &&
>> +       git worktree list >out && sed "s/  */ /g" <out >actual &&
>
> I think it's better if the 'sed' command is on a separate line.
>
> Also you may have used just "out" instead of "<out" in the 'sed' command...
>

Actually I noticed that:
$ git grep sed |grep "<" |wc -l
307

As at most places, wherever pipes aren't being used, the input to sed command is
passed using "<". Hence I chose to use "<" at places specifically at
places where sed
was used, even after knowing that just "out" will work.


>>         test_cmp expect actual
>>  '
>>
>> @@ -118,9 +118,9 @@ test_expect_success 'broken main worktree still at the top' '
>>                 cd linked &&
>>                 echo "worktree $(pwd)" >expected &&
>>                 echo "ref: .broken" >../.git/HEAD &&
>> -               git worktree list --porcelain | head -n 3 >actual &&
>> +               git worktree list --porcelain >out && head -n 3 out >actual &&
>
> ... as above you use "out" not "<out" in the 'head' command.
>
>>                 test_cmp ../expected actual &&
>> -               git worktree list | head -n 1 >actual.2 &&
>> +               git worktree list >out && head -n 1 out >actual.2 &&
>>                 grep -F "(error)" actual.2
>>         )
>>  '