{"thread":{"id":"44797","subject":"[PATCH] don't use test_must_fail with grep","startedAt":"2016-12-31T11:44:30Z","lastAt":"2017-01-09T09:54:37Z","messageCount":21,"participants":["Pranit Bauva","Luke Diamand","Johannes Sixt","Stefan Beller","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"308553","messageId":"20161231114412.23439-1-pranit.bauva@gmail.com","threadId":"44797","inReplyTo":null,"subject":"[PATCH] don't use test_must_fail with grep","fromName":"Pranit Bauva","fromEmail":"pranit.bauva@gmail.com","sentAt":"2016-12-31T11:44:12Z","receivedAt":"2016-12-31T11:44:30Z","isPatch":true,"sender":{"key":"pranit.bauva@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2959938?v=4"},"body":"test_must_fail should only be used for testing git commands. To test the\nfailure of other commands use `!`.\n\nReported-by: Stefan Beller <sbeller@google.com>\nSigned-off-by: Pranit Bauva <pranit.bauva@gmail.com>\n---\n t/t3510-cherry-pick-sequence.sh  |  6 +++---\n t/t5504-fetch-receive-strict.sh  |  2 +-\n t/t5516-fetch-push.sh            |  2 +-\n t/t5601-clone.sh                 |  2 +-\n t/t6030-bisect-porcelain.sh      |  2 +-\n t/t7610-mergetool.sh             |  2 +-\n t/t9001-send-email.sh            |  2 +-\n t/t9117-git-svn-init-clone.sh    | 12 ++++++------\n t/t9813-git-p4-preserve-users.sh |  8 ++++----\n t/t9814-git-p4-rename.sh         |  6 +++---\n 10 files changed, 22 insertions(+), 22 deletions(-)\n\ndiff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh\nindex 372307c21..0acf4b146 100755\n--- a/t/t3510-cherry-pick-sequence.sh\n+++ b/t/t3510-cherry-pick-sequence.sh\n@@ -385,7 +385,7 @@ test_expect_success '--continue respects opts' '\n \tgit cat-file commit HEAD~1 >picked_msg &&\n \tgit cat-file commit HEAD~2 >unrelatedpick_msg &&\n \tgit cat-file commit HEAD~3 >initial_msg &&\n-\ttest_must_fail grep \"cherry picked from\" initial_msg &&\n+\t! grep \"cherry picked from\" initial_msg &&\n \tgrep \"cherry picked from\" unrelatedpick_msg &&\n \tgrep \"cherry picked from\" picked_msg &&\n \tgrep \"cherry picked from\" anotherpick_msg\n@@ -426,9 +426,9 @@ test_expect_failure '--signoff is automatically propagated to resolved conflict'\n \tgit cat-file commit HEAD~1 >picked_msg &&\n \tgit cat-file commit HEAD~2 >unrelatedpick_msg &&\n \tgit cat-file commit HEAD~3 >initial_msg &&\n-\ttest_must_fail grep \"Signed-off-by:\" initial_msg &&\n+\t! grep \"Signed-off-by:\" initial_msg &&\n \tgrep \"Signed-off-by:\" unrelatedpick_msg &&\n-\ttest_must_fail grep \"Signed-off-by:\" picked_msg &&\n+\t! grep \"Signed-off-by:\" picked_msg &&\n \tgrep \"Signed-off-by:\" anotherpick_msg\n '\n \ndiff --git a/t/t5504-fetch-receive-strict.sh b/t/t5504-fetch-receive-strict.sh\nindex 9b19cff72..49d3621a9 100755\n--- a/t/t5504-fetch-receive-strict.sh\n+++ b/t/t5504-fetch-receive-strict.sh\n@@ -152,7 +152,7 @@ test_expect_success 'push with receive.fsck.missingEmail=warn' '\n \tgit --git-dir=dst/.git config --add \\\n \t\treceive.fsck.badDate warn &&\n \tgit push --porcelain dst bogus >act 2>&1 &&\n-\ttest_must_fail grep \"missingEmail\" act\n+\t! grep \"missingEmail\" act\n '\n \n test_expect_success \\\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 26b2cafc4..0fc5a7c59 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -1004,7 +1004,7 @@ test_expect_success 'push --porcelain' '\n test_expect_success 'push --porcelain bad url' '\n \tmk_empty testrepo &&\n \ttest_must_fail git push >.git/bar --porcelain asdfasdfasd refs/heads/master:refs/remotes/origin/master &&\n-\ttest_must_fail grep -q Done .git/bar\n+\t! grep -q Done .git/bar\n '\n \n test_expect_success 'push --porcelain rejected' '\ndiff --git a/t/t5601-clone.sh b/t/t5601-clone.sh\nindex a43339420..4241ea5b3 100755\n--- a/t/t5601-clone.sh\n+++ b/t/t5601-clone.sh\n@@ -151,7 +151,7 @@ test_expect_success 'clone --mirror does not repeat tags' '\n \tgit clone --mirror src mirror2 &&\n \t(cd mirror2 &&\n \t git show-ref 2> clone.err > clone.out) &&\n-\ttest_must_fail grep Duplicate mirror2/clone.err &&\n+\t! grep Duplicate mirror2/clone.err &&\n \tgrep some-tag mirror2/clone.out\n \n '\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex 5e5370feb..8c2c6eaef 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -407,7 +407,7 @@ test_expect_success 'good merge base when good and bad are siblings' '\n \ttest_i18ngrep \"merge base must be tested\" my_bisect_log.txt &&\n \tgrep $HASH4 my_bisect_log.txt &&\n \tgit bisect good > my_bisect_log.txt &&\n-\ttest_must_fail grep \"merge base must be tested\" my_bisect_log.txt &&\n+\t! grep \"merge base must be tested\" my_bisect_log.txt &&\n \tgrep $HASH6 my_bisect_log.txt &&\n \tgit bisect reset\n '\ndiff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh\nindex 63d36fb28..0fe7e58cf 100755\n--- a/t/t7610-mergetool.sh\n+++ b/t/t7610-mergetool.sh\n@@ -602,7 +602,7 @@ test_expect_success MKTEMP 'temporary filenames are used with mergetool.writeToT\n \ttest_config mergetool.myecho.trustExitCode true &&\n \ttest_must_fail git merge master &&\n \tgit mergetool --no-prompt --tool myecho -- both >actual &&\n-\ttest_must_fail grep ^\\./both_LOCAL_ actual >/dev/null &&\n+\t! grep ^\\./both_LOCAL_ actual >/dev/null &&\n \tgrep /both_LOCAL_ actual >/dev/null &&\n \tgit reset --hard master >/dev/null 2>&1\n '\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 3dc4a3454..0f398dd16 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -50,7 +50,7 @@ test_no_confirm () {\n \t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n \t\t$@ \\\n \t\t$patches >stdout &&\n-\t\ttest_must_fail grep \"Send this email\" stdout &&\n+\t\t! grep \"Send this email\" stdout &&\n \t\t>no_confirm_okay\n }\n \ndiff --git a/t/t9117-git-svn-init-clone.sh b/t/t9117-git-svn-init-clone.sh\nindex 69a675052..044f65e91 100755\n--- a/t/t9117-git-svn-init-clone.sh\n+++ b/t/t9117-git-svn-init-clone.sh\n@@ -55,7 +55,7 @@ test_expect_success 'clone to target directory with --stdlayout' '\n test_expect_success 'init without -s/-T/-b/-t does not warn' '\n \ttest ! -d trunk &&\n \tgit svn init \"$svnrepo\"/project/trunk trunk 2>warning &&\n-\ttest_must_fail grep -q prefix warning &&\n+\t! grep -q prefix warning &&\n \trm -rf trunk &&\n \trm -f warning\n \t'\n@@ -63,7 +63,7 @@ test_expect_success 'init without -s/-T/-b/-t does not warn' '\n test_expect_success 'clone without -s/-T/-b/-t does not warn' '\n \ttest ! -d trunk &&\n \tgit svn clone \"$svnrepo\"/project/trunk 2>warning &&\n-\ttest_must_fail grep -q prefix warning &&\n+\t! grep -q prefix warning &&\n \trm -rf trunk &&\n \trm -f warning\n \t'\n@@ -86,7 +86,7 @@ EOF\n test_expect_success 'init with -s/-T/-b/-t assumes --prefix=origin/' '\n \ttest ! -d project &&\n \tgit svn init -s \"$svnrepo\"/project project 2>warning &&\n-\ttest_must_fail grep -q prefix warning &&\n+\t! grep -q prefix warning &&\n \ttest_svn_configured_prefix \"origin/\" &&\n \trm -rf project &&\n \trm -f warning\n@@ -95,7 +95,7 @@ test_expect_success 'init with -s/-T/-b/-t assumes --prefix=origin/' '\n test_expect_success 'clone with -s/-T/-b/-t assumes --prefix=origin/' '\n \ttest ! -d project &&\n \tgit svn clone -s \"$svnrepo\"/project 2>warning &&\n-\ttest_must_fail grep -q prefix warning &&\n+\t! grep -q prefix warning &&\n \ttest_svn_configured_prefix \"origin/\" &&\n \trm -rf project &&\n \trm -f warning\n@@ -104,7 +104,7 @@ test_expect_success 'clone with -s/-T/-b/-t assumes --prefix=origin/' '\n test_expect_success 'init with -s/-T/-b/-t and --prefix \"\" still works' '\n \ttest ! -d project &&\n \tgit svn init -s \"$svnrepo\"/project project --prefix \"\" 2>warning &&\n-\ttest_must_fail grep -q prefix warning &&\n+\t! grep -q prefix warning &&\n \ttest_svn_configured_prefix \"\" &&\n \trm -rf project &&\n \trm -f warning\n@@ -113,7 +113,7 @@ test_expect_success 'init with -s/-T/-b/-t and --prefix \"\" still works' '\n test_expect_success 'clone with -s/-T/-b/-t and --prefix \"\" still works' '\n \ttest ! -d project &&\n \tgit svn clone -s \"$svnrepo\"/project --prefix \"\" 2>warning &&\n-\ttest_must_fail grep -q prefix warning &&\n+\t! grep -q prefix warning &&\n \ttest_svn_configured_prefix \"\" &&\n \trm -rf project &&\n \trm -f warning\ndiff --git a/t/t9813-git-p4-preserve-users.sh b/t/t9813-git-p4-preserve-users.sh\nindex 0fe231280..2384535a7 100755\n--- a/t/t9813-git-p4-preserve-users.sh\n+++ b/t/t9813-git-p4-preserve-users.sh\n@@ -126,13 +126,13 @@ test_expect_success 'not preserving user with mixed authorship' '\n \t\tgrep \"git author charlie@example.com does not match\" &&\n \n \t\tmake_change_by_user usernamefile3 alice alice@example.com &&\n-\t\tgit p4 commit |\\\n-\t\ttest_must_fail grep \"git author.*does not match\" &&\n+\t\t! git p4 commit |\\\n+\t\tgrep \"git author.*does not match\" &&\n \n \t\tgit config git-p4.skipUserNameCheck true &&\n \t\tmake_change_by_user usernamefile3 Charlie charlie@example.com &&\n-\t\tgit p4 commit |\\\n-\t\ttest_must_fail grep \"git author.*does not match\" &&\n+\t\t! git p4 commit |\\\n+\t\tgrep \"git author.*does not match\" &&\n \n \t\tp4_check_commit_author usernamefile3 alice\n \t)\ndiff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh\nindex c89992cf9..e7e0268e9 100755\n--- a/t/t9814-git-p4-rename.sh\n+++ b/t/t9814-git-p4-rename.sh\n@@ -141,7 +141,7 @@ test_expect_success 'detect copies' '\n \t\tgit diff-tree -r -C HEAD &&\n \t\tgit p4 submit &&\n \t\tp4 filelog //depot/file8 &&\n-\t\tp4 filelog //depot/file8 | test_must_fail grep -q \"branch from\" &&\n+\t\t! p4 filelog //depot/file8 | grep -q \"branch from\" &&\n \n \t\techo \"file9\" >>file2 &&\n \t\tgit commit -a -m \"Differentiate file2\" &&\n@@ -154,7 +154,7 @@ test_expect_success 'detect copies' '\n \t\tgit config git-p4.detectCopies true &&\n \t\tgit p4 submit &&\n \t\tp4 filelog //depot/file9 &&\n-\t\tp4 filelog //depot/file9 | test_must_fail grep -q \"branch from\" &&\n+\t\t! p4 filelog //depot/file9 | grep -q \"branch from\" &&\n \n \t\techo \"file10\" >>file2 &&\n \t\tgit commit -a -m \"Differentiate file2\" &&\n@@ -202,7 +202,7 @@ test_expect_success 'detect copies' '\n \t\tgit config git-p4.detectCopies $(($level + 2)) &&\n \t\tgit p4 submit &&\n \t\tp4 filelog //depot/file12 &&\n-\t\tp4 filelog //depot/file12 | test_must_fail grep -q \"branch from\" &&\n+\t\t! p4 filelog //depot/file12 | grep -q \"branch from\" &&\n \n \t\techo \"file13\" >>file2 &&\n \t\tgit commit -a -m \"Differentiate file2\" &&\n-- \n2.11.0\n\n"},{"id":"308577","messageId":"CAE5ih7-7e+ZLUbE7iquWV2=qP4ofzAHUC2ZPg3b-ivSpCo4eRw@mail.gmail.com","threadId":"44797","inReplyTo":"20161231114412.23439-1-pranit.bauva@gmail.com","subject":"Re: [PATCH] don't use test_must_fail with grep","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2017-01-01T14:23:43Z","receivedAt":"2017-01-01T14:23:52Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 31 December 2016 at 11:44, Pranit Bauva <pranit.bauva@gmail.com> wrote:\n> test_must_fail should only be used for testing git commands. To test the\n> failure of other commands use `!`.\n>\n> Reported-by: Stefan Beller <sbeller@google.com>\n> Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>\n> ---\n>  t/t3510-cherry-pick-sequence.sh  |  6 +++---\n>  t/t5504-fetch-receive-strict.sh  |  2 +-\n>  t/t5516-fetch-push.sh            |  2 +-\n>  t/t5601-clone.sh                 |  2 +-\n>  t/t6030-bisect-porcelain.sh      |  2 +-\n>  t/t7610-mergetool.sh             |  2 +-\n>  t/t9001-send-email.sh            |  2 +-\n>  t/t9117-git-svn-init-clone.sh    | 12 ++++++------\n>  t/t9813-git-p4-preserve-users.sh |  8 ++++----\n>  t/t9814-git-p4-rename.sh         |  6 +++---\n>  10 files changed, 22 insertions(+), 22 deletions(-)\n>\n> diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh\n> index 372307c21..0acf4b146 100755\n> --- a/t/t3510-cherry-pick-sequence.sh\n> +++ b/t/t3510-cherry-pick-sequence.sh\n> @@ -385,7 +385,7 @@ test_expect_success '--continue respects opts' '\n>         git cat-file commit HEAD~1 >picked_msg &&\n>         git cat-file commit HEAD~2 >unrelatedpick_msg &&\n>         git cat-file commit HEAD~3 >initial_msg &&\n> -       test_must_fail grep \"cherry picked from\" initial_msg &&\n> +       ! grep \"cherry picked from\" initial_msg &&\n>         grep \"cherry picked from\" unrelatedpick_msg &&\n>         grep \"cherry picked from\" picked_msg &&\n>         grep \"cherry picked from\" anotherpick_msg\n> @@ -426,9 +426,9 @@ test_expect_failure '--signoff is automatically propagated to resolved conflict'\n>         git cat-file commit HEAD~1 >picked_msg &&\n>         git cat-file commit HEAD~2 >unrelatedpick_msg &&\n>         git cat-file commit HEAD~3 >initial_msg &&\n> -       test_must_fail grep \"Signed-off-by:\" initial_msg &&\n> +       ! grep \"Signed-off-by:\" initial_msg &&\n>         grep \"Signed-off-by:\" unrelatedpick_msg &&\n> -       test_must_fail grep \"Signed-off-by:\" picked_msg &&\n> +       ! grep \"Signed-off-by:\" picked_msg &&\n>         grep \"Signed-off-by:\" anotherpick_msg\n>  '\n>\n> diff --git a/t/t5504-fetch-receive-strict.sh b/t/t5504-fetch-receive-strict.sh\n> index 9b19cff72..49d3621a9 100755\n> --- a/t/t5504-fetch-receive-strict.sh\n> +++ b/t/t5504-fetch-receive-strict.sh\n> @@ -152,7 +152,7 @@ test_expect_success 'push with receive.fsck.missingEmail=warn' '\n>         git --git-dir=dst/.git config --add \\\n>                 receive.fsck.badDate warn &&\n>         git push --porcelain dst bogus >act 2>&1 &&\n> -       test_must_fail grep \"missingEmail\" act\n> +       ! grep \"missingEmail\" act\n>  '\n>\n>  test_expect_success \\\n> diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\n> index 26b2cafc4..0fc5a7c59 100755\n> --- a/t/t5516-fetch-push.sh\n> +++ b/t/t5516-fetch-push.sh\n> @@ -1004,7 +1004,7 @@ test_expect_success 'push --porcelain' '\n>  test_expect_success 'push --porcelain bad url' '\n>         mk_empty testrepo &&\n>         test_must_fail git push >.git/bar --porcelain asdfasdfasd refs/heads/master:refs/remotes/origin/master &&\n> -       test_must_fail grep -q Done .git/bar\n> +       ! grep -q Done .git/bar\n>  '\n>\n>  test_expect_success 'push --porcelain rejected' '\n> diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh\n> index a43339420..4241ea5b3 100755\n> --- a/t/t5601-clone.sh\n> +++ b/t/t5601-clone.sh\n> @@ -151,7 +151,7 @@ test_expect_success 'clone --mirror does not repeat tags' '\n>         git clone --mirror src mirror2 &&\n>         (cd mirror2 &&\n>          git show-ref 2> clone.err > clone.out) &&\n> -       test_must_fail grep Duplicate mirror2/clone.err &&\n> +       ! grep Duplicate mirror2/clone.err &&\n>         grep some-tag mirror2/clone.out\n>\n>  '\n> diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\n> index 5e5370feb..8c2c6eaef 100755\n> --- a/t/t6030-bisect-porcelain.sh\n> +++ b/t/t6030-bisect-porcelain.sh\n> @@ -407,7 +407,7 @@ test_expect_success 'good merge base when good and bad are siblings' '\n>         test_i18ngrep \"merge base must be tested\" my_bisect_log.txt &&\n>         grep $HASH4 my_bisect_log.txt &&\n>         git bisect good > my_bisect_log.txt &&\n> -       test_must_fail grep \"merge base must be tested\" my_bisect_log.txt &&\n> +       ! grep \"merge base must be tested\" my_bisect_log.txt &&\n>         grep $HASH6 my_bisect_log.txt &&\n>         git bisect reset\n>  '\n> diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh\n> index 63d36fb28..0fe7e58cf 100755\n> --- a/t/t7610-mergetool.sh\n> +++ b/t/t7610-mergetool.sh\n> @@ -602,7 +602,7 @@ test_expect_success MKTEMP 'temporary filenames are used with mergetool.writeToT\n>         test_config mergetool.myecho.trustExitCode true &&\n>         test_must_fail git merge master &&\n>         git mergetool --no-prompt --tool myecho -- both >actual &&\n> -       test_must_fail grep ^\\./both_LOCAL_ actual >/dev/null &&\n> +       ! grep ^\\./both_LOCAL_ actual >/dev/null &&\n>         grep /both_LOCAL_ actual >/dev/null &&\n>         git reset --hard master >/dev/null 2>&1\n>  '\n> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\n> index 3dc4a3454..0f398dd16 100755\n> --- a/t/t9001-send-email.sh\n> +++ b/t/t9001-send-email.sh\n> @@ -50,7 +50,7 @@ test_no_confirm () {\n>                 --smtp-server=\"$(pwd)/fake.sendmail\" \\\n>                 $@ \\\n>                 $patches >stdout &&\n> -               test_must_fail grep \"Send this email\" stdout &&\n> +               ! grep \"Send this email\" stdout &&\n>                 >no_confirm_okay\n>  }\n>\n> diff --git a/t/t9117-git-svn-init-clone.sh b/t/t9117-git-svn-init-clone.sh\n> index 69a675052..044f65e91 100755\n> --- a/t/t9117-git-svn-init-clone.sh\n> +++ b/t/t9117-git-svn-init-clone.sh\n> @@ -55,7 +55,7 @@ test_expect_success 'clone to target directory with --stdlayout' '\n>  test_expect_success 'init without -s/-T/-b/-t does not warn' '\n>         test ! -d trunk &&\n>         git svn init \"$svnrepo\"/project/trunk trunk 2>warning &&\n> -       test_must_fail grep -q prefix warning &&\n> +       ! grep -q prefix warning &&\n>         rm -rf trunk &&\n>         rm -f warning\n>         '\n> @@ -63,7 +63,7 @@ test_expect_success 'init without -s/-T/-b/-t does not warn' '\n>  test_expect_success 'clone without -s/-T/-b/-t does not warn' '\n>         test ! -d trunk &&\n>         git svn clone \"$svnrepo\"/project/trunk 2>warning &&\n> -       test_must_fail grep -q prefix warning &&\n> +       ! grep -q prefix warning &&\n>         rm -rf trunk &&\n>         rm -f warning\n>         '\n> @@ -86,7 +86,7 @@ EOF\n>  test_expect_success 'init with -s/-T/-b/-t assumes --prefix=origin/' '\n>         test ! -d project &&\n>         git svn init -s \"$svnrepo\"/project project 2>warning &&\n> -       test_must_fail grep -q prefix warning &&\n> +       ! grep -q prefix warning &&\n>         test_svn_configured_prefix \"origin/\" &&\n>         rm -rf project &&\n>         rm -f warning\n> @@ -95,7 +95,7 @@ test_expect_success 'init with -s/-T/-b/-t assumes --prefix=origin/' '\n>  test_expect_success 'clone with -s/-T/-b/-t assumes --prefix=origin/' '\n>         test ! -d project &&\n>         git svn clone -s \"$svnrepo\"/project 2>warning &&\n> -       test_must_fail grep -q prefix warning &&\n> +       ! grep -q prefix warning &&\n>         test_svn_configured_prefix \"origin/\" &&\n>         rm -rf project &&\n>         rm -f warning\n> @@ -104,7 +104,7 @@ test_expect_success 'clone with -s/-T/-b/-t assumes --prefix=origin/' '\n>  test_expect_success 'init with -s/-T/-b/-t and --prefix \"\" still works' '\n>         test ! -d project &&\n>         git svn init -s \"$svnrepo\"/project project --prefix \"\" 2>warning &&\n> -       test_must_fail grep -q prefix warning &&\n> +       ! grep -q prefix warning &&\n>         test_svn_configured_prefix \"\" &&\n>         rm -rf project &&\n>         rm -f warning\n> @@ -113,7 +113,7 @@ test_expect_success 'init with -s/-T/-b/-t and --prefix \"\" still works' '\n>  test_expect_success 'clone with -s/-T/-b/-t and --prefix \"\" still works' '\n>         test ! -d project &&\n>         git svn clone -s \"$svnrepo\"/project --prefix \"\" 2>warning &&\n> -       test_must_fail grep -q prefix warning &&\n> +       ! grep -q prefix warning &&\n>         test_svn_configured_prefix \"\" &&\n>         rm -rf project &&\n>         rm -f warning\n> diff --git a/t/t9813-git-p4-preserve-users.sh b/t/t9813-git-p4-preserve-users.sh\n> index 0fe231280..2384535a7 100755\n> --- a/t/t9813-git-p4-preserve-users.sh\n> +++ b/t/t9813-git-p4-preserve-users.sh\n> @@ -126,13 +126,13 @@ test_expect_success 'not preserving user with mixed authorship' '\n>                 grep \"git author charlie@example.com does not match\" &&\n>\n>                 make_change_by_user usernamefile3 alice alice@example.com &&\n> -               git p4 commit |\\\n> -               test_must_fail grep \"git author.*does not match\" &&\n> +               ! git p4 commit |\\\n> +               grep \"git author.*does not match\" &&\n\nWould it be clearer to use this?\n\n    git p4 commit |\\\n    grep -q -v \"git author.*does not match\" &&\n\nWith your original change, I think that if \"git p4 commit\" fails, then\nthat expression will be treated as a pass. What we want is for \"git p4\ncommit\" to pass, but the string to be missing.\n\n(I would have used \"--invert-match\" rather than \"-v\", but it seems\nthat's not supported on Solaris).\n\nLuke\n"},{"id":"308578","messageId":"285ed013-5c59-0b98-7dc0-8f729587a313@kdbg.org","threadId":"44797","inReplyTo":"CAE5ih7-7e+ZLUbE7iquWV2=qP4ofzAHUC2ZPg3b-ivSpCo4eRw@mail.gmail.com","subject":"Re: [PATCH] don't use test_must_fail with grep","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2017-01-01T14:50:38Z","receivedAt":"2017-01-01T14:50:49Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 01.01.2017 um 15:23 schrieb Luke Diamand:\n> On 31 December 2016 at 11:44, Pranit Bauva <pranit.bauva@gmail.com> wrote:\n>> diff --git a/t/t9813-git-p4-preserve-users.sh b/t/t9813-git-p4-preserve-users.sh\n>> index 0fe231280..2384535a7 100755\n>> --- a/t/t9813-git-p4-preserve-users.sh\n>> +++ b/t/t9813-git-p4-preserve-users.sh\n>> @@ -126,13 +126,13 @@ test_expect_success 'not preserving user with mixed authorship' '\n>>                 grep \"git author charlie@example.com does not match\" &&\n>>\n>>                 make_change_by_user usernamefile3 alice alice@example.com &&\n>> -               git p4 commit |\\\n>> -               test_must_fail grep \"git author.*does not match\" &&\n>> +               ! git p4 commit |\\\n>> +               grep \"git author.*does not match\" &&\n>\n> Would it be clearer to use this?\n>\n>     git p4 commit |\\\n>     grep -q -v \"git author.*does not match\" &&\n>\n> With your original change, I think that if \"git p4 commit\" fails, then\n> that expression will be treated as a pass.\n\nNo. The exit code of the upstream in a pipe is ignored. For this reason, \nhaving a git invocation as the upstream of a pipe *anywhere* in the test \nsuite is frowned upon. Hence, a better rewrite would be\n\n\tgit p4 commit >actual &&\n\t! grep \"git author.*does not match\" actual &&\n\nwhich makes me wonder: Is the message that we do expect not to occur \nactually printed on stdout? It sounds much more like an error message, \ni.e., text that is printed on stderr. Wouldn't we need this?\n\n\tgit p4 commit >actual 2>&1 &&\n\t! grep \"git author.*does not match\" actual &&\n\n-- Hannes\n\n"},{"id":"308579","messageId":"CAE5ih7-b7LpPYPkuDnJakb12LPZ5UE2TeV17aYXAsbP2aH5zEA@mail.gmail.com","threadId":"44797","inReplyTo":"285ed013-5c59-0b98-7dc0-8f729587a313@kdbg.org","subject":"Re: [PATCH] don't use test_must_fail with grep","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2017-01-01T15:24:49Z","receivedAt":"2017-01-01T15:24:56Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 1 January 2017 at 14:50, Johannes Sixt <j6t@kdbg.org> wrote:\n> Am 01.01.2017 um 15:23 schrieb Luke Diamand:\n>>\n>> On 31 December 2016 at 11:44, Pranit Bauva <pranit.bauva@gmail.com> wrote:\n>>>\n>>> diff --git a/t/t9813-git-p4-preserve-users.sh\n>>> b/t/t9813-git-p4-preserve-users.sh\n>>> index 0fe231280..2384535a7 100755\n>>> --- a/t/t9813-git-p4-preserve-users.sh\n>>> +++ b/t/t9813-git-p4-preserve-users.sh\n>>> @@ -126,13 +126,13 @@ test_expect_success 'not preserving user with mixed\n>>> authorship' '\n>>>                 grep \"git author charlie@example.com does not match\" &&\n>>>\n>>>                 make_change_by_user usernamefile3 alice alice@example.com\n>>> &&\n>>> -               git p4 commit |\\\n>>> -               test_must_fail grep \"git author.*does not match\" &&\n>>> +               ! git p4 commit |\\\n>>> +               grep \"git author.*does not match\" &&\n>>\n>>\n>> Would it be clearer to use this?\n>>\n>>     git p4 commit |\\\n>>     grep -q -v \"git author.*does not match\" &&\n>>\n>> With your original change, I think that if \"git p4 commit\" fails, then\n>> that expression will be treated as a pass.\n>\n>\n> No. The exit code of the upstream in a pipe is ignored. For this reason,\n> having a git invocation as the upstream of a pipe *anywhere* in the test\n> suite is frowned upon. Hence, a better rewrite would be\n>\n>         git p4 commit >actual &&\n>         ! grep \"git author.*does not match\" actual &&\n>\n> which makes me wonder: Is the message that we do expect not to occur\n> actually printed on stdout? It sounds much more like an error message, i.e.,\n> text that is printed on stderr. Wouldn't we need this?\n>\n>         git p4 commit >actual 2>&1 &&\n>         ! grep \"git author.*does not match\" actual &&\n\nThe message is actually part of a template presented to the user via\ntheir chosen editor. For this test, we set the editor to be \"cat\", so\nit comes out on stdout.\n\nYour first suggestion would therefore be fine (and similarly for the\nother cases).\n"},{"id":"308620","messageId":"CAFZEwPNbtamFfFy7vYXurpEWBDmRMyPB9+Ep-hm4uZVMREbq5Q@mail.gmail.com","threadId":"44797","inReplyTo":"285ed013-5c59-0b98-7dc0-8f729587a313@kdbg.org","subject":"Re: [PATCH] don't use test_must_fail with grep","fromName":"Pranit Bauva","fromEmail":"pranit.bauva@gmail.com","sentAt":"2017-01-02T13:40:12Z","receivedAt":"2017-01-02T13:40:18Z","isPatch":true,"sender":{"key":"pranit.bauva@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2959938?v=4"},"body":"Hey Johannes,\n\nOn Sun, Jan 1, 2017 at 8:20 PM, Johannes Sixt <j6t@kdbg.org> wrote:\n> which makes me wonder: Is the message that we do expect not to occur\n> actually printed on stdout? It sounds much more like an error message, i.e.,\n> text that is printed on stderr. Wouldn't we need this?\n>\n>         git p4 commit >actual 2>&1 &&\n>         ! grep \"git author.*does not match\" actual &&\n>\n> -- Hannes\n\nThis seems better! Since I am at it, I can remove the traces of pipes\nin an another patch.\n\nRegards,\nPranit Bauva\n"},{"id":"308688","messageId":"20170102184536.10488-1-pranit.bauva@gmail.com","threadId":"44797","inReplyTo":"20161231114412.23439-1-pranit.bauva@gmail.com","subject":"[PATCH v2 1/2] don't use test_must_fail with grep","fromName":"Pranit Bauva","fromEmail":"pranit.bauva@gmail.com","sentAt":"2017-01-02T18:45:35Z","receivedAt":"2017-01-02T18:47:02Z","isPatch":true,"sender":{"key":"pranit.bauva@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2959938?v=4"},"body":"test_must_fail should only be used for testing git commands. To test the\nfailure of other commands use `!`.\n\nReported-by: Stefan Beller <sbeller@google.com>\nSigned-off-by: Pranit Bauva <pranit.bauva@gmail.com>\n---\n t/t3510-cherry-pick-sequence.sh  |  6 +++---\n t/t5504-fetch-receive-strict.sh  |  2 +-\n t/t5516-fetch-push.sh            |  2 +-\n t/t5601-clone.sh                 |  2 +-\n t/t6030-bisect-porcelain.sh      |  2 +-\n t/t7610-mergetool.sh             |  2 +-\n t/t9001-send-email.sh            |  2 +-\n t/t9117-git-svn-init-clone.sh    | 12 ++++++------\n t/t9813-git-p4-preserve-users.sh |  8 ++++----\n t/t9814-git-p4-rename.sh         |  6 +++---\n 10 files changed, 22 insertions(+), 22 deletions(-)\n\ndiff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh\nindex 372307c21..0acf4b146 100755\n--- a/t/t3510-cherry-pick-sequence.sh\n+++ b/t/t3510-cherry-pick-sequence.sh\n@@ -385,7 +385,7 @@ test_expect_success '--continue respects opts' '\n \tgit cat-file commit HEAD~1 >picked_msg &&\n \tgit cat-file commit HEAD~2 >unrelatedpick_msg &&\n \tgit cat-file commit HEAD~3 >initial_msg &&\n-\ttest_must_fail grep \"cherry picked from\" initial_msg &&\n+\t! grep \"cherry picked from\" initial_msg &&\n \tgrep \"cherry picked from\" unrelatedpick_msg &&\n \tgrep \"cherry picked from\" picked_msg &&\n \tgrep \"cherry picked from\" anotherpick_msg\n@@ -426,9 +426,9 @@ test_expect_failure '--signoff is automatically propagated to resolved conflict'\n \tgit cat-file commit HEAD~1 >picked_msg &&\n \tgit cat-file commit HEAD~2 >unrelatedpick_msg &&\n \tgit cat-file commit HEAD~3 >initial_msg &&\n-\ttest_must_fail grep \"Signed-off-by:\" initial_msg &&\n+\t! grep \"Signed-off-by:\" initial_msg &&\n \tgrep \"Signed-off-by:\" unrelatedpick_msg &&\n-\ttest_must_fail grep \"Signed-off-by:\" picked_msg &&\n+\t! grep \"Signed-off-by:\" picked_msg &&\n \tgrep \"Signed-off-by:\" anotherpick_msg\n '\n \ndiff --git a/t/t5504-fetch-receive-strict.sh b/t/t5504-fetch-receive-strict.sh\nindex 9b19cff72..49d3621a9 100755\n--- a/t/t5504-fetch-receive-strict.sh\n+++ b/t/t5504-fetch-receive-strict.sh\n@@ -152,7 +152,7 @@ test_expect_success 'push with receive.fsck.missingEmail=warn' '\n \tgit --git-dir=dst/.git config --add \\\n \t\treceive.fsck.badDate warn &&\n \tgit push --porcelain dst bogus >act 2>&1 &&\n-\ttest_must_fail grep \"missingEmail\" act\n+\t! grep \"missingEmail\" act\n '\n \n test_expect_success \\\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 26b2cafc4..0fc5a7c59 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -1004,7 +1004,7 @@ test_expect_success 'push --porcelain' '\n test_expect_success 'push --porcelain bad url' '\n \tmk_empty testrepo &&\n \ttest_must_fail git push >.git/bar --porcelain asdfasdfasd refs/heads/master:refs/remotes/origin/master &&\n-\ttest_must_fail grep -q Done .git/bar\n+\t! grep -q Done .git/bar\n '\n \n test_expect_success 'push --porcelain rejected' '\ndiff --git a/t/t5601-clone.sh b/t/t5601-clone.sh\nindex a43339420..4241ea5b3 100755\n--- a/t/t5601-clone.sh\n+++ b/t/t5601-clone.sh\n@@ -151,7 +151,7 @@ test_expect_success 'clone --mirror does not repeat tags' '\n \tgit clone --mirror src mirror2 &&\n \t(cd mirror2 &&\n \t git show-ref 2> clone.err > clone.out) &&\n-\ttest_must_fail grep Duplicate mirror2/clone.err &&\n+\t! grep Duplicate mirror2/clone.err &&\n \tgrep some-tag mirror2/clone.out\n \n '\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex 5e5370feb..8c2c6eaef 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -407,7 +407,7 @@ test_expect_success 'good merge base when good and bad are siblings' '\n \ttest_i18ngrep \"merge base must be tested\" my_bisect_log.txt &&\n \tgrep $HASH4 my_bisect_log.txt &&\n \tgit bisect good > my_bisect_log.txt &&\n-\ttest_must_fail grep \"merge base must be tested\" my_bisect_log.txt &&\n+\t! grep \"merge base must be tested\" my_bisect_log.txt &&\n \tgrep $HASH6 my_bisect_log.txt &&\n \tgit bisect reset\n '\ndiff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh\nindex 63d36fb28..0fe7e58cf 100755\n--- a/t/t7610-mergetool.sh\n+++ b/t/t7610-mergetool.sh\n@@ -602,7 +602,7 @@ test_expect_success MKTEMP 'temporary filenames are used with mergetool.writeToT\n \ttest_config mergetool.myecho.trustExitCode true &&\n \ttest_must_fail git merge master &&\n \tgit mergetool --no-prompt --tool myecho -- both >actual &&\n-\ttest_must_fail grep ^\\./both_LOCAL_ actual >/dev/null &&\n+\t! grep ^\\./both_LOCAL_ actual >/dev/null &&\n \tgrep /both_LOCAL_ actual >/dev/null &&\n \tgit reset --hard master >/dev/null 2>&1\n '\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 3dc4a3454..0f398dd16 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -50,7 +50,7 @@ test_no_confirm () {\n \t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n \t\t$@ \\\n \t\t$patches >stdout &&\n-\t\ttest_must_fail grep \"Send this email\" stdout &&\n+\t\t! grep \"Send this email\" stdout &&\n \t\t>no_confirm_okay\n }\n \ndiff --git a/t/t9117-git-svn-init-clone.sh b/t/t9117-git-svn-init-clone.sh\nindex 69a675052..044f65e91 100755\n--- a/t/t9117-git-svn-init-clone.sh\n+++ b/t/t9117-git-svn-init-clone.sh\n@@ -55,7 +55,7 @@ test_expect_success 'clone to target directory with --stdlayout' '\n test_expect_success 'init without -s/-T/-b/-t does not warn' '\n \ttest ! -d trunk &&\n \tgit svn init \"$svnrepo\"/project/trunk trunk 2>warning &&\n-\ttest_must_fail grep -q prefix warning &&\n+\t! grep -q prefix warning &&\n \trm -rf trunk &&\n \trm -f warning\n \t'\n@@ -63,7 +63,7 @@ test_expect_success 'init without -s/-T/-b/-t does not warn' '\n test_expect_success 'clone without -s/-T/-b/-t does not warn' '\n \ttest ! -d trunk &&\n \tgit svn clone \"$svnrepo\"/project/trunk 2>warning &&\n-\ttest_must_fail grep -q prefix warning &&\n+\t! grep -q prefix warning &&\n \trm -rf trunk &&\n \trm -f warning\n \t'\n@@ -86,7 +86,7 @@ EOF\n test_expect_success 'init with -s/-T/-b/-t assumes --prefix=origin/' '\n \ttest ! -d project &&\n \tgit svn init -s \"$svnrepo\"/project project 2>warning &&\n-\ttest_must_fail grep -q prefix warning &&\n+\t! grep -q prefix warning &&\n \ttest_svn_configured_prefix \"origin/\" &&\n \trm -rf project &&\n \trm -f warning\n@@ -95,7 +95,7 @@ test_expect_success 'init with -s/-T/-b/-t assumes --prefix=origin/' '\n test_expect_success 'clone with -s/-T/-b/-t assumes --prefix=origin/' '\n \ttest ! -d project &&\n \tgit svn clone -s \"$svnrepo\"/project 2>warning &&\n-\ttest_must_fail grep -q prefix warning &&\n+\t! grep -q prefix warning &&\n \ttest_svn_configured_prefix \"origin/\" &&\n \trm -rf project &&\n \trm -f warning\n@@ -104,7 +104,7 @@ test_expect_success 'clone with -s/-T/-b/-t assumes --prefix=origin/' '\n test_expect_success 'init with -s/-T/-b/-t and --prefix \"\" still works' '\n \ttest ! -d project &&\n \tgit svn init -s \"$svnrepo\"/project project --prefix \"\" 2>warning &&\n-\ttest_must_fail grep -q prefix warning &&\n+\t! grep -q prefix warning &&\n \ttest_svn_configured_prefix \"\" &&\n \trm -rf project &&\n \trm -f warning\n@@ -113,7 +113,7 @@ test_expect_success 'init with -s/-T/-b/-t and --prefix \"\" still works' '\n test_expect_success 'clone with -s/-T/-b/-t and --prefix \"\" still works' '\n \ttest ! -d project &&\n \tgit svn clone -s \"$svnrepo\"/project --prefix \"\" 2>warning &&\n-\ttest_must_fail grep -q prefix warning &&\n+\t! grep -q prefix warning &&\n \ttest_svn_configured_prefix \"\" &&\n \trm -rf project &&\n \trm -f warning\ndiff --git a/t/t9813-git-p4-preserve-users.sh b/t/t9813-git-p4-preserve-users.sh\nindex 0fe231280..798bf2b67 100755\n--- a/t/t9813-git-p4-preserve-users.sh\n+++ b/t/t9813-git-p4-preserve-users.sh\n@@ -126,13 +126,13 @@ test_expect_success 'not preserving user with mixed authorship' '\n \t\tgrep \"git author charlie@example.com does not match\" &&\n \n \t\tmake_change_by_user usernamefile3 alice alice@example.com &&\n-\t\tgit p4 commit |\\\n-\t\ttest_must_fail grep \"git author.*does not match\" &&\n+\t\tgit p4 commit >actual 2>&1 &&\n+\t\t! grep \"git author.*does not match\" actual &&\n \n \t\tgit config git-p4.skipUserNameCheck true &&\n \t\tmake_change_by_user usernamefile3 Charlie charlie@example.com &&\n-\t\tgit p4 commit |\\\n-\t\ttest_must_fail grep \"git author.*does not match\" &&\n+\t\tgit p4 commit >actual 2>&1 &&\n+\t\t! grep \"git author.*does not match\" actual &&\n \n \t\tp4_check_commit_author usernamefile3 alice\n \t)\ndiff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh\nindex c89992cf9..e7e0268e9 100755\n--- a/t/t9814-git-p4-rename.sh\n+++ b/t/t9814-git-p4-rename.sh\n@@ -141,7 +141,7 @@ test_expect_success 'detect copies' '\n \t\tgit diff-tree -r -C HEAD &&\n \t\tgit p4 submit &&\n \t\tp4 filelog //depot/file8 &&\n-\t\tp4 filelog //depot/file8 | test_must_fail grep -q \"branch from\" &&\n+\t\t! p4 filelog //depot/file8 | grep -q \"branch from\" &&\n \n \t\techo \"file9\" >>file2 &&\n \t\tgit commit -a -m \"Differentiate file2\" &&\n@@ -154,7 +154,7 @@ test_expect_success 'detect copies' '\n \t\tgit config git-p4.detectCopies true &&\n \t\tgit p4 submit &&\n \t\tp4 filelog //depot/file9 &&\n-\t\tp4 filelog //depot/file9 | test_must_fail grep -q \"branch from\" &&\n+\t\t! p4 filelog //depot/file9 | grep -q \"branch from\" &&\n \n \t\techo \"file10\" >>file2 &&\n \t\tgit commit -a -m \"Differentiate file2\" &&\n@@ -202,7 +202,7 @@ test_expect_success 'detect copies' '\n \t\tgit config git-p4.detectCopies $(($level + 2)) &&\n \t\tgit p4 submit &&\n \t\tp4 filelog //depot/file12 &&\n-\t\tp4 filelog //depot/file12 | test_must_fail grep -q \"branch from\" &&\n+\t\t! p4 filelog //depot/file12 | grep -q \"branch from\" &&\n \n \t\techo \"file13\" >>file2 &&\n \t\tgit commit -a -m \"Differentiate file2\" &&\n-- \n2.11.0\n\n"},{"id":"308689","messageId":"20170102184536.10488-2-pranit.bauva@gmail.com","threadId":"44797","inReplyTo":"20170102184536.10488-1-pranit.bauva@gmail.com","subject":"[PATCH v2 2/2] t9813: avoid using pipes","fromName":"Pranit Bauva","fromEmail":"pranit.bauva@gmail.com","sentAt":"2017-01-02T18:45:36Z","receivedAt":"2017-01-02T18:47:06Z","isPatch":true,"sender":{"key":"pranit.bauva@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2959938?v=4"},"body":"The exit code of the upstream in a pipe is ignored thus we should avoid\nusing it. By writing out the output of the git command to a file, we can\ntest the exit codes of both the commands.\n\nSigned-off-by: Pranit Bauva <pranit.bauva@gmail.com>\n---\n t/t9813-git-p4-preserve-users.sh | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t9813-git-p4-preserve-users.sh b/t/t9813-git-p4-preserve-users.sh\nindex 798bf2b67..9d7550ff3 100755\n--- a/t/t9813-git-p4-preserve-users.sh\n+++ b/t/t9813-git-p4-preserve-users.sh\n@@ -118,12 +118,12 @@ test_expect_success 'not preserving user with mixed authorship' '\n \t\tmake_change_by_user usernamefile3 Derek derek@example.com &&\n \t\tP4EDITOR=cat P4USER=alice P4PASSWD=secret &&\n \t\texport P4EDITOR P4USER P4PASSWD &&\n-\t\tgit p4 commit |\\\n-\t\tgrep \"git author derek@example.com does not match\" &&\n+\t\tgit p4 commit >actual 2>&1 &&\n+\t\tgrep \"git author derek@example.com does not match\" actual &&\n \n \t\tmake_change_by_user usernamefile3 Charlie charlie@example.com &&\n-\t\tgit p4 commit |\\\n-\t\tgrep \"git author charlie@example.com does not match\" &&\n+\t\tgit p4 commit >actual 2>&1 &&\n+\t\tgrep \"git author charlie@example.com does not match\" actual &&\n \n \t\tmake_change_by_user usernamefile3 alice alice@example.com &&\n \t\tgit p4 commit >actual 2>&1 &&\n-- \n2.11.0\n\n"},{"id":"308706","messageId":"CAGZ79kYbJuujDpT-3Dj6AFkZUaKUrj8NJ=WgBbT1YbgwTibc=Q@mail.gmail.com","threadId":"44797","inReplyTo":"20161231114412.23439-1-pranit.bauva@gmail.com","subject":"Re: [PATCH] don't use test_must_fail with grep","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-01-03T17:52:35Z","receivedAt":"2017-01-03T17:52:42Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Sat, Dec 31, 2016 at 3:44 AM, Pranit Bauva <pranit.bauva@gmail.com> wrote:\n> test_must_fail should only be used for testing git commands. To test the\n> failure of other commands use `!`.\n>\n> Reported-by: Stefan Beller <sbeller@google.com>\n> Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>\n\nThanks for writing up such a patch!\nI had put it on my todo list, but you\nwere faster on actually going through.\n\nThanks,\nStefan\n"},{"id":"308708","messageId":"CAGZ79kZRFLzD7wcAnFvke9vBxxTAgE7=Ud7F_O95EfkWqz=LJw@mail.gmail.com","threadId":"44797","inReplyTo":"20170102184536.10488-2-pranit.bauva@gmail.com","subject":"Re: [PATCH v2 2/2] t9813: avoid using pipes","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-01-03T17:58:40Z","receivedAt":"2017-01-03T17:59:04Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Jan 2, 2017 at 10:45 AM, Pranit Bauva <pranit.bauva@gmail.com> wrote:\n> The exit code of the upstream in a pipe is ignored thus we should avoid\n> using it.\n\nfor commands under test, i.e. git things. Other parts can be piped if that makes\nthe test easier. Though I guess that can be guessed by the reader as well,\nas you only convert git commands on upstream pipes.\n\n> By writing out the output of the git command to a file, we can\n> test the exit codes of both the commands.\n>\n> Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>\n\nThanks for taking ownership of this issue as well. :)\n\n> ---\n>  t/t9813-git-p4-preserve-users.sh | 8 ++++----\n>  1 file changed, 4 insertions(+), 4 deletions(-)\n>\n> diff --git a/t/t9813-git-p4-preserve-users.sh b/t/t9813-git-p4-preserve-users.sh\n> index 798bf2b67..9d7550ff3 100755\n> --- a/t/t9813-git-p4-preserve-users.sh\n> +++ b/t/t9813-git-p4-preserve-users.sh\n> @@ -118,12 +118,12 @@ test_expect_success 'not preserving user with mixed authorship' '\n>                 make_change_by_user usernamefile3 Derek derek@example.com &&\n>                 P4EDITOR=cat P4USER=alice P4PASSWD=secret &&\n>                 export P4EDITOR P4USER P4PASSWD &&\n> -               git p4 commit |\\\n> -               grep \"git author derek@example.com does not match\" &&\n> +               git p4 commit >actual 2>&1 &&\n\nWhy do we need to pipe 2>&1 here?\nOriginally the piping only fed the stdout to grep, so this patch changes the\ntest? Maybe\n\n    2>actual.err &&\n    test_must_be_empty actual.err\n\ninstead?\n\nThanks,\nStefan\n"},{"id":"308738","messageId":"CAFZEwPPfE_WSn2QbmER+5mkaC8RnVDs5gsSJE+Y0v-CfYaZB2w@mail.gmail.com","threadId":"44797","inReplyTo":"CAGZ79kZRFLzD7wcAnFvke9vBxxTAgE7=Ud7F_O95EfkWqz=LJw@mail.gmail.com","subject":"Re: [PATCH v2 2/2] t9813: avoid using pipes","fromName":"Pranit Bauva","fromEmail":"pranit.bauva@gmail.com","sentAt":"2017-01-03T19:44:56Z","receivedAt":"2017-01-03T19:45:02Z","isPatch":true,"sender":{"key":"pranit.bauva@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2959938?v=4"},"body":"Hey Stefan,\n\nOn Tue, Jan 3, 2017 at 11:28 PM, Stefan Beller <sbeller@google.com> wrote:\n> On Mon, Jan 2, 2017 at 10:45 AM, Pranit Bauva <pranit.bauva@gmail.com> wrote:\n>> The exit code of the upstream in a pipe is ignored thus we should avoid\n>> using it.\n>\n> for commands under test, i.e. git things. Other parts can be piped if that makes\n> the test easier. Though I guess that can be guessed by the reader as well,\n> as you only convert git commands on upstream pipes.\n>\n>> By writing out the output of the git command to a file, we can\n>> test the exit codes of both the commands.\n>>\n>> Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>\n>\n> Thanks for taking ownership of this issue as well. :)\n\nWelcome! ;)\n\n>> ---\n>>  t/t9813-git-p4-preserve-users.sh | 8 ++++----\n>>  1 file changed, 4 insertions(+), 4 deletions(-)\n>>\n>> diff --git a/t/t9813-git-p4-preserve-users.sh b/t/t9813-git-p4-preserve-users.sh\n>> index 798bf2b67..9d7550ff3 100755\n>> --- a/t/t9813-git-p4-preserve-users.sh\n>> +++ b/t/t9813-git-p4-preserve-users.sh\n>> @@ -118,12 +118,12 @@ test_expect_success 'not preserving user with mixed authorship' '\n>>                 make_change_by_user usernamefile3 Derek derek@example.com &&\n>>                 P4EDITOR=cat P4USER=alice P4PASSWD=secret &&\n>>                 export P4EDITOR P4USER P4PASSWD &&\n>> -               git p4 commit |\\\n>> -               grep \"git author derek@example.com does not match\" &&\n>> +               git p4 commit >actual 2>&1 &&\n>\n> Why do we need to pipe 2>&1 here?\n> Originally the piping only fed the stdout to grep, so this patch changes the\n> test? Maybe\n>\n>     2>actual.err &&\n>     test_must_be_empty actual.err\n>\n> instead?\n\nI tried this out but it seems that travis-ci build fails[1]. And I\ndon't have p4 on my machine to test what's happening actually. But I\njust pushed out a few thing modifications to travis and it seems that\nactual.err isn't really empty for some reason. So I think, I just\nleave it as,\n\ngit p4 commit >actual &&\ngrep \"git author derek@example.com does not match\" actual &&\n\nWhat do you think?\n\n[1]: https://travis-ci.org/pranitbauva1997/git/jobs/188633734\n\nRegards,\nPranit Bauva\n"},{"id":"308740","messageId":"CAGZ79kYS9QGea57R5PnbXWdUnHH+=vw+E4_=eqUPd1LoW_7r0A@mail.gmail.com","threadId":"44797","inReplyTo":"CAFZEwPPfE_WSn2QbmER+5mkaC8RnVDs5gsSJE+Y0v-CfYaZB2w@mail.gmail.com","subject":"Re: [PATCH v2 2/2] t9813: avoid using pipes","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-01-03T19:48:35Z","receivedAt":"2017-01-03T19:49:00Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":">\n> git p4 commit >actual &&\n> grep \"git author derek@example.com does not match\" actual &&\n>\n> What do you think?\n\nFrom the travis logs:\n\n    'actual.err' is not empty, it contains:\n    ... - file(s) up-to-date.\n\nI think(/hope) such a progress is tested for at another test,\nand not relevant here so I'd think the proposed\n\n    git p4 commit >actual &&\n    grep \"git author derek@example.com does not match\" actual &&\n\nis fine here.\n\nThanks,\nStefan\n"},{"id":"308741","messageId":"20170103195708.15157-1-pranit.bauva@gmail.com","threadId":"44797","inReplyTo":"20170102184536.10488-1-pranit.bauva@gmail.com","subject":"[PATCH v3 1/2] don't use test_must_fail with grep","fromName":"Pranit Bauva","fromEmail":"pranit.bauva@gmail.com","sentAt":"2017-01-03T19:57:07Z","receivedAt":"2017-01-03T19:57:34Z","isPatch":true,"sender":{"key":"pranit.bauva@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2959938?v=4"},"body":"test_must_fail should only be used for testing git commands. To test the\nfailure of other commands use `!`.\n\nReported-by: Stefan Beller <sbeller@google.com>\nSigned-off-by: Pranit Bauva <pranit.bauva@gmail.com>\n---\n t/t3510-cherry-pick-sequence.sh  |  6 +++---\n t/t5504-fetch-receive-strict.sh  |  2 +-\n t/t5516-fetch-push.sh            |  2 +-\n t/t5601-clone.sh                 |  2 +-\n t/t6030-bisect-porcelain.sh      |  2 +-\n t/t7610-mergetool.sh             |  2 +-\n t/t9001-send-email.sh            |  2 +-\n t/t9117-git-svn-init-clone.sh    | 12 ++++++------\n t/t9813-git-p4-preserve-users.sh |  8 ++++----\n t/t9814-git-p4-rename.sh         |  6 +++---\n 10 files changed, 22 insertions(+), 22 deletions(-)\n\ndiff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh\nindex 372307c21..0acf4b146 100755\n--- a/t/t3510-cherry-pick-sequence.sh\n+++ b/t/t3510-cherry-pick-sequence.sh\n@@ -385,7 +385,7 @@ test_expect_success '--continue respects opts' '\n \tgit cat-file commit HEAD~1 >picked_msg &&\n \tgit cat-file commit HEAD~2 >unrelatedpick_msg &&\n \tgit cat-file commit HEAD~3 >initial_msg &&\n-\ttest_must_fail grep \"cherry picked from\" initial_msg &&\n+\t! grep \"cherry picked from\" initial_msg &&\n \tgrep \"cherry picked from\" unrelatedpick_msg &&\n \tgrep \"cherry picked from\" picked_msg &&\n \tgrep \"cherry picked from\" anotherpick_msg\n@@ -426,9 +426,9 @@ test_expect_failure '--signoff is automatically propagated to resolved conflict'\n \tgit cat-file commit HEAD~1 >picked_msg &&\n \tgit cat-file commit HEAD~2 >unrelatedpick_msg &&\n \tgit cat-file commit HEAD~3 >initial_msg &&\n-\ttest_must_fail grep \"Signed-off-by:\" initial_msg &&\n+\t! grep \"Signed-off-by:\" initial_msg &&\n \tgrep \"Signed-off-by:\" unrelatedpick_msg &&\n-\ttest_must_fail grep \"Signed-off-by:\" picked_msg &&\n+\t! grep \"Signed-off-by:\" picked_msg &&\n \tgrep \"Signed-off-by:\" anotherpick_msg\n '\n \ndiff --git a/t/t5504-fetch-receive-strict.sh b/t/t5504-fetch-receive-strict.sh\nindex 9b19cff72..49d3621a9 100755\n--- a/t/t5504-fetch-receive-strict.sh\n+++ b/t/t5504-fetch-receive-strict.sh\n@@ -152,7 +152,7 @@ test_expect_success 'push with receive.fsck.missingEmail=warn' '\n \tgit --git-dir=dst/.git config --add \\\n \t\treceive.fsck.badDate warn &&\n \tgit push --porcelain dst bogus >act 2>&1 &&\n-\ttest_must_fail grep \"missingEmail\" act\n+\t! grep \"missingEmail\" act\n '\n \n test_expect_success \\\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 26b2cafc4..0fc5a7c59 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -1004,7 +1004,7 @@ test_expect_success 'push --porcelain' '\n test_expect_success 'push --porcelain bad url' '\n \tmk_empty testrepo &&\n \ttest_must_fail git push >.git/bar --porcelain asdfasdfasd refs/heads/master:refs/remotes/origin/master &&\n-\ttest_must_fail grep -q Done .git/bar\n+\t! grep -q Done .git/bar\n '\n \n test_expect_success 'push --porcelain rejected' '\ndiff --git a/t/t5601-clone.sh b/t/t5601-clone.sh\nindex a43339420..4241ea5b3 100755\n--- a/t/t5601-clone.sh\n+++ b/t/t5601-clone.sh\n@@ -151,7 +151,7 @@ test_expect_success 'clone --mirror does not repeat tags' '\n \tgit clone --mirror src mirror2 &&\n \t(cd mirror2 &&\n \t git show-ref 2> clone.err > clone.out) &&\n-\ttest_must_fail grep Duplicate mirror2/clone.err &&\n+\t! grep Duplicate mirror2/clone.err &&\n \tgrep some-tag mirror2/clone.out\n \n '\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex 5e5370feb..8c2c6eaef 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -407,7 +407,7 @@ test_expect_success 'good merge base when good and bad are siblings' '\n \ttest_i18ngrep \"merge base must be tested\" my_bisect_log.txt &&\n \tgrep $HASH4 my_bisect_log.txt &&\n \tgit bisect good > my_bisect_log.txt &&\n-\ttest_must_fail grep \"merge base must be tested\" my_bisect_log.txt &&\n+\t! grep \"merge base must be tested\" my_bisect_log.txt &&\n \tgrep $HASH6 my_bisect_log.txt &&\n \tgit bisect reset\n '\ndiff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh\nindex 63d36fb28..0fe7e58cf 100755\n--- a/t/t7610-mergetool.sh\n+++ b/t/t7610-mergetool.sh\n@@ -602,7 +602,7 @@ test_expect_success MKTEMP 'temporary filenames are used with mergetool.writeToT\n \ttest_config mergetool.myecho.trustExitCode true &&\n \ttest_must_fail git merge master &&\n \tgit mergetool --no-prompt --tool myecho -- both >actual &&\n-\ttest_must_fail grep ^\\./both_LOCAL_ actual >/dev/null &&\n+\t! grep ^\\./both_LOCAL_ actual >/dev/null &&\n \tgrep /both_LOCAL_ actual >/dev/null &&\n \tgit reset --hard master >/dev/null 2>&1\n '\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 3dc4a3454..0f398dd16 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -50,7 +50,7 @@ test_no_confirm () {\n \t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n \t\t$@ \\\n \t\t$patches >stdout &&\n-\t\ttest_must_fail grep \"Send this email\" stdout &&\n+\t\t! grep \"Send this email\" stdout &&\n \t\t>no_confirm_okay\n }\n \ndiff --git a/t/t9117-git-svn-init-clone.sh b/t/t9117-git-svn-init-clone.sh\nindex 69a675052..044f65e91 100755\n--- a/t/t9117-git-svn-init-clone.sh\n+++ b/t/t9117-git-svn-init-clone.sh\n@@ -55,7 +55,7 @@ test_expect_success 'clone to target directory with --stdlayout' '\n test_expect_success 'init without -s/-T/-b/-t does not warn' '\n \ttest ! -d trunk &&\n \tgit svn init \"$svnrepo\"/project/trunk trunk 2>warning &&\n-\ttest_must_fail grep -q prefix warning &&\n+\t! grep -q prefix warning &&\n \trm -rf trunk &&\n \trm -f warning\n \t'\n@@ -63,7 +63,7 @@ test_expect_success 'init without -s/-T/-b/-t does not warn' '\n test_expect_success 'clone without -s/-T/-b/-t does not warn' '\n \ttest ! -d trunk &&\n \tgit svn clone \"$svnrepo\"/project/trunk 2>warning &&\n-\ttest_must_fail grep -q prefix warning &&\n+\t! grep -q prefix warning &&\n \trm -rf trunk &&\n \trm -f warning\n \t'\n@@ -86,7 +86,7 @@ EOF\n test_expect_success 'init with -s/-T/-b/-t assumes --prefix=origin/' '\n \ttest ! -d project &&\n \tgit svn init -s \"$svnrepo\"/project project 2>warning &&\n-\ttest_must_fail grep -q prefix warning &&\n+\t! grep -q prefix warning &&\n \ttest_svn_configured_prefix \"origin/\" &&\n \trm -rf project &&\n \trm -f warning\n@@ -95,7 +95,7 @@ test_expect_success 'init with -s/-T/-b/-t assumes --prefix=origin/' '\n test_expect_success 'clone with -s/-T/-b/-t assumes --prefix=origin/' '\n \ttest ! -d project &&\n \tgit svn clone -s \"$svnrepo\"/project 2>warning &&\n-\ttest_must_fail grep -q prefix warning &&\n+\t! grep -q prefix warning &&\n \ttest_svn_configured_prefix \"origin/\" &&\n \trm -rf project &&\n \trm -f warning\n@@ -104,7 +104,7 @@ test_expect_success 'clone with -s/-T/-b/-t assumes --prefix=origin/' '\n test_expect_success 'init with -s/-T/-b/-t and --prefix \"\" still works' '\n \ttest ! -d project &&\n \tgit svn init -s \"$svnrepo\"/project project --prefix \"\" 2>warning &&\n-\ttest_must_fail grep -q prefix warning &&\n+\t! grep -q prefix warning &&\n \ttest_svn_configured_prefix \"\" &&\n \trm -rf project &&\n \trm -f warning\n@@ -113,7 +113,7 @@ test_expect_success 'init with -s/-T/-b/-t and --prefix \"\" still works' '\n test_expect_success 'clone with -s/-T/-b/-t and --prefix \"\" still works' '\n \ttest ! -d project &&\n \tgit svn clone -s \"$svnrepo\"/project --prefix \"\" 2>warning &&\n-\ttest_must_fail grep -q prefix warning &&\n+\t! grep -q prefix warning &&\n \ttest_svn_configured_prefix \"\" &&\n \trm -rf project &&\n \trm -f warning\ndiff --git a/t/t9813-git-p4-preserve-users.sh b/t/t9813-git-p4-preserve-users.sh\nindex 0fe231280..798bf2b67 100755\n--- a/t/t9813-git-p4-preserve-users.sh\n+++ b/t/t9813-git-p4-preserve-users.sh\n@@ -126,13 +126,13 @@ test_expect_success 'not preserving user with mixed authorship' '\n \t\tgrep \"git author charlie@example.com does not match\" &&\n \n \t\tmake_change_by_user usernamefile3 alice alice@example.com &&\n-\t\tgit p4 commit |\\\n-\t\ttest_must_fail grep \"git author.*does not match\" &&\n+\t\tgit p4 commit >actual 2>&1 &&\n+\t\t! grep \"git author.*does not match\" actual &&\n \n \t\tgit config git-p4.skipUserNameCheck true &&\n \t\tmake_change_by_user usernamefile3 Charlie charlie@example.com &&\n-\t\tgit p4 commit |\\\n-\t\ttest_must_fail grep \"git author.*does not match\" &&\n+\t\tgit p4 commit >actual 2>&1 &&\n+\t\t! grep \"git author.*does not match\" actual &&\n \n \t\tp4_check_commit_author usernamefile3 alice\n \t)\ndiff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh\nindex c89992cf9..e7e0268e9 100755\n--- a/t/t9814-git-p4-rename.sh\n+++ b/t/t9814-git-p4-rename.sh\n@@ -141,7 +141,7 @@ test_expect_success 'detect copies' '\n \t\tgit diff-tree -r -C HEAD &&\n \t\tgit p4 submit &&\n \t\tp4 filelog //depot/file8 &&\n-\t\tp4 filelog //depot/file8 | test_must_fail grep -q \"branch from\" &&\n+\t\t! p4 filelog //depot/file8 | grep -q \"branch from\" &&\n \n \t\techo \"file9\" >>file2 &&\n \t\tgit commit -a -m \"Differentiate file2\" &&\n@@ -154,7 +154,7 @@ test_expect_success 'detect copies' '\n \t\tgit config git-p4.detectCopies true &&\n \t\tgit p4 submit &&\n \t\tp4 filelog //depot/file9 &&\n-\t\tp4 filelog //depot/file9 | test_must_fail grep -q \"branch from\" &&\n+\t\t! p4 filelog //depot/file9 | grep -q \"branch from\" &&\n \n \t\techo \"file10\" >>file2 &&\n \t\tgit commit -a -m \"Differentiate file2\" &&\n@@ -202,7 +202,7 @@ test_expect_success 'detect copies' '\n \t\tgit config git-p4.detectCopies $(($level + 2)) &&\n \t\tgit p4 submit &&\n \t\tp4 filelog //depot/file12 &&\n-\t\tp4 filelog //depot/file12 | test_must_fail grep -q \"branch from\" &&\n+\t\t! p4 filelog //depot/file12 | grep -q \"branch from\" &&\n \n \t\techo \"file13\" >>file2 &&\n \t\tgit commit -a -m \"Differentiate file2\" &&\n-- \n2.11.0\n\n"},{"id":"308742","messageId":"20170103195708.15157-2-pranit.bauva@gmail.com","threadId":"44797","inReplyTo":"20170103195708.15157-1-pranit.bauva@gmail.com","subject":"[PATCH v3 2/2] t9813: avoid using pipes","fromName":"Pranit Bauva","fromEmail":"pranit.bauva@gmail.com","sentAt":"2017-01-03T19:57:08Z","receivedAt":"2017-01-03T19:57:40Z","isPatch":true,"sender":{"key":"pranit.bauva@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2959938?v=4"},"body":"The exit code of the upstream in a pipe is ignored thus we should avoid\nusing it. By writing out the output of the git command to a file, we can\ntest the exit codes of both the commands.\n\nSigned-off-by: Pranit Bauva <pranit.bauva@gmail.com>\n---\n t/t9813-git-p4-preserve-users.sh | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t9813-git-p4-preserve-users.sh b/t/t9813-git-p4-preserve-users.sh\nindex 798bf2b67..2133b21ae 100755\n--- a/t/t9813-git-p4-preserve-users.sh\n+++ b/t/t9813-git-p4-preserve-users.sh\n@@ -118,12 +118,12 @@ test_expect_success 'not preserving user with mixed authorship' '\n \t\tmake_change_by_user usernamefile3 Derek derek@example.com &&\n \t\tP4EDITOR=cat P4USER=alice P4PASSWD=secret &&\n \t\texport P4EDITOR P4USER P4PASSWD &&\n-\t\tgit p4 commit |\\\n-\t\tgrep \"git author derek@example.com does not match\" &&\n+\t\tgit p4 commit >actual &&\n+\t\tgrep \"git author derek@example.com does not match\" actual &&\n \n \t\tmake_change_by_user usernamefile3 Charlie charlie@example.com &&\n-\t\tgit p4 commit |\\\n-\t\tgrep \"git author charlie@example.com does not match\" &&\n+\t\tgit p4 commit >actual &&\n+\t\tgrep \"git author charlie@example.com does not match\" actual &&\n \n \t\tmake_change_by_user usernamefile3 alice alice@example.com &&\n \t\tgit p4 commit >actual 2>&1 &&\n-- \n2.11.0\n\n"},{"id":"308766","messageId":"CAE5ih78vLwDubesnAxD=g3TzsbN0sQZae3McdFcwDAZfYYhXSg@mail.gmail.com","threadId":"44797","inReplyTo":"20170103195708.15157-2-pranit.bauva@gmail.com","subject":"Re: [PATCH v3 2/2] t9813: avoid using pipes","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2017-01-04T09:11:33Z","receivedAt":"2017-01-04T09:11:59Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 3 January 2017 at 19:57, Pranit Bauva <pranit.bauva@gmail.com> wrote:\n> The exit code of the upstream in a pipe is ignored thus we should avoid\n> using it. By writing out the output of the git command to a file, we can\n> test the exit codes of both the commands.\n\nDo we also need to fix t9814-git-p4-rename.sh ?\n\n>\n> Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>\n> ---\n>  t/t9813-git-p4-preserve-users.sh | 8 ++++----\n>  1 file changed, 4 insertions(+), 4 deletions(-)\n>\n> diff --git a/t/t9813-git-p4-preserve-users.sh b/t/t9813-git-p4-preserve-users.sh\n> index 798bf2b67..2133b21ae 100755\n> --- a/t/t9813-git-p4-preserve-users.sh\n> +++ b/t/t9813-git-p4-preserve-users.sh\n> @@ -118,12 +118,12 @@ test_expect_success 'not preserving user with mixed authorship' '\n>                 make_change_by_user usernamefile3 Derek derek@example.com &&\n>                 P4EDITOR=cat P4USER=alice P4PASSWD=secret &&\n>                 export P4EDITOR P4USER P4PASSWD &&\n> -               git p4 commit |\\\n> -               grep \"git author derek@example.com does not match\" &&\n> +               git p4 commit >actual &&\n> +               grep \"git author derek@example.com does not match\" actual &&\n>\n>                 make_change_by_user usernamefile3 Charlie charlie@example.com &&\n> -               git p4 commit |\\\n> -               grep \"git author charlie@example.com does not match\" &&\n> +               git p4 commit >actual &&\n> +               grep \"git author charlie@example.com does not match\" actual &&\n>\n>                 make_change_by_user usernamefile3 alice alice@example.com &&\n>                 git p4 commit >actual 2>&1 &&\n> --\n> 2.11.0\n>\n"},{"id":"308769","messageId":"CAFZEwPNuWf3WPY_WjTK8on1mzC58nZgmFhNdkmqQY5=-HE9XCg@mail.gmail.com","threadId":"44797","inReplyTo":"CAE5ih78vLwDubesnAxD=g3TzsbN0sQZae3McdFcwDAZfYYhXSg@mail.gmail.com","subject":"Re: [PATCH v3 2/2] t9813: avoid using pipes","fromName":"Pranit Bauva","fromEmail":"pranit.bauva@gmail.com","sentAt":"2017-01-04T11:49:11Z","receivedAt":"2017-01-04T11:49:49Z","isPatch":true,"sender":{"key":"pranit.bauva@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2959938?v=4"},"body":"Hey Luke,\n\nOn Wed, Jan 4, 2017 at 2:41 PM, Luke Diamand <luke@diamand.org> wrote:\n> On 3 January 2017 at 19:57, Pranit Bauva <pranit.bauva@gmail.com> wrote:\n>> The exit code of the upstream in a pipe is ignored thus we should avoid\n>> using it. By writing out the output of the git command to a file, we can\n>> test the exit codes of both the commands.\n>\n> Do we also need to fix t9814-git-p4-rename.sh ?\n\nI don't think so. As Johannes[1] and Stefan[2] pointed out, we should\navoid upstream pipes for git. p4 can be treated as an \"external\ncommand\" just like grep/sed.\n\n[1]: http://public-inbox.org/git/285ed013-5c59-0b98-7dc0-8f729587a313@kdbg.org/\n[2]: http://public-inbox.org/git/CAGZ79kZRFLzD7wcAnFvke9vBxxTAgE7=Ud7F_O95EfkWqz=LJw@mail.gmail.com/\n"},{"id":"308931","messageId":"xmqqy3ym36y2.fsf@gitster.mtv.corp.google.com","threadId":"44797","inReplyTo":"CAFZEwPNbtamFfFy7vYXurpEWBDmRMyPB9+Ep-hm4uZVMREbq5Q@mail.gmail.com","subject":"Re: [PATCH] don't use test_must_fail with grep","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-07T21:18:13Z","receivedAt":"2017-01-07T21:18:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pranit Bauva <pranit.bauva@gmail.com> writes:\n\n> Hey Johannes,\n>\n> On Sun, Jan 1, 2017 at 8:20 PM, Johannes Sixt <j6t@kdbg.org> wrote:\n>> which makes me wonder: Is the message that we do expect not to occur\n>> actually printed on stdout? It sounds much more like an error message, i.e.,\n>> text that is printed on stderr. Wouldn't we need this?\n>>\n>>         git p4 commit >actual 2>&1 &&\n>>         ! grep \"git author.*does not match\" actual &&\n>>\n>> -- Hannes\n>\n> This seems better! Since I am at it, I can remove the traces of pipes\n> in an another patch.\n>\n> Regards,\n> Pranit Bauva\n\nI see v3 that has 2>&1 but according to Luke's comment (\"the message\ncomes from cat\"), it shouldn't be there?  I am behind clearing the\nbacklog in my mailbox and I could tweak it out from v3 while\nqueuing, or I may forget about it after looking at other topics ;-)\nin which case you may want to send v4 with the fix?\n"},{"id":"308963","messageId":"CAFZEwPNhfYHLZvdEV0k22dJgJ+zhpSE9k1jUowNiPJFkASMz2w@mail.gmail.com","threadId":"44797","inReplyTo":"xmqqy3ym36y2.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] don't use test_must_fail with grep","fromName":"Pranit Bauva","fromEmail":"pranit.bauva@gmail.com","sentAt":"2017-01-08T16:53:16Z","receivedAt":"2017-01-08T16:53:24Z","isPatch":true,"sender":{"key":"pranit.bauva@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2959938?v=4"},"body":"Hey Junio,\n\nOn Sun, Jan 8, 2017 at 2:48 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> I see v3 that has 2>&1 but according to Luke's comment (\"the message\n> comes from cat\"), it shouldn't be there?  I am behind clearing the\n> backlog in my mailbox and I could tweak it out from v3 while\n> queuing, or I may forget about it after looking at other topics ;-)\n> in which case you may want to send v4 with the fix?\n\nYeah sure! No problem! :)\n\nRegards,\nPranit Bauva\n"},{"id":"308964","messageId":"010201597f017978-356bf9e9-ee78-498b-926b-5c00466b1d9e-000000@eu-west-1.amazonses.com","threadId":"44797","inReplyTo":"20170103195708.15157-1-pranit.bauva@gmail.com","subject":"[PATCH v4 1/2] don't use test_must_fail with grep","fromName":"Pranit Bauva","fromEmail":"pranit.bauva@gmail.com","sentAt":"2017-01-08T16:55:20Z","receivedAt":"2017-01-08T16:55:28Z","isPatch":true,"sender":{"key":"pranit.bauva@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2959938?v=4"},"body":"test_must_fail should only be used for testing git commands. To test the\nfailure of other commands use `!`.\n\nReported-by: Stefan Beller <sbeller@google.com>\nSigned-off-by: Pranit Bauva <pranit.bauva@gmail.com>\n---\n t/t3510-cherry-pick-sequence.sh  |  6 +++---\n t/t5504-fetch-receive-strict.sh  |  2 +-\n t/t5516-fetch-push.sh            |  2 +-\n t/t5601-clone.sh                 |  2 +-\n t/t6030-bisect-porcelain.sh      |  2 +-\n t/t7610-mergetool.sh             |  2 +-\n t/t9001-send-email.sh            |  2 +-\n t/t9117-git-svn-init-clone.sh    | 12 ++++++------\n t/t9813-git-p4-preserve-users.sh |  8 ++++----\n t/t9814-git-p4-rename.sh         |  6 +++---\n 10 files changed, 22 insertions(+), 22 deletions(-)\n\ndiff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh\nindex 372307c..0acf4b1 100755\n--- a/t/t3510-cherry-pick-sequence.sh\n+++ b/t/t3510-cherry-pick-sequence.sh\n@@ -385,7 +385,7 @@ test_expect_success '--continue respects opts' '\n \tgit cat-file commit HEAD~1 >picked_msg &&\n \tgit cat-file commit HEAD~2 >unrelatedpick_msg &&\n \tgit cat-file commit HEAD~3 >initial_msg &&\n-\ttest_must_fail grep \"cherry picked from\" initial_msg &&\n+\t! grep \"cherry picked from\" initial_msg &&\n \tgrep \"cherry picked from\" unrelatedpick_msg &&\n \tgrep \"cherry picked from\" picked_msg &&\n \tgrep \"cherry picked from\" anotherpick_msg\n@@ -426,9 +426,9 @@ test_expect_failure '--signoff is automatically propagated to resolved conflict'\n \tgit cat-file commit HEAD~1 >picked_msg &&\n \tgit cat-file commit HEAD~2 >unrelatedpick_msg &&\n \tgit cat-file commit HEAD~3 >initial_msg &&\n-\ttest_must_fail grep \"Signed-off-by:\" initial_msg &&\n+\t! grep \"Signed-off-by:\" initial_msg &&\n \tgrep \"Signed-off-by:\" unrelatedpick_msg &&\n-\ttest_must_fail grep \"Signed-off-by:\" picked_msg &&\n+\t! grep \"Signed-off-by:\" picked_msg &&\n \tgrep \"Signed-off-by:\" anotherpick_msg\n '\n \ndiff --git a/t/t5504-fetch-receive-strict.sh b/t/t5504-fetch-receive-strict.sh\nindex 9b19cff..49d3621 100755\n--- a/t/t5504-fetch-receive-strict.sh\n+++ b/t/t5504-fetch-receive-strict.sh\n@@ -152,7 +152,7 @@ test_expect_success 'push with receive.fsck.missingEmail=warn' '\n \tgit --git-dir=dst/.git config --add \\\n \t\treceive.fsck.badDate warn &&\n \tgit push --porcelain dst bogus >act 2>&1 &&\n-\ttest_must_fail grep \"missingEmail\" act\n+\t! grep \"missingEmail\" act\n '\n \n test_expect_success \\\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 26b2caf..0fc5a7c 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -1004,7 +1004,7 @@ test_expect_success 'push --porcelain' '\n test_expect_success 'push --porcelain bad url' '\n \tmk_empty testrepo &&\n \ttest_must_fail git push >.git/bar --porcelain asdfasdfasd refs/heads/master:refs/remotes/origin/master &&\n-\ttest_must_fail grep -q Done .git/bar\n+\t! grep -q Done .git/bar\n '\n \n test_expect_success 'push --porcelain rejected' '\ndiff --git a/t/t5601-clone.sh b/t/t5601-clone.sh\nindex a433394..4241ea5 100755\n--- a/t/t5601-clone.sh\n+++ b/t/t5601-clone.sh\n@@ -151,7 +151,7 @@ test_expect_success 'clone --mirror does not repeat tags' '\n \tgit clone --mirror src mirror2 &&\n \t(cd mirror2 &&\n \t git show-ref 2> clone.err > clone.out) &&\n-\ttest_must_fail grep Duplicate mirror2/clone.err &&\n+\t! grep Duplicate mirror2/clone.err &&\n \tgrep some-tag mirror2/clone.out\n \n '\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex 5e5370f..8c2c6ea 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -407,7 +407,7 @@ test_expect_success 'good merge base when good and bad are siblings' '\n \ttest_i18ngrep \"merge base must be tested\" my_bisect_log.txt &&\n \tgrep $HASH4 my_bisect_log.txt &&\n \tgit bisect good > my_bisect_log.txt &&\n-\ttest_must_fail grep \"merge base must be tested\" my_bisect_log.txt &&\n+\t! grep \"merge base must be tested\" my_bisect_log.txt &&\n \tgrep $HASH6 my_bisect_log.txt &&\n \tgit bisect reset\n '\ndiff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh\nindex 63d36fb..0fe7e58 100755\n--- a/t/t7610-mergetool.sh\n+++ b/t/t7610-mergetool.sh\n@@ -602,7 +602,7 @@ test_expect_success MKTEMP 'temporary filenames are used with mergetool.writeToT\n \ttest_config mergetool.myecho.trustExitCode true &&\n \ttest_must_fail git merge master &&\n \tgit mergetool --no-prompt --tool myecho -- both >actual &&\n-\ttest_must_fail grep ^\\./both_LOCAL_ actual >/dev/null &&\n+\t! grep ^\\./both_LOCAL_ actual >/dev/null &&\n \tgrep /both_LOCAL_ actual >/dev/null &&\n \tgit reset --hard master >/dev/null 2>&1\n '\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 3dc4a34..0f398dd 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -50,7 +50,7 @@ test_no_confirm () {\n \t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n \t\t$@ \\\n \t\t$patches >stdout &&\n-\t\ttest_must_fail grep \"Send this email\" stdout &&\n+\t\t! grep \"Send this email\" stdout &&\n \t\t>no_confirm_okay\n }\n \ndiff --git a/t/t9117-git-svn-init-clone.sh b/t/t9117-git-svn-init-clone.sh\nindex 69a6750..044f65e 100755\n--- a/t/t9117-git-svn-init-clone.sh\n+++ b/t/t9117-git-svn-init-clone.sh\n@@ -55,7 +55,7 @@ test_expect_success 'clone to target directory with --stdlayout' '\n test_expect_success 'init without -s/-T/-b/-t does not warn' '\n \ttest ! -d trunk &&\n \tgit svn init \"$svnrepo\"/project/trunk trunk 2>warning &&\n-\ttest_must_fail grep -q prefix warning &&\n+\t! grep -q prefix warning &&\n \trm -rf trunk &&\n \trm -f warning\n \t'\n@@ -63,7 +63,7 @@ test_expect_success 'init without -s/-T/-b/-t does not warn' '\n test_expect_success 'clone without -s/-T/-b/-t does not warn' '\n \ttest ! -d trunk &&\n \tgit svn clone \"$svnrepo\"/project/trunk 2>warning &&\n-\ttest_must_fail grep -q prefix warning &&\n+\t! grep -q prefix warning &&\n \trm -rf trunk &&\n \trm -f warning\n \t'\n@@ -86,7 +86,7 @@ EOF\n test_expect_success 'init with -s/-T/-b/-t assumes --prefix=origin/' '\n \ttest ! -d project &&\n \tgit svn init -s \"$svnrepo\"/project project 2>warning &&\n-\ttest_must_fail grep -q prefix warning &&\n+\t! grep -q prefix warning &&\n \ttest_svn_configured_prefix \"origin/\" &&\n \trm -rf project &&\n \trm -f warning\n@@ -95,7 +95,7 @@ test_expect_success 'init with -s/-T/-b/-t assumes --prefix=origin/' '\n test_expect_success 'clone with -s/-T/-b/-t assumes --prefix=origin/' '\n \ttest ! -d project &&\n \tgit svn clone -s \"$svnrepo\"/project 2>warning &&\n-\ttest_must_fail grep -q prefix warning &&\n+\t! grep -q prefix warning &&\n \ttest_svn_configured_prefix \"origin/\" &&\n \trm -rf project &&\n \trm -f warning\n@@ -104,7 +104,7 @@ test_expect_success 'clone with -s/-T/-b/-t assumes --prefix=origin/' '\n test_expect_success 'init with -s/-T/-b/-t and --prefix \"\" still works' '\n \ttest ! -d project &&\n \tgit svn init -s \"$svnrepo\"/project project --prefix \"\" 2>warning &&\n-\ttest_must_fail grep -q prefix warning &&\n+\t! grep -q prefix warning &&\n \ttest_svn_configured_prefix \"\" &&\n \trm -rf project &&\n \trm -f warning\n@@ -113,7 +113,7 @@ test_expect_success 'init with -s/-T/-b/-t and --prefix \"\" still works' '\n test_expect_success 'clone with -s/-T/-b/-t and --prefix \"\" still works' '\n \ttest ! -d project &&\n \tgit svn clone -s \"$svnrepo\"/project --prefix \"\" 2>warning &&\n-\ttest_must_fail grep -q prefix warning &&\n+\t! grep -q prefix warning &&\n \ttest_svn_configured_prefix \"\" &&\n \trm -rf project &&\n \trm -f warning\ndiff --git a/t/t9813-git-p4-preserve-users.sh b/t/t9813-git-p4-preserve-users.sh\nindex 0fe2312..76004a5 100755\n--- a/t/t9813-git-p4-preserve-users.sh\n+++ b/t/t9813-git-p4-preserve-users.sh\n@@ -126,13 +126,13 @@ test_expect_success 'not preserving user with mixed authorship' '\n \t\tgrep \"git author charlie@example.com does not match\" &&\n \n \t\tmake_change_by_user usernamefile3 alice alice@example.com &&\n-\t\tgit p4 commit |\\\n-\t\ttest_must_fail grep \"git author.*does not match\" &&\n+\t\tgit p4 commit >actual &&\n+\t\t! grep \"git author.*does not match\" actual &&\n \n \t\tgit config git-p4.skipUserNameCheck true &&\n \t\tmake_change_by_user usernamefile3 Charlie charlie@example.com &&\n-\t\tgit p4 commit |\\\n-\t\ttest_must_fail grep \"git author.*does not match\" &&\n+\t\tgit p4 commit >actual &&\n+\t\t! grep \"git author.*does not match\" actual &&\n \n \t\tp4_check_commit_author usernamefile3 alice\n \t)\ndiff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh\nindex c89992c..e7e0268 100755\n--- a/t/t9814-git-p4-rename.sh\n+++ b/t/t9814-git-p4-rename.sh\n@@ -141,7 +141,7 @@ test_expect_success 'detect copies' '\n \t\tgit diff-tree -r -C HEAD &&\n \t\tgit p4 submit &&\n \t\tp4 filelog //depot/file8 &&\n-\t\tp4 filelog //depot/file8 | test_must_fail grep -q \"branch from\" &&\n+\t\t! p4 filelog //depot/file8 | grep -q \"branch from\" &&\n \n \t\techo \"file9\" >>file2 &&\n \t\tgit commit -a -m \"Differentiate file2\" &&\n@@ -154,7 +154,7 @@ test_expect_success 'detect copies' '\n \t\tgit config git-p4.detectCopies true &&\n \t\tgit p4 submit &&\n \t\tp4 filelog //depot/file9 &&\n-\t\tp4 filelog //depot/file9 | test_must_fail grep -q \"branch from\" &&\n+\t\t! p4 filelog //depot/file9 | grep -q \"branch from\" &&\n \n \t\techo \"file10\" >>file2 &&\n \t\tgit commit -a -m \"Differentiate file2\" &&\n@@ -202,7 +202,7 @@ test_expect_success 'detect copies' '\n \t\tgit config git-p4.detectCopies $(($level + 2)) &&\n \t\tgit p4 submit &&\n \t\tp4 filelog //depot/file12 &&\n-\t\tp4 filelog //depot/file12 | test_must_fail grep -q \"branch from\" &&\n+\t\t! p4 filelog //depot/file12 | grep -q \"branch from\" &&\n \n \t\techo \"file13\" >>file2 &&\n \t\tgit commit -a -m \"Differentiate file2\" &&\n\n--\nhttps://github.com/git/git/pull/314\n"},{"id":"308965","messageId":"010201597f0179fb-fc4c0240-5ec7-466b-96b9-59f4840954d7-000000@eu-west-1.amazonses.com","threadId":"44797","inReplyTo":"010201597f017978-356bf9e9-ee78-498b-926b-5c00466b1d9e-000000@eu-west-1.amazonses.com","subject":"[PATCH v4 2/2] t9813: avoid using pipes","fromName":"Pranit Bauva","fromEmail":"pranit.bauva@gmail.com","sentAt":"2017-01-08T16:55:20Z","receivedAt":"2017-01-08T16:55:31Z","isPatch":true,"sender":{"key":"pranit.bauva@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2959938?v=4"},"body":"The exit code of the upstream in a pipe is ignored thus we should avoid\nusing it. By writing out the output of the git command to a file, we can\ntest the exit codes of both the commands.\n\nSigned-off-by: Pranit Bauva <pranit.bauva@gmail.com>\n---\n t/t9813-git-p4-preserve-users.sh | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t9813-git-p4-preserve-users.sh b/t/t9813-git-p4-preserve-users.sh\nindex 76004a5..bda222a 100755\n--- a/t/t9813-git-p4-preserve-users.sh\n+++ b/t/t9813-git-p4-preserve-users.sh\n@@ -118,12 +118,12 @@ test_expect_success 'not preserving user with mixed authorship' '\n \t\tmake_change_by_user usernamefile3 Derek derek@example.com &&\n \t\tP4EDITOR=cat P4USER=alice P4PASSWD=secret &&\n \t\texport P4EDITOR P4USER P4PASSWD &&\n-\t\tgit p4 commit |\\\n-\t\tgrep \"git author derek@example.com does not match\" &&\n+\t\tgit p4 commit >actual &&\n+\t\tgrep \"git author derek@example.com does not match\" actual &&\n \n \t\tmake_change_by_user usernamefile3 Charlie charlie@example.com &&\n-\t\tgit p4 commit |\\\n-\t\tgrep \"git author charlie@example.com does not match\" &&\n+\t\tgit p4 commit >actual &&\n+\t\tgrep \"git author charlie@example.com does not match\" actual &&\n \n \t\tmake_change_by_user usernamefile3 alice alice@example.com &&\n \t\tgit p4 commit >actual &&\n\n--\nhttps://github.com/git/git/pull/314\n"},{"id":"309008","messageId":"CAE5ih7-mAfezTwdbWrAWFOSoCf-z_NJOic+FQdCmHbCyR8ng9w@mail.gmail.com","threadId":"44797","inReplyTo":"010201597f0179fb-fc4c0240-5ec7-466b-96b9-59f4840954d7-000000@eu-west-1.amazonses.com","subject":"Re: [PATCH v4 2/2] t9813: avoid using pipes","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2017-01-09T09:11:06Z","receivedAt":"2017-01-09T09:11:13Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 8 January 2017 at 16:55, Pranit Bauva <pranit.bauva@gmail.com> wrote:\n> The exit code of the upstream in a pipe is ignored thus we should avoid\n> using it. By writing out the output of the git command to a file, we can\n> test the exit codes of both the commands.\n\nLooks good to me, thanks!\n\nAck.\n\n>\n> Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>\n> ---\n>  t/t9813-git-p4-preserve-users.sh | 8 ++++----\n>  1 file changed, 4 insertions(+), 4 deletions(-)\n>\n> diff --git a/t/t9813-git-p4-preserve-users.sh b/t/t9813-git-p4-preserve-users.sh\n> index 76004a5..bda222a 100755\n> --- a/t/t9813-git-p4-preserve-users.sh\n> +++ b/t/t9813-git-p4-preserve-users.sh\n> @@ -118,12 +118,12 @@ test_expect_success 'not preserving user with mixed authorship' '\n>                 make_change_by_user usernamefile3 Derek derek@example.com &&\n>                 P4EDITOR=cat P4USER=alice P4PASSWD=secret &&\n>                 export P4EDITOR P4USER P4PASSWD &&\n> -               git p4 commit |\\\n> -               grep \"git author derek@example.com does not match\" &&\n> +               git p4 commit >actual &&\n> +               grep \"git author derek@example.com does not match\" actual &&\n>\n>                 make_change_by_user usernamefile3 Charlie charlie@example.com &&\n> -               git p4 commit |\\\n> -               grep \"git author charlie@example.com does not match\" &&\n> +               git p4 commit >actual &&\n> +               grep \"git author charlie@example.com does not match\" actual &&\n>\n>                 make_change_by_user usernamefile3 alice alice@example.com &&\n>                 git p4 commit >actual &&\n>\n> --\n> https://github.com/git/git/pull/314\n"},{"id":"309012","messageId":"xmqq37gsy2wa.fsf@gitster.mtv.corp.google.com","threadId":"44797","inReplyTo":"CAE5ih7-mAfezTwdbWrAWFOSoCf-z_NJOic+FQdCmHbCyR8ng9w@mail.gmail.com","subject":"Re: [PATCH v4 2/2] t9813: avoid using pipes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-09T09:54:29Z","receivedAt":"2017-01-09T09:54:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Luke Diamand <luke@diamand.org> writes:\n\n> On 8 January 2017 at 16:55, Pranit Bauva <pranit.bauva@gmail.com> wrote:\n>> The exit code of the upstream in a pipe is ignored thus we should avoid\n>> using it. By writing out the output of the git command to a file, we can\n>> test the exit codes of both the commands.\n>\n> Looks good to me, thanks!\n>\n> Ack.\n\nThanks, both.\n\n"}]}