{"thread":{"id":"45315","subject":"[PATCH] t2027: avoid using pipes","startedAt":"2017-03-08T15:13:42Z","lastAt":"2017-03-10T13:34:21Z","messageCount":11,"participants":["Prathamesh Chavan","Jon Loeliger","Christian Couder"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"313498","messageId":"0102015aae7b8536-00c57d0a-1d48-4153-a202-87c4ea9e0e19-000000@eu-west-1.amazonses.com","threadId":"45315","inReplyTo":null,"subject":"[PATCH] t2027: avoid using pipes","fromName":"Prathamesh Chavan","fromEmail":"pc44800@gmail.com","sentAt":"2017-03-08T15:13:35Z","receivedAt":"2017-03-08T15:13:42Z","isPatch":true,"sender":{"key":"pc44800@gmail.com","avatar":"https://avatars.githubusercontent.com/u/17272661?v=4"},"body":"The exit code of the upstream of a pipe is ignored thus we should avoid\nusing it. By writing out the output of the git command to a file, we\ncan test the exit codes of both the commands.\n\nSigned-off-by: Prathamesh <pc44800@gmail.com>\n---\n t/t2027-worktree-list.sh | 14 +++++++-------\n 1 file changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh\nindex 848da5f..daa7a04 100755\n--- a/t/t2027-worktree-list.sh\n+++ b/t/t2027-worktree-list.sh\n@@ -31,7 +31,7 @@ test_expect_success '\"list\" all worktrees from main' '\n \ttest_when_finished \"rm -rf here && git worktree prune\" &&\n \tgit worktree add --detach here master &&\n \techo \"$(git -C here rev-parse --show-toplevel) $(git rev-parse --short HEAD) (detached HEAD)\" >>expect &&\n-\tgit worktree list | sed \"s/  */ /g\" >actual &&\n+\tgit worktree list >out && sed \"s/  */ /g\" <out >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -40,7 +40,7 @@ test_expect_success '\"list\" all worktrees from linked' '\n \ttest_when_finished \"rm -rf here && git worktree prune\" &&\n \tgit worktree add --detach here master &&\n \techo \"$(git -C here rev-parse --show-toplevel) $(git rev-parse --short HEAD) (detached HEAD)\" >>expect &&\n-\tgit -C here worktree list | sed \"s/  */ /g\" >actual &&\n+\tgit -C here worktree list >out && sed \"s/  */ /g\" <out >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -73,7 +73,7 @@ test_expect_success '\"list\" all worktrees from bare main' '\n \tgit -C bare1 worktree add --detach ../there master &&\n \techo \"$(pwd)/bare1 (bare)\" >expect &&\n \techo \"$(git -C there rev-parse --show-toplevel) $(git -C there rev-parse --short HEAD) (detached HEAD)\" >>expect &&\n-\tgit -C bare1 worktree list | sed \"s/  */ /g\" >actual &&\n+\tgit -C bare1 worktree list >out && sed \"s/  */ /g\" <out >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -96,7 +96,7 @@ test_expect_success '\"list\" all worktrees from linked with a bare main' '\n \tgit -C bare1 worktree add --detach ../there master &&\n \techo \"$(pwd)/bare1 (bare)\" >expect &&\n \techo \"$(git -C there rev-parse --show-toplevel) $(git -C there rev-parse --short HEAD) (detached HEAD)\" >>expect &&\n-\tgit -C there worktree list | sed \"s/  */ /g\" >actual &&\n+\tgit -C there worktree list >out && sed \"s/  */ /g\" <out >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -118,9 +118,9 @@ test_expect_success 'broken main worktree still at the top' '\n \t\tcd linked &&\n \t\techo \"worktree $(pwd)\" >expected &&\n \t\techo \"ref: .broken\" >../.git/HEAD &&\n-\t\tgit worktree list --porcelain | head -n 3 >actual &&\n+\t\tgit worktree list --porcelain >out && head -n 3 out >actual &&\n \t\ttest_cmp ../expected actual &&\n-\t\tgit worktree list | head -n 1 >actual.2 &&\n+\t\tgit worktree list >out && head -n 1 out >actual.2 &&\n \t\tgrep -F \"(error)\" actual.2\n \t)\n '\n@@ -134,7 +134,7 @@ test_expect_success 'linked worktrees are sorted' '\n \t\ttest_commit new &&\n \t\tgit worktree add ../first &&\n \t\tgit worktree add ../second &&\n-\t\tgit worktree list --porcelain | grep ^worktree >actual\n+\t\tgit worktree list --porcelain >out && grep ^worktree out >actual\n \t) &&\n \tcat >expected <<-EOF &&\n \tworktree $(pwd)/sorted/main\n\n--\nhttps://github.com/git/git/pull/336\n"},{"id":"313504","messageId":"E1cldl4-0006L6-CU@mylo.jdl.com","threadId":"45315","inReplyTo":"0102015aae7b8536-00c57d0a-1d48-4153-a202-87c4ea9e0e19-000000@eu-west-1.amazonses.com","subject":"Re: [PATCH] t2027: avoid using pipes","fromName":"Jon Loeliger","fromEmail":"jdl@jdl.com","sentAt":"2017-03-08T15:44:10Z","receivedAt":"2017-03-08T15:44:30Z","isPatch":true,"sender":{"key":"jdl@jdl.com","avatar":"https://gravatar.com/avatar/75ce9a10b151acd2c28ec4ab2136dba7b2ff1634530bd04b155981a749d08a64?d=mp&s=160"},"body":"So, like, Prathamesh Chavan said:\n> The exit code of the upstream of a pipe is ignored thus we should avoid\n> using it. By writing out the output of the git command to a file, we\n> can test the exit codes of both the commands.\n> \n> Signed-off-by: Prathamesh <pc44800@gmail.com>\n> ---\n>  t/t2027-worktree-list.sh | 14 +++++++-------\n>  1 file changed, 7 insertions(+), 7 deletions(-)\n> \n> diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh\n> index 848da5f..daa7a04 100755\n> --- a/t/t2027-worktree-list.sh\n> +++ b/t/t2027-worktree-list.sh\n> @@ -31,7 +31,7 @@ test_expect_success '\"list\" all worktrees from main' '\n>  \ttest_when_finished \"rm -rf here && git worktree prune\" &&\n>  \tgit worktree add --detach here master &&\n>  \techo \"$(git -C here rev-parse --show-toplevel) $(git rev-parse --short \n> HEAD) (detached HEAD)\" >>expect &&\n> -\tgit worktree list | sed \"s/  */ /g\" >actual &&\n> +\tgit worktree list >out && sed \"s/  */ /g\" <out >actual &&\n>  \ttest_cmp expect actual\n>  '\n\nI confess I am not familiar with the test set up.\nHowever, I'd ask the question do we care about the\nlingering \"out\" and \"actual\" files here?  Or will\nthey silently be cleaned up along the way later?\n\nThanks,\njdl\n"},{"id":"313562","messageId":"CAME+mvV__RT5fbSBRU_SwP69xC-JuBE5bmfRbw91VN36-ToxZA@mail.gmail.com","threadId":"45315","inReplyTo":"CAME+mvWVDPT+-F7Z-O=XR_EN4qeNEoQ5ksLpLVkVBb0O9LKROg@mail.gmail.com","subject":"Re: [PATCH] t2027: avoid using pipes","fromName":"Prathamesh Chavan","fromEmail":"pc44800@gmail.com","sentAt":"2017-03-08T22:13:18Z","receivedAt":"2017-03-08T22:13:24Z","isPatch":true,"sender":{"key":"pc44800@gmail.com","avatar":"https://avatars.githubusercontent.com/u/17272661?v=4"},"body":"But when I read the function carefully, it only removes the trash files created\nwhen test_failure is equal to zero. But as far as I know, I can see the files\nbeing removed even when a test_failure is non-zero for some test script.\n\nOn Thu, Mar 9, 2017 at 3:08 AM, Prathamesh Chavan <pc44800@gmail.com> wrote:\n> Whenever a test suite is executed, after finishing every test, after running\n> all tests, the function test_done is called. You may find this function in\n> test-lib.sh . This function displays the result of the test and also removes\n> the trash created by running the test.\n>\n> On Wed, Mar 8, 2017 at 9:14 PM, Jon Loeliger <jdl@jdl.com> wrote:\n>> So, like, Prathamesh Chavan said:\n>>> The exit code of the upstream of a pipe is ignored thus we should avoid\n>>> using it. By writing out the output of the git command to a file, we\n>>> can test the exit codes of both the commands.\n>>>\n>>> Signed-off-by: Prathamesh <pc44800@gmail.com>\n>>> ---\n>>>  t/t2027-worktree-list.sh | 14 +++++++-------\n>>>  1 file changed, 7 insertions(+), 7 deletions(-)\n>>>\n>>> diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh\n>>> index 848da5f..daa7a04 100755\n>>> --- a/t/t2027-worktree-list.sh\n>>> +++ b/t/t2027-worktree-list.sh\n>>> @@ -31,7 +31,7 @@ test_expect_success '\"list\" all worktrees from main' '\n>>>       test_when_finished \"rm -rf here && git worktree prune\" &&\n>>>       git worktree add --detach here master &&\n>>>       echo \"$(git -C here rev-parse --show-toplevel) $(git rev-parse --short\n>>> HEAD) (detached HEAD)\" >>expect &&\n>>> -     git worktree list | sed \"s/  */ /g\" >actual &&\n>>> +     git worktree list >out && sed \"s/  */ /g\" <out >actual &&\n>>>       test_cmp expect actual\n>>>  '\n>>\n>> I confess I am not familiar with the test set up.\n>> However, I'd ask the question do we care about the\n>> lingering \"out\" and \"actual\" files here?  Or will\n>> they silently be cleaned up along the way later?\n>>\n>> Thanks,\n>> jdl\n"},{"id":"313564","messageId":"CAME+mvWVDPT+-F7Z-O=XR_EN4qeNEoQ5ksLpLVkVBb0O9LKROg@mail.gmail.com","threadId":"45315","inReplyTo":"E1cldl4-0006L6-CU@mylo.jdl.com","subject":"Re: [PATCH] t2027: avoid using pipes","fromName":"Prathamesh Chavan","fromEmail":"pc44800@gmail.com","sentAt":"2017-03-08T21:38:39Z","receivedAt":"2017-03-08T22:28:28Z","isPatch":true,"sender":{"key":"pc44800@gmail.com","avatar":"https://avatars.githubusercontent.com/u/17272661?v=4"},"body":"Whenever a test suite is executed, after finishing every test, after running\nall tests, the function test_done is called. You may find this function in\ntest-lib.sh . This function displays the result of the test and also removes\nthe trash created by running the test.\n\nOn Wed, Mar 8, 2017 at 9:14 PM, Jon Loeliger <jdl@jdl.com> wrote:\n> So, like, Prathamesh Chavan said:\n>> The exit code of the upstream of a pipe is ignored thus we should avoid\n>> using it. By writing out the output of the git command to a file, we\n>> can test the exit codes of both the commands.\n>>\n>> Signed-off-by: Prathamesh <pc44800@gmail.com>\n>> ---\n>>  t/t2027-worktree-list.sh | 14 +++++++-------\n>>  1 file changed, 7 insertions(+), 7 deletions(-)\n>>\n>> diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh\n>> index 848da5f..daa7a04 100755\n>> --- a/t/t2027-worktree-list.sh\n>> +++ b/t/t2027-worktree-list.sh\n>> @@ -31,7 +31,7 @@ test_expect_success '\"list\" all worktrees from main' '\n>>       test_when_finished \"rm -rf here && git worktree prune\" &&\n>>       git worktree add --detach here master &&\n>>       echo \"$(git -C here rev-parse --show-toplevel) $(git rev-parse --short\n>> HEAD) (detached HEAD)\" >>expect &&\n>> -     git worktree list | sed \"s/  */ /g\" >actual &&\n>> +     git worktree list >out && sed \"s/  */ /g\" <out >actual &&\n>>       test_cmp expect actual\n>>  '\n>\n> I confess I am not familiar with the test set up.\n> However, I'd ask the question do we care about the\n> lingering \"out\" and \"actual\" files here?  Or will\n> they silently be cleaned up along the way later?\n>\n> Thanks,\n> jdl\n"},{"id":"313621","messageId":"CAP8UFD0GtRdjCMcbhjgA0rVaAMFtyto8JxfqbivODarBB0eK8w@mail.gmail.com","threadId":"45315","inReplyTo":"0102015aae7b8536-00c57d0a-1d48-4153-a202-87c4ea9e0e19-000000@eu-west-1.amazonses.com","subject":"Re: [PATCH] t2027: avoid using pipes","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-03-09T08:08:33Z","receivedAt":"2017-03-09T08:08:43Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Wed, Mar 8, 2017 at 4:13 PM, Prathamesh Chavan <pc44800@gmail.com> wrote:\n> The exit code of the upstream of a pipe is ignored thus we should avoid\n> using it.\n\nYou might want to say more specifically that we should avoid piping a\ngit command into another one as this could mask a failure of the git\ncommand.\n\n> By writing out the output of the git command to a file, we\n> can test the exit codes of both the commands.\n>\n> Signed-off-by: Prathamesh <pc44800@gmail.com>\n> ---\n>  t/t2027-worktree-list.sh | 14 +++++++-------\n>  1 file changed, 7 insertions(+), 7 deletions(-)\n>\n> diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh\n> index 848da5f..daa7a04 100755\n> --- a/t/t2027-worktree-list.sh\n> +++ b/t/t2027-worktree-list.sh\n> @@ -31,7 +31,7 @@ test_expect_success '\"list\" all worktrees from main' '\n>         test_when_finished \"rm -rf here && git worktree prune\" &&\n>         git worktree add --detach here master &&\n>         echo \"$(git -C here rev-parse --show-toplevel) $(git rev-parse --short HEAD) (detached HEAD)\" >>expect &&\n> -       git worktree list | sed \"s/  */ /g\" >actual &&\n> +       git worktree list >out && sed \"s/  */ /g\" <out >actual &&\n\nI think it's better if the 'sed' command is on a separate line.\n\nAlso you may have used just \"out\" instead of \"<out\" in the 'sed' command...\n\n>         test_cmp expect actual\n>  '\n>\n> @@ -118,9 +118,9 @@ test_expect_success 'broken main worktree still at the top' '\n>                 cd linked &&\n>                 echo \"worktree $(pwd)\" >expected &&\n>                 echo \"ref: .broken\" >../.git/HEAD &&\n> -               git worktree list --porcelain | head -n 3 >actual &&\n> +               git worktree list --porcelain >out && head -n 3 out >actual &&\n\n... as above you use \"out\" not \"<out\" in the 'head' command.\n\n>                 test_cmp ../expected actual &&\n> -               git worktree list | head -n 1 >actual.2 &&\n> +               git worktree list >out && head -n 1 out >actual.2 &&\n>                 grep -F \"(error)\" actual.2\n>         )\n>  '\n"},{"id":"313622","messageId":"CAME+mvUDsBec0L9o_wpAMin-rbn-SqS1OZcuyfRw+U7b-EOXeQ@mail.gmail.com","threadId":"45315","inReplyTo":"CAP8UFD0GtRdjCMcbhjgA0rVaAMFtyto8JxfqbivODarBB0eK8w@mail.gmail.com","subject":"Re: [PATCH] t2027: avoid using pipes","fromName":"Prathamesh Chavan","fromEmail":"pc44800@gmail.com","sentAt":"2017-03-09T08:56:09Z","receivedAt":"2017-03-09T09:31:38Z","isPatch":true,"sender":{"key":"pc44800@gmail.com","avatar":"https://avatars.githubusercontent.com/u/17272661?v=4"},"body":"On Thu, Mar 9, 2017 at 1:38 PM, Christian Couder\n<christian.couder@gmail.com> wrote:\n> On Wed, Mar 8, 2017 at 4:13 PM, Prathamesh Chavan <pc44800@gmail.com> wrote:\n>> The exit code of the upstream of a pipe is ignored thus we should avoid\n>> using it.\n>\n> You might want to say more specifically that we should avoid piping a\n> git command into another one as this could mask a failure of the git\n> command.\n\nYes. I will add be specific, and update my patch.\n\n>\n>> By writing out the output of the git command to a file, we\n>> can test the exit codes of both the commands.\n>>\n>> Signed-off-by: Prathamesh <pc44800@gmail.com>\n>> ---\n>>  t/t2027-worktree-list.sh | 14 +++++++-------\n>>  1 file changed, 7 insertions(+), 7 deletions(-)\n>>\n>> diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh\n>> index 848da5f..daa7a04 100755\n>> --- a/t/t2027-worktree-list.sh\n>> +++ b/t/t2027-worktree-list.sh\n>> @@ -31,7 +31,7 @@ test_expect_success '\"list\" all worktrees from main' '\n>>         test_when_finished \"rm -rf here && git worktree prune\" &&\n>>         git worktree add --detach here master &&\n>>         echo \"$(git -C here rev-parse --show-toplevel) $(git rev-parse --short HEAD) (detached HEAD)\" >>expect &&\n>> -       git worktree list | sed \"s/  */ /g\" >actual &&\n>> +       git worktree list >out && sed \"s/  */ /g\" <out >actual &&\n>\n> I think it's better if the 'sed' command is on a separate line.\n>\n> Also you may have used just \"out\" instead of \"<out\" in the 'sed' command...\n>\n\nActually I noticed that:\n$ git grep sed |grep \"<\" |wc -l\n307\n\nAs at most places, wherever pipes aren't being used, the input to sed command is\npassed using \"<\". Hence I chose to use \"<\" at places specifically at\nplaces where sed\nwas used, even after knowing that just \"out\" will work.\n\n\n>>         test_cmp expect actual\n>>  '\n>>\n>> @@ -118,9 +118,9 @@ test_expect_success 'broken main worktree still at the top' '\n>>                 cd linked &&\n>>                 echo \"worktree $(pwd)\" >expected &&\n>>                 echo \"ref: .broken\" >../.git/HEAD &&\n>> -               git worktree list --porcelain | head -n 3 >actual &&\n>> +               git worktree list --porcelain >out && head -n 3 out >actual &&\n>\n> ... as above you use \"out\" not \"<out\" in the 'head' command.\n>\n>>                 test_cmp ../expected actual &&\n>> -               git worktree list | head -n 1 >actual.2 &&\n>> +               git worktree list >out && head -n 1 out >actual.2 &&\n>>                 grep -F \"(error)\" actual.2\n>>         )\n>>  '\n"},{"id":"313623","messageId":"0102015ab26fcf13-1659be12-a85c-47be-9a77-8f1b0b8a3897-000000@eu-west-1.amazonses.com","threadId":"45315","inReplyTo":"0102015aae7b8536-00c57d0a-1d48-4153-a202-87c4ea9e0e19-000000@eu-west-1.amazonses.com","subject":"[PATCH v2] t2027: avoid using pipes","fromName":"Prathamesh Chavan","fromEmail":"pc44800@gmail.com","sentAt":"2017-03-09T09:39:16Z","receivedAt":"2017-03-09T09:40:58Z","isPatch":true,"sender":{"key":"pc44800@gmail.com","avatar":"https://avatars.githubusercontent.com/u/17272661?v=4"},"body":"The exit code of the upstream of a pipe is ignored thus we should avoid\nusing it. By writing out the output of the git command to a file, we\ncan test the exit codes of both the commands.\n\nSigned-off-by: Prathamesh <pc44800@gmail.com>\n---\n t/t2027-worktree-list.sh | 14 +++++++-------\n 1 file changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh\nindex 848da5f..daa7a04 100755\n--- a/t/t2027-worktree-list.sh\n+++ b/t/t2027-worktree-list.sh\n@@ -31,7 +31,7 @@ test_expect_success '\"list\" all worktrees from main' '\n \ttest_when_finished \"rm -rf here && git worktree prune\" &&\n \tgit worktree add --detach here master &&\n \techo \"$(git -C here rev-parse --show-toplevel) $(git rev-parse --short HEAD) (detached HEAD)\" >>expect &&\n-\tgit worktree list | sed \"s/  */ /g\" >actual &&\n+\tgit worktree list >out && sed \"s/  */ /g\" <out >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -40,7 +40,7 @@ test_expect_success '\"list\" all worktrees from linked' '\n \ttest_when_finished \"rm -rf here && git worktree prune\" &&\n \tgit worktree add --detach here master &&\n \techo \"$(git -C here rev-parse --show-toplevel) $(git rev-parse --short HEAD) (detached HEAD)\" >>expect &&\n-\tgit -C here worktree list | sed \"s/  */ /g\" >actual &&\n+\tgit -C here worktree list >out && sed \"s/  */ /g\" <out >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -73,7 +73,7 @@ test_expect_success '\"list\" all worktrees from bare main' '\n \tgit -C bare1 worktree add --detach ../there master &&\n \techo \"$(pwd)/bare1 (bare)\" >expect &&\n \techo \"$(git -C there rev-parse --show-toplevel) $(git -C there rev-parse --short HEAD) (detached HEAD)\" >>expect &&\n-\tgit -C bare1 worktree list | sed \"s/  */ /g\" >actual &&\n+\tgit -C bare1 worktree list >out && sed \"s/  */ /g\" <out >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -96,7 +96,7 @@ test_expect_success '\"list\" all worktrees from linked with a bare main' '\n \tgit -C bare1 worktree add --detach ../there master &&\n \techo \"$(pwd)/bare1 (bare)\" >expect &&\n \techo \"$(git -C there rev-parse --show-toplevel) $(git -C there rev-parse --short HEAD) (detached HEAD)\" >>expect &&\n-\tgit -C there worktree list | sed \"s/  */ /g\" >actual &&\n+\tgit -C there worktree list >out && sed \"s/  */ /g\" <out >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -118,9 +118,9 @@ test_expect_success 'broken main worktree still at the top' '\n \t\tcd linked &&\n \t\techo \"worktree $(pwd)\" >expected &&\n \t\techo \"ref: .broken\" >../.git/HEAD &&\n-\t\tgit worktree list --porcelain | head -n 3 >actual &&\n+\t\tgit worktree list --porcelain >out && head -n 3 out >actual &&\n \t\ttest_cmp ../expected actual &&\n-\t\tgit worktree list | head -n 1 >actual.2 &&\n+\t\tgit worktree list >out && head -n 1 out >actual.2 &&\n \t\tgrep -F \"(error)\" actual.2\n \t)\n '\n@@ -134,7 +134,7 @@ test_expect_success 'linked worktrees are sorted' '\n \t\ttest_commit new &&\n \t\tgit worktree add ../first &&\n \t\tgit worktree add ../second &&\n-\t\tgit worktree list --porcelain | grep ^worktree >actual\n+\t\tgit worktree list --porcelain >out && grep ^worktree out >actual\n \t) &&\n \tcat >expected <<-EOF &&\n \tworktree $(pwd)/sorted/main\n\n--\nhttps://github.com/git/git/pull/336\n"},{"id":"313624","messageId":"0102015ab27c6633-c61f56f2-0504-4af3-badc-34246cf635aa-000000@eu-west-1.amazonses.com","threadId":"45315","inReplyTo":"0102015ab26fcf13-1659be12-a85c-47be-9a77-8f1b0b8a3897-000000@eu-west-1.amazonses.com","subject":"[PATCH v2] t2027: avoid using pipes","fromName":"Prathamesh Chavan","fromEmail":"pc44800@gmail.com","sentAt":"2017-03-09T09:53:01Z","receivedAt":"2017-03-09T09:54:30Z","isPatch":true,"sender":{"key":"pc44800@gmail.com","avatar":"https://avatars.githubusercontent.com/u/17272661?v=4"},"body":"Whenever a git command is present in the upstream of a pipe, its failure\ngets masked by piping and hence it should be avoided for testing the\nupstream git command. By writing out the output of the git command to\na file, we can test the exit codes of both the commands as a failure exit\ncode in any command is able to stop the && chain.\n\nSigned-off-by: Prathamesh <pc44800@gmail.com>\n---\n t/t2027-worktree-list.sh | 14 +++++++-------\n 1 file changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh\nindex 848da5f..daa7a04 100755\n--- a/t/t2027-worktree-list.sh\n+++ b/t/t2027-worktree-list.sh\n@@ -31,7 +31,7 @@ test_expect_success '\"list\" all worktrees from main' '\n \ttest_when_finished \"rm -rf here && git worktree prune\" &&\n \tgit worktree add --detach here master &&\n \techo \"$(git -C here rev-parse --show-toplevel) $(git rev-parse --short HEAD) (detached HEAD)\" >>expect &&\n-\tgit worktree list | sed \"s/  */ /g\" >actual &&\n+\tgit worktree list >out && sed \"s/  */ /g\" <out >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -40,7 +40,7 @@ test_expect_success '\"list\" all worktrees from linked' '\n \ttest_when_finished \"rm -rf here && git worktree prune\" &&\n \tgit worktree add --detach here master &&\n \techo \"$(git -C here rev-parse --show-toplevel) $(git rev-parse --short HEAD) (detached HEAD)\" >>expect &&\n-\tgit -C here worktree list | sed \"s/  */ /g\" >actual &&\n+\tgit -C here worktree list >out && sed \"s/  */ /g\" <out >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -73,7 +73,7 @@ test_expect_success '\"list\" all worktrees from bare main' '\n \tgit -C bare1 worktree add --detach ../there master &&\n \techo \"$(pwd)/bare1 (bare)\" >expect &&\n \techo \"$(git -C there rev-parse --show-toplevel) $(git -C there rev-parse --short HEAD) (detached HEAD)\" >>expect &&\n-\tgit -C bare1 worktree list | sed \"s/  */ /g\" >actual &&\n+\tgit -C bare1 worktree list >out && sed \"s/  */ /g\" <out >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -96,7 +96,7 @@ test_expect_success '\"list\" all worktrees from linked with a bare main' '\n \tgit -C bare1 worktree add --detach ../there master &&\n \techo \"$(pwd)/bare1 (bare)\" >expect &&\n \techo \"$(git -C there rev-parse --show-toplevel) $(git -C there rev-parse --short HEAD) (detached HEAD)\" >>expect &&\n-\tgit -C there worktree list | sed \"s/  */ /g\" >actual &&\n+\tgit -C there worktree list >out && sed \"s/  */ /g\" <out >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -118,9 +118,9 @@ test_expect_success 'broken main worktree still at the top' '\n \t\tcd linked &&\n \t\techo \"worktree $(pwd)\" >expected &&\n \t\techo \"ref: .broken\" >../.git/HEAD &&\n-\t\tgit worktree list --porcelain | head -n 3 >actual &&\n+\t\tgit worktree list --porcelain >out && head -n 3 out >actual &&\n \t\ttest_cmp ../expected actual &&\n-\t\tgit worktree list | head -n 1 >actual.2 &&\n+\t\tgit worktree list >out && head -n 1 out >actual.2 &&\n \t\tgrep -F \"(error)\" actual.2\n \t)\n '\n@@ -134,7 +134,7 @@ test_expect_success 'linked worktrees are sorted' '\n \t\ttest_commit new &&\n \t\tgit worktree add ../first &&\n \t\tgit worktree add ../second &&\n-\t\tgit worktree list --porcelain | grep ^worktree >actual\n+\t\tgit worktree list --porcelain >out && grep ^worktree out >actual\n \t) &&\n \tcat >expected <<-EOF &&\n \tworktree $(pwd)/sorted/main\n\n--\nhttps://github.com/git/git/pull/336\n"},{"id":"313638","messageId":"CAP8UFD19njU30HODYvp1pddpZaVSVGgn7whcTa2rdjMPe-vzYQ@mail.gmail.com","threadId":"45315","inReplyTo":"0102015ab27c6633-c61f56f2-0504-4af3-badc-34246cf635aa-000000@eu-west-1.amazonses.com","subject":"Re: [PATCH v2] t2027: avoid using pipes","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-03-09T12:30:45Z","receivedAt":"2017-03-09T12:31:28Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Thu, Mar 9, 2017 at 10:53 AM, Prathamesh Chavan <pc44800@gmail.com> wrote:\n> Whenever a git command is present in the upstream of a pipe, its failure\n> gets masked by piping and hence it should be avoided for testing the\n> upstream git command. By writing out the output of the git command to\n> a file, we can test the exit codes of both the commands as a failure exit\n> code in any command is able to stop the && chain.\n>\n> Signed-off-by: Prathamesh <pc44800@gmail.com>\n> ---\n\nPlease add in Cc those who previously commented on the patch.\n\n>  t/t2027-worktree-list.sh | 14 +++++++-------\n>  1 file changed, 7 insertions(+), 7 deletions(-)\n>\n> diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh\n> index 848da5f..daa7a04 100755\n> --- a/t/t2027-worktree-list.sh\n> +++ b/t/t2027-worktree-list.sh\n> @@ -31,7 +31,7 @@ test_expect_success '\"list\" all worktrees from main' '\n>         test_when_finished \"rm -rf here && git worktree prune\" &&\n>         git worktree add --detach here master &&\n>         echo \"$(git -C here rev-parse --show-toplevel) $(git rev-parse --short HEAD) (detached HEAD)\" >>expect &&\n> -       git worktree list | sed \"s/  */ /g\" >actual &&\n> +       git worktree list >out && sed \"s/  */ /g\" <out >actual &&\n\nI still think that it would be better if the 'sed' commend was on its\nown line like this:\n\n+       git worktree list >out &&\n+       sed \"s/  */ /g\" <out >actual &&\n\n>         test_cmp expect actual\n>  '\n"},{"id":"313669","messageId":"20170309191807.32361-1-pc44800@gmail.com","threadId":"45315","inReplyTo":"CAP8UFD19njU30HODYvp1pddpZaVSVGgn7whcTa2rdjMPe-vzYQ@mail.gmail.com","subject":"[PATCH] t2027: avoid using pipes","fromName":"Prathamesh Chavan","fromEmail":"pc44800@gmail.com","sentAt":"2017-03-09T19:18:07Z","receivedAt":"2017-03-09T19:29:32Z","isPatch":true,"sender":{"key":"pc44800@gmail.com","avatar":"https://avatars.githubusercontent.com/u/17272661?v=4"},"body":"From: Prathamesh <pc44800@gmail.com>\n\nWhenever a git command is present in the upstream of a pipe, its failure\ngets masked by piping and hence it should be avoided for testing the\nupstream git command. By writing out the output of the git command to\na file, we can test the exit codes of both the commands as a failure exit\ncode in any command is able to stop the && chain.\n\nSigned-off-by: Prathamesh <pc44800@gmail.com>\n---\n t/t2027-worktree-list.sh | 18 +++++++++++-------\n 1 file changed, 11 insertions(+), 7 deletions(-)\n\ndiff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh\nindex 848da5f36..d8b3907e0 100755\n--- a/t/t2027-worktree-list.sh\n+++ b/t/t2027-worktree-list.sh\n@@ -31,7 +31,8 @@ test_expect_success '\"list\" all worktrees from main' '\n \ttest_when_finished \"rm -rf here && git worktree prune\" &&\n \tgit worktree add --detach here master &&\n \techo \"$(git -C here rev-parse --show-toplevel) $(git rev-parse --short HEAD) (detached HEAD)\" >>expect &&\n-\tgit worktree list | sed \"s/  */ /g\" >actual &&\n+\tgit worktree list >out &&\n+\tsed \"s/  */ /g\" <out >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -40,7 +41,8 @@ test_expect_success '\"list\" all worktrees from linked' '\n \ttest_when_finished \"rm -rf here && git worktree prune\" &&\n \tgit worktree add --detach here master &&\n \techo \"$(git -C here rev-parse --show-toplevel) $(git rev-parse --short HEAD) (detached HEAD)\" >>expect &&\n-\tgit -C here worktree list | sed \"s/  */ /g\" >actual &&\n+\tgit -C here worktree list >out &&\n+\tsed \"s/  */ /g\" <out >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -73,7 +75,8 @@ test_expect_success '\"list\" all worktrees from bare main' '\n \tgit -C bare1 worktree add --detach ../there master &&\n \techo \"$(pwd)/bare1 (bare)\" >expect &&\n \techo \"$(git -C there rev-parse --show-toplevel) $(git -C there rev-parse --short HEAD) (detached HEAD)\" >>expect &&\n-\tgit -C bare1 worktree list | sed \"s/  */ /g\" >actual &&\n+\tgit -C bare1 worktree list >out &&\n+\tsed \"s/  */ /g\" <out >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -96,7 +99,8 @@ test_expect_success '\"list\" all worktrees from linked with a bare main' '\n \tgit -C bare1 worktree add --detach ../there master &&\n \techo \"$(pwd)/bare1 (bare)\" >expect &&\n \techo \"$(git -C there rev-parse --show-toplevel) $(git -C there rev-parse --short HEAD) (detached HEAD)\" >>expect &&\n-\tgit -C there worktree list | sed \"s/  */ /g\" >actual &&\n+\tgit -C there worktree list >out &&\n+\tsed \"s/  */ /g\" <out >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -118,9 +122,9 @@ test_expect_success 'broken main worktree still at the top' '\n \t\tcd linked &&\n \t\techo \"worktree $(pwd)\" >expected &&\n \t\techo \"ref: .broken\" >../.git/HEAD &&\n-\t\tgit worktree list --porcelain | head -n 3 >actual &&\n+\t\tgit worktree list --porcelain >out && head -n 3 out >actual &&\n \t\ttest_cmp ../expected actual &&\n-\t\tgit worktree list | head -n 1 >actual.2 &&\n+\t\tgit worktree list >out && head -n 1 out >actual.2 &&\n \t\tgrep -F \"(error)\" actual.2\n \t)\n '\n@@ -134,7 +138,7 @@ test_expect_success 'linked worktrees are sorted' '\n \t\ttest_commit new &&\n \t\tgit worktree add ../first &&\n \t\tgit worktree add ../second &&\n-\t\tgit worktree list --porcelain | grep ^worktree >actual\n+\t\tgit worktree list --porcelain >out && grep ^worktree out >actual\n \t) &&\n \tcat >expected <<-EOF &&\n \tworktree $(pwd)/sorted/main\n-- \n2.11.0\n\n"},{"id":"313749","messageId":"CAME+mvV0i7gZWUX_77Z2QrsdOWEq0LRDXX3iqKJ=9bCN+yv=vA@mail.gmail.com","threadId":"45315","inReplyTo":"CAP8UFD19njU30HODYvp1pddpZaVSVGgn7whcTa2rdjMPe-vzYQ@mail.gmail.com","subject":"Re: [PATCH v2] t2027: avoid using pipes","fromName":"Prathamesh Chavan","fromEmail":"pc44800@gmail.com","sentAt":"2017-03-10T13:34:08Z","receivedAt":"2017-03-10T13:34:21Z","isPatch":true,"sender":{"key":"pc44800@gmail.com","avatar":"https://avatars.githubusercontent.com/u/17272661?v=4"},"body":"On Thu, Mar 9, 2017 at 6:00 PM, Christian Couder\n<christian.couder@gmail.com> wrote:\n> On Thu, Mar 9, 2017 at 10:53 AM, Prathamesh Chavan <pc44800@gmail.com> wrote:\n>> Whenever a git command is present in the upstream of a pipe, its failure\n>> gets masked by piping and hence it should be avoided for testing the\n>> upstream git command. By writing out the output of the git command to\n>> a file, we can test the exit codes of both the commands as a failure exit\n>> code in any command is able to stop the && chain.\n>>\n>> Signed-off-by: Prathamesh <pc44800@gmail.com>\n>> ---\n>\n> Please add in Cc those who previously commented on the patch.\n\nActually I initially used submitGit to send patches, where there was\nno option of\nadding cc to the patch. But after your comment I have switched to git send-email\nand git format-patch for sending patches.\n\n>\n>>  t/t2027-worktree-list.sh | 14 +++++++-------\n>>  1 file changed, 7 insertions(+), 7 deletions(-)\n>>\n>> diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh\n>> index 848da5f..daa7a04 100755\n>> --- a/t/t2027-worktree-list.sh\n>> +++ b/t/t2027-worktree-list.sh\n>> @@ -31,7 +31,7 @@ test_expect_success '\"list\" all worktrees from main' '\n>>         test_when_finished \"rm -rf here && git worktree prune\" &&\n>>         git worktree add --detach here master &&\n>>         echo \"$(git -C here rev-parse --show-toplevel) $(git rev-parse --short HEAD) (detached HEAD)\" >>expect &&\n>> -       git worktree list | sed \"s/  */ /g\" >actual &&\n>> +       git worktree list >out && sed \"s/  */ /g\" <out >actual &&\n>\n> I still think that it would be better if the 'sed' commend was on its\n> own line like this:\n>\n> +       git worktree list >out &&\n> +       sed \"s/  */ /g\" <out >actual &&\n>\n>>         test_cmp expect actual\n>>  '\n"}]}