threads / patch / 45315

patcht2027: avoid using pipes

Subject: [PATCH] t2027: avoid using pipes

## tl;dr

11 messages between Mar 8, 2017 and Mar 10, 2017. Diffs are folded; open one to read it.

replies: 10people: 3as markdown or json

Prathamesh Chavan· Mar 8, 2017, 15:13 UTC · lore

The exit code of the upstream of a pipe is ignored thus we should avoid using it. 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@gmail.com>
---
 t/t2027-worktree-list.sh | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Show changes to t/t2027-worktree-list.sh +7 −8

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 &&
 	test_cmp expect actual
 '
 
@@ -40,7 +40,7 @@ test_expect_success '"list" all worktrees from linked' '
 	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 -C here worktree list | sed "s/  */ /g" >actual &&
+	git -C here worktree list >out && sed "s/  */ /g" <out >actual &&
 	test_cmp expect actual
 '
 
@@ -73,7 +73,7 @@ test_expect_success '"list" all worktrees from bare main' '
 	git -C bare1 worktree add --detach ../there master &&
 	echo "$(pwd)/bare1 (bare)" >expect &&
 	echo "$(git -C there rev-parse --show-toplevel) $(git -C there rev-parse --short HEAD) (detached HEAD)" >>expect &&
-	git -C bare1 worktree list | sed "s/  */ /g" >actual &&
+	git -C bare1 worktree list >out && sed "s/  */ /g" <out >actual &&
 	test_cmp expect actual
 '
 
@@ -96,7 +96,7 @@ test_expect_success '"list" all worktrees from linked with a bare main' '
 	git -C bare1 worktree add --detach ../there master &&
 	echo "$(pwd)/bare1 (bare)" >expect &&
 	echo "$(git -C there rev-parse --show-toplevel) $(git -C there rev-parse --short HEAD) (detached HEAD)" >>expect &&
-	git -C there worktree list | sed "s/  */ /g" >actual &&
+	git -C there worktree list >out && sed "s/  */ /g" <out >actual &&
 	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 &&
 		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
 	)
 '
@@ -134,7 +134,7 @@ test_expect_success 'linked worktrees are sorted' '
 		test_commit new &&
 		git worktree add ../first &&
 		git worktree add ../second &&
-		git worktree list --porcelain | grep ^worktree >actual
+		git worktree list --porcelain >out && grep ^worktree out >actual
 	) &&
 	cat >expected <<-EOF &&
 	worktree $(pwd)/sorted/main

--
https://github.com/git/git/pull/336

Jon Loeliger· Mar 8, 2017, 15:44 UTC · re: Prathamesh Chavan · lore

Re: [PATCH] t2027: avoid using pipes

So, like, Prathamesh Chavan said:

Show 22 quoted lines

> The exit code of the upstream of a pipe is ignored thus we should avoid
> using it. 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@gmail.com>
> ---
>  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 &&
>  	test_cmp expect actual
>  '

I confess I am not familiar with the test set up. However, I'd ask the question do we care about the lingering "out" and "actual" files here? Or will they silently be cleaned up along the way later?

Thanks, jdl

Prathamesh Chavan· Mar 8, 2017, 21:38 UTC · re: Jon Loeliger · lore

Re: [PATCH] t2027: avoid using pipes

Whenever a test suite is executed, after finishing every test, after running all tests, the function test_done is called. You may find this function in test-lib.sh . This function displays the result of the test and also removes the trash created by running the test.

On Wed, Mar 8, 2017 at 9:14 PM, Jon Loeliger <jdl@jdl.com> wrote:

Show 31 quoted lines

> So, like, Prathamesh Chavan said:
>> The exit code of the upstream of a pipe is ignored thus we should avoid
>> using it. 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@gmail.com>
>> ---
>>  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 &&
>>       test_cmp expect actual
>>  '
>
> I confess I am not familiar with the test set up.
> However, I'd ask the question do we care about the
> lingering "out" and "actual" files here?  Or will
> they silently be cleaned up along the way later?
>
> Thanks,
> jdl

Prathamesh Chavan· Mar 8, 2017, 22:13 UTC · re: Prathamesh Chavan · lore

Re: [PATCH] t2027: avoid using pipes

But when I read the function carefully, it only removes the trash files created when test_failure is equal to zero. But as far as I know, I can see the files being removed even when a test_failure is non-zero for some test script.

On Thu, Mar 9, 2017 at 3:08 AM, Prathamesh Chavan <pc44800@gmail.com> wrote:

Show 37 quoted lines

> Whenever a test suite is executed, after finishing every test, after running
> all tests, the function test_done is called. You may find this function in
> test-lib.sh . This function displays the result of the test and also removes
> the trash created by running the test.
>
> On Wed, Mar 8, 2017 at 9:14 PM, Jon Loeliger <jdl@jdl.com> wrote:
>> So, like, Prathamesh Chavan said:
>>> The exit code of the upstream of a pipe is ignored thus we should avoid
>>> using it. 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@gmail.com>
>>> ---
>>>  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 &&
>>>       test_cmp expect actual
>>>  '
>>
>> I confess I am not familiar with the test set up.
>> However, I'd ask the question do we care about the
>> lingering "out" and "actual" files here?  Or will
>> they silently be cleaned up along the way later?
>>
>> Thanks,
>> jdl

Christian Couder· Mar 9, 2017, 08:08 UTC · re: Prathamesh Chavan · lore

Re: [PATCH] t2027: avoid using pipes

On Wed, Mar 8, 2017 at 4:13 PM, Prathamesh Chavan <pc44800@gmail.com> 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.

Show 18 quoted lines

> 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@gmail.com>
> ---
>  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...

Show 9 quoted lines

>         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.

Show 6 quoted lines

>                 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
>         )
>  '

Prathamesh Chavan· Mar 9, 2017, 08:56 UTC · re: Christian Couder · lore

Re: [PATCH] t2027: avoid using pipes

On Thu, Mar 9, 2017 at 1:38 PM, Christian Couder <christian.couder@gmail.com> wrote:

Show 7 quoted lines

> On Wed, Mar 8, 2017 at 4:13 PM, Prathamesh Chavan <pc44800@gmail.com> 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.

Show 24 quoted lines

>
>> 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@gmail.com>
>> ---
>>  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.

Show 18 quoted lines

>>         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
>>         )
>>  '

Prathamesh Chavan· Mar 9, 2017, 09:39 UTC · re: Prathamesh Chavan · lore

[PATCH v2] t2027: avoid using pipes

The exit code of the upstream of a pipe is ignored thus we should avoid using it. 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@gmail.com>
---
 t/t2027-worktree-list.sh | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Show changes to t/t2027-worktree-list.sh +7 −8

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 &&
 	test_cmp expect actual
 '
 
@@ -40,7 +40,7 @@ test_expect_success '"list" all worktrees from linked' '
 	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 -C here worktree list | sed "s/  */ /g" >actual &&
+	git -C here worktree list >out && sed "s/  */ /g" <out >actual &&
 	test_cmp expect actual
 '
 
@@ -73,7 +73,7 @@ test_expect_success '"list" all worktrees from bare main' '
 	git -C bare1 worktree add --detach ../there master &&
 	echo "$(pwd)/bare1 (bare)" >expect &&
 	echo "$(git -C there rev-parse --show-toplevel) $(git -C there rev-parse --short HEAD) (detached HEAD)" >>expect &&
-	git -C bare1 worktree list | sed "s/  */ /g" >actual &&
+	git -C bare1 worktree list >out && sed "s/  */ /g" <out >actual &&
 	test_cmp expect actual
 '
 
@@ -96,7 +96,7 @@ test_expect_success '"list" all worktrees from linked with a bare main' '
 	git -C bare1 worktree add --detach ../there master &&
 	echo "$(pwd)/bare1 (bare)" >expect &&
 	echo "$(git -C there rev-parse --show-toplevel) $(git -C there rev-parse --short HEAD) (detached HEAD)" >>expect &&
-	git -C there worktree list | sed "s/  */ /g" >actual &&
+	git -C there worktree list >out && sed "s/  */ /g" <out >actual &&
 	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 &&
 		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
 	)
 '
@@ -134,7 +134,7 @@ test_expect_success 'linked worktrees are sorted' '
 		test_commit new &&
 		git worktree add ../first &&
 		git worktree add ../second &&
-		git worktree list --porcelain | grep ^worktree >actual
+		git worktree list --porcelain >out && grep ^worktree out >actual
 	) &&
 	cat >expected <<-EOF &&
 	worktree $(pwd)/sorted/main

--
https://github.com/git/git/pull/336

Prathamesh Chavan· Mar 9, 2017, 09:53 UTC · re: Prathamesh Chavan · lore

[PATCH v2] t2027: avoid using pipes

Whenever a git command is present in the upstream of a pipe, its failure gets masked by piping and hence it should be avoided for testing the upstream git command. By writing out the output of the git command to a file, we can test the exit codes of both the commands as a failure exit code in any command is able to stop the && chain.

Signed-off-by: Prathamesh <pc44800@gmail.com>
---
 t/t2027-worktree-list.sh | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Show changes to t/t2027-worktree-list.sh +7 −8

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 &&
 	test_cmp expect actual
 '
 
@@ -40,7 +40,7 @@ test_expect_success '"list" all worktrees from linked' '
 	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 -C here worktree list | sed "s/  */ /g" >actual &&
+	git -C here worktree list >out && sed "s/  */ /g" <out >actual &&
 	test_cmp expect actual
 '
 
@@ -73,7 +73,7 @@ test_expect_success '"list" all worktrees from bare main' '
 	git -C bare1 worktree add --detach ../there master &&
 	echo "$(pwd)/bare1 (bare)" >expect &&
 	echo "$(git -C there rev-parse --show-toplevel) $(git -C there rev-parse --short HEAD) (detached HEAD)" >>expect &&
-	git -C bare1 worktree list | sed "s/  */ /g" >actual &&
+	git -C bare1 worktree list >out && sed "s/  */ /g" <out >actual &&
 	test_cmp expect actual
 '
 
@@ -96,7 +96,7 @@ test_expect_success '"list" all worktrees from linked with a bare main' '
 	git -C bare1 worktree add --detach ../there master &&
 	echo "$(pwd)/bare1 (bare)" >expect &&
 	echo "$(git -C there rev-parse --show-toplevel) $(git -C there rev-parse --short HEAD) (detached HEAD)" >>expect &&
-	git -C there worktree list | sed "s/  */ /g" >actual &&
+	git -C there worktree list >out && sed "s/  */ /g" <out >actual &&
 	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 &&
 		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
 	)
 '
@@ -134,7 +134,7 @@ test_expect_success 'linked worktrees are sorted' '
 		test_commit new &&
 		git worktree add ../first &&
 		git worktree add ../second &&
-		git worktree list --porcelain | grep ^worktree >actual
+		git worktree list --porcelain >out && grep ^worktree out >actual
 	) &&
 	cat >expected <<-EOF &&
 	worktree $(pwd)/sorted/main

--
https://github.com/git/git/pull/336

Christian Couder· Mar 9, 2017, 12:30 UTC · re: Prathamesh Chavan · lore

Re: [PATCH v2] t2027: avoid using pipes

On Thu, Mar 9, 2017 at 10:53 AM, Prathamesh Chavan <pc44800@gmail.com> wrote:

Show 8 quoted lines

> Whenever a git command is present in the upstream of a pipe, its failure
> gets masked by piping and hence it should be avoided for testing the
> upstream git command. By writing out the output of the git command to
> a file, we can test the exit codes of both the commands as a failure exit
> code in any command is able to stop the && chain.
>
> Signed-off-by: Prathamesh <pc44800@gmail.com>
> ---

Please add in Cc those who previously commented on the patch.

Show 13 quoted lines

>  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 still think that it would be better if the 'sed' commend was on its own line like this:

+       git worktree list >out &&
+       sed "s/  */ /g" <out >actual &&

>         test_cmp expect actual
>  '

Prathamesh Chavan· Mar 9, 2017, 19:18 UTC · re: Christian Couder · lore

From: Prathamesh <pc44800@gmail.com>

Signed-off-by: Prathamesh <pc44800@gmail.com>
---
 t/t2027-worktree-list.sh | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

Show changes to t/t2027-worktree-list.sh +11 −7

diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh
index 848da5f36..d8b3907e0 100755
--- a/t/t2027-worktree-list.sh
+++ b/t/t2027-worktree-list.sh
@@ -31,7 +31,8 @@ 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 &&
 	test_cmp expect actual
 '
 
@@ -40,7 +41,8 @@ test_expect_success '"list" all worktrees from linked' '
 	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 -C here worktree list | sed "s/  */ /g" >actual &&
+	git -C here worktree list >out &&
+	sed "s/  */ /g" <out >actual &&
 	test_cmp expect actual
 '
 
@@ -73,7 +75,8 @@ test_expect_success '"list" all worktrees from bare main' '
 	git -C bare1 worktree add --detach ../there master &&
 	echo "$(pwd)/bare1 (bare)" >expect &&
 	echo "$(git -C there rev-parse --show-toplevel) $(git -C there rev-parse --short HEAD) (detached HEAD)" >>expect &&
-	git -C bare1 worktree list | sed "s/  */ /g" >actual &&
+	git -C bare1 worktree list >out &&
+	sed "s/  */ /g" <out >actual &&
 	test_cmp expect actual
 '
 
@@ -96,7 +99,8 @@ test_expect_success '"list" all worktrees from linked with a bare main' '
 	git -C bare1 worktree add --detach ../there master &&
 	echo "$(pwd)/bare1 (bare)" >expect &&
 	echo "$(git -C there rev-parse --show-toplevel) $(git -C there rev-parse --short HEAD) (detached HEAD)" >>expect &&
-	git -C there worktree list | sed "s/  */ /g" >actual &&
+	git -C there worktree list >out &&
+	sed "s/  */ /g" <out >actual &&
 	test_cmp expect actual
 '
 
@@ -118,9 +122,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 &&
 		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
 	)
 '
@@ -134,7 +138,7 @@ test_expect_success 'linked worktrees are sorted' '
 		test_commit new &&
 		git worktree add ../first &&
 		git worktree add ../second &&
-		git worktree list --porcelain | grep ^worktree >actual
+		git worktree list --porcelain >out && grep ^worktree out >actual
 	) &&
 	cat >expected <<-EOF &&
 	worktree $(pwd)/sorted/main

-- 
2.11.0

Prathamesh Chavan· Mar 10, 2017, 13:34 UTC · re: Christian Couder · lore

Re: [PATCH v2] t2027: avoid using pipes

On Thu, Mar 9, 2017 at 6:00 PM, Christian Couder <christian.couder@gmail.com> wrote:

Show 11 quoted lines

> On Thu, Mar 9, 2017 at 10:53 AM, Prathamesh Chavan <pc44800@gmail.com> wrote:
>> Whenever a git command is present in the upstream of a pipe, its failure
>> gets masked by piping and hence it should be avoided for testing the
>> upstream git command. By writing out the output of the git command to
>> a file, we can test the exit codes of both the commands as a failure exit
>> code in any command is able to stop the && chain.
>>
>> Signed-off-by: Prathamesh <pc44800@gmail.com>
>> ---
>
> Please add in Cc those who previously commented on the patch.

Actually I initially used submitGit to send patches, where there was no option of adding cc to the patch. But after your comment I have switched to git send-email and git format-patch for sending patches.

Show 23 quoted lines

>
>>  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 still think that it would be better if the 'sed' commend was on its
> own line like this:
>
> +       git worktree list >out &&
> +       sed "s/  */ /g" <out >actual &&
>
>>         test_cmp expect actual
>>  '