{"thread":{"id":"57639","subject":"[PATCH] t3501: use test_path_is_* functions","startedAt":"2022-03-30T21:43:12Z","lastAt":"2022-04-22T11:26:35Z","messageCount":16,"participants":["Labnan via GitGitGadget","Ævar Arnfjörð Bjarmason","Labnann","Junio C Hamano","Khalid Masum"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"452706","messageId":"pull.1195.git.1648676585765.gitgitgadget@gmail.com","threadId":"57639","inReplyTo":null,"subject":"[PATCH] t3501: use test_path_is_* functions","fromName":"Labnan via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-03-30T21:43:05Z","receivedAt":"2022-03-30T21:43:12Z","isPatch":true,"sender":{"key":"name:Labnan","avatar":null},"body":"From: Labnann <khalid.masum.92@gmail.com>\n\nTwo test -f are present in t3501. They can be replaced with appropriate\nhelper function: test_path_is_file\n\nSigned-off-by: Labnann <khalid.masum.92@gmail.com>\n---\n    [GSoC][PATCH] t3501: Use test_path_is_* Functions\n    \n    Two test -f are present in t3501. They can be replaced with appropriate\n    helper function: test_path_is_file. Which makes the script more readable\n    and gives better error messages.\n    \n    Signed-off-by: Labnann khalid.masum.92@gmail.com\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1195%2FLabnann%2Ft3501-helper-functions-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1195/Labnann/t3501-helper-functions-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1195\n\n t/t3501-revert-cherry-pick.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh\nindex 8617efaaf1e..45492a7ed09 100755\n--- a/t/t3501-revert-cherry-pick.sh\n+++ b/t/t3501-revert-cherry-pick.sh\n@@ -67,7 +67,7 @@ test_expect_success 'cherry-pick after renaming branch' '\n \tgit checkout rename2 &&\n \tgit cherry-pick added &&\n \ttest $(git rev-parse HEAD^) = $(git rev-parse rename2) &&\n-\ttest -f opos &&\n+\ttest_path_is_file opos &&\n \tgrep \"Add extra line at the end\" opos &&\n \tgit reflog -1 | grep cherry-pick\n \n@@ -78,7 +78,7 @@ test_expect_success 'revert after renaming branch' '\n \tgit checkout rename1 &&\n \tgit revert added &&\n \ttest $(git rev-parse HEAD^) = $(git rev-parse rename1) &&\n-\ttest -f spoo &&\n+\ttest_path_is_file spoo &&\n \t! grep \"Add extra line at the end\" spoo &&\n \tgit reflog -1 | grep revert\n \n\nbase-commit: 805e0a68082a217f0112db9ee86a022227a9c81b\n-- \ngitgitgadget\n"},{"id":"452713","messageId":"220331.86bkxnt77a.gmgdl@evledraar.gmail.com","threadId":"57639","inReplyTo":"pull.1195.git.1648676585765.gitgitgadget@gmail.com","subject":"Re: [PATCH] t3501: use test_path_is_* functions","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-30T22:11:48Z","receivedAt":"2022-03-30T22:19:04Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Mar 30 2022, Labnan via GitGitGadget wrote:\n\n> From: Labnann <khalid.masum.92@gmail.com>\n>\n> Two test -f are present in t3501. They can be replaced with appropriate\n> helper function: test_path_is_file\n>\n> Signed-off-by: Labnann <khalid.masum.92@gmail.com>\n\nThanks for contributing to git, this is a much needed area of\nimprovement.\n\n> diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh\n> index 8617efaaf1e..45492a7ed09 100755\n> --- a/t/t3501-revert-cherry-pick.sh\n> +++ b/t/t3501-revert-cherry-pick.sh\n> @@ -67,7 +67,7 @@ test_expect_success 'cherry-pick after renaming branch' '\n>  \tgit checkout rename2 &&\n>  \tgit cherry-pick added &&\n>  \ttest $(git rev-parse HEAD^) = $(git rev-parse rename2) &&\n> -\ttest -f opos &&\n> +\ttest_path_is_file opos &&\n>  \tgrep \"Add extra line at the end\" opos &&\n>  \tgit reflog -1 | grep cherry-pick\n\nWhile perfect shouldn't be the enemy of the good, I think it would also\nbe a very nice use of review bandwidth to end up with some good\nend-state here if possible.\n\nI.e. a pre-existing issue here is:\n\n * We are hiding the exit code of git due to using \"test\", see\n   c419562860e (checkout tests: don't ignore \"git <cmd>\" exit code,\n   2022-03-07) for one example of how to fixthat.\n\n * The \"test -f\" here is really redundant to begin with, because we'd\n   get an error from \"grep\" if opos didn't exist.\n\n * Ditto ignoring the exit code of \"git reflog -1\".\n\n> @@ -78,7 +78,7 @@ test_expect_success 'revert after renaming branch' '\n>  \tgit checkout rename1 &&\n>  \tgit revert added &&\n>  \ttest $(git rev-parse HEAD^) = $(git rev-parse rename1) &&\n> -\ttest -f spoo &&\n> +\ttest_path_is_file spoo &&\n>  \t! grep \"Add extra line at the end\" spoo &&\n\n\nSame issues here, except that the \"test -f\" aka \"test_path_is_file\"\nisn't redundant, as the grep is inverted.\n\nIt seems to me (other issues aside) that what this test actually wants\nto express is something like this:\n\t\n\tdiff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh\n\tindex 8617efaaf1e..00096b75376 100755\n\t--- a/t/t3501-revert-cherry-pick.sh\n\t+++ b/t/t3501-revert-cherry-pick.sh\n\t@@ -78,8 +78,9 @@ test_expect_success 'revert after renaming branch' '\n\t \tgit checkout rename1 &&\n\t \tgit revert added &&\n\t \ttest $(git rev-parse HEAD^) = $(git rev-parse rename1) &&\n\t-\ttest -f spoo &&\n\t-\t! grep \"Add extra line at the end\" spoo &&\n\t+\tgit rev-parse initial:oops >expect &&\n\t+\tgit rev-parse HEAD:spoo >actual &&\n\t+\ttest_cmp expect actual &&\n\t \tgit reflog -1 | grep revert\n\t \n\t '\n\nI.e. we did a revert of a file we had so that it's the same as in\n\"initial\", but now it's at a different path, which we can exhaustively\ndo by checking the blob OIDs.\n\n>  \tgit reflog -1 | grep revert\n>  \n>\n> base-commit: 805e0a68082a217f0112db9ee86a022227a9c81b\n\n"},{"id":"452820","messageId":"20220331191525.17927-1-khalid.masum.92@gmail.com","threadId":"57639","inReplyTo":"220331.86bkxnt77a.gmgdl@evledraar.gmail.com","subject":"[PATCH v2 0/3][GSoC] t3501: remove test and test -f","fromName":"Labnann","fromEmail":"khalid.masum.92@gmail.com","sentAt":"2022-03-31T19:15:22Z","receivedAt":"2022-03-31T19:15:34Z","isPatch":true,"sender":{"key":"khalid.masum.92@gmail.com","avatar":"https://avatars.githubusercontent.com/u/55809005?v=4"},"body":"Hello!\nThess remove test -f from t3501 and test command. It uses test_cmp \nto hide exit code of git.\n\nCan I get help with \"git reflog -1 | grep revert\"? About which helper\nfunction I can use to hide the exit code.\n\nThank you.\n\n\nLabnann (3):\n  t3501: use test_path_is_* functions\n  t3501: don't ignore \"git <cmd>\" exit code\n  t3501: test cherry-pick revert with oids\n\n t/t3501-revert-cherry-pick.sh | 14 +++++++++-----\n 1 file changed, 9 insertions(+), 5 deletions(-)\n\nRange-diff against v1:\n1:  8b7d38a66f = 1:  8b7d38a66f t3501: use test_path_is_* functions\n-:  ---------- > 2:  04b0bf1c5d t3501: don't ignore \"git <cmd>\" exit code\n-:  ---------- > 3:  36445b40fb t3501: test cherry-pick revert with oids\n-- \n2.35.1\n\n"},{"id":"452821","messageId":"20220331191525.17927-2-khalid.masum.92@gmail.com","threadId":"57639","inReplyTo":"20220331191525.17927-1-khalid.masum.92@gmail.com","subject":"[PATCH v2 1/3] t3501: use test_path_is_* functions","fromName":"Labnann","fromEmail":"khalid.masum.92@gmail.com","sentAt":"2022-03-31T19:15:23Z","receivedAt":"2022-03-31T19:15:41Z","isPatch":true,"sender":{"key":"khalid.masum.92@gmail.com","avatar":"https://avatars.githubusercontent.com/u/55809005?v=4"},"body":"Two test -f are present in t3501. They can be replaced with appropriate\nhelper function: test_path_is_file\n\nSigned-off-by: Labnann <khalid.masum.92@gmail.com>\n---\n t/t3501-revert-cherry-pick.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh\nindex 8617efaaf1..45492a7ed0 100755\n--- a/t/t3501-revert-cherry-pick.sh\n+++ b/t/t3501-revert-cherry-pick.sh\n@@ -67,7 +67,7 @@ test_expect_success 'cherry-pick after renaming branch' '\n \tgit checkout rename2 &&\n \tgit cherry-pick added &&\n \ttest $(git rev-parse HEAD^) = $(git rev-parse rename2) &&\n-\ttest -f opos &&\n+\ttest_path_is_file opos &&\n \tgrep \"Add extra line at the end\" opos &&\n \tgit reflog -1 | grep cherry-pick\n \n@@ -78,7 +78,7 @@ test_expect_success 'revert after renaming branch' '\n \tgit checkout rename1 &&\n \tgit revert added &&\n \ttest $(git rev-parse HEAD^) = $(git rev-parse rename1) &&\n-\ttest -f spoo &&\n+\ttest_path_is_file spoo &&\n \t! grep \"Add extra line at the end\" spoo &&\n \tgit reflog -1 | grep revert\n \n-- \n2.35.1\n\n"},{"id":"452822","messageId":"20220331191525.17927-3-khalid.masum.92@gmail.com","threadId":"57639","inReplyTo":"20220331191525.17927-1-khalid.masum.92@gmail.com","subject":"[PATCH v2 2/3] t3501: don't ignore \"git <cmd>\" exit code","fromName":"Labnann","fromEmail":"khalid.masum.92@gmail.com","sentAt":"2022-03-31T19:15:24Z","receivedAt":"2022-03-31T19:15:43Z","isPatch":true,"sender":{"key":"khalid.masum.92@gmail.com","avatar":"https://avatars.githubusercontent.com/u/55809005?v=4"},"body":"without this change this test will become flaky e.g under\nSANITIZE=leak if some (but not all) memory leaks revealed by\nthese commands, according to c419562860e\n\nSigned-off-by: Labnann <khalid.masum.92@gmail.com>\n---\n t/t3501-revert-cherry-pick.sh | 10 ++++++++--\n 1 file changed, 8 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh\nindex 45492a7ed0..bd19c272d6 100755\n--- a/t/t3501-revert-cherry-pick.sh\n+++ b/t/t3501-revert-cherry-pick.sh\n@@ -44,6 +44,12 @@ test_expect_success setup '\n \tgit tag rename2\n '\n \n+test_cmp_rev_parse () {\n+\tgit rev-parse $1 >expect &&\n+\tgit rev-parse $2 >actual &&\n+\ttest_cmp expect actual\n+}\n+\n test_expect_success 'cherry-pick --nonsense' '\n \n \tpos=$(git rev-parse HEAD) &&\n@@ -66,7 +72,7 @@ test_expect_success 'cherry-pick after renaming branch' '\n \n \tgit checkout rename2 &&\n \tgit cherry-pick added &&\n-\ttest $(git rev-parse HEAD^) = $(git rev-parse rename2) &&\n+\ttest_cmp_rev_parse HEAD^ rename2 &&\n \ttest_path_is_file opos &&\n \tgrep \"Add extra line at the end\" opos &&\n \tgit reflog -1 | grep cherry-pick\n@@ -77,7 +83,7 @@ test_expect_success 'revert after renaming branch' '\n \n \tgit checkout rename1 &&\n \tgit revert added &&\n-\ttest $(git rev-parse HEAD^) = $(git rev-parse rename1) &&\n+\ttest_cmp_rev_parse HEAD^ rename1 &&\n \ttest_path_is_file spoo &&\n \t! grep \"Add extra line at the end\" spoo &&\n \tgit reflog -1 | grep revert\n-- \n2.35.1\n\n"},{"id":"452823","messageId":"20220331191525.17927-4-khalid.masum.92@gmail.com","threadId":"57639","inReplyTo":"20220331191525.17927-1-khalid.masum.92@gmail.com","subject":"[PATCH v2 3/3] t3501: test cherry-pick revert with oids","fromName":"Labnann","fromEmail":"khalid.masum.92@gmail.com","sentAt":"2022-03-31T19:15:25Z","receivedAt":"2022-03-31T19:15:45Z","isPatch":true,"sender":{"key":"khalid.masum.92@gmail.com","avatar":"https://avatars.githubusercontent.com/u/55809005?v=4"},"body":" we did a revert of a file we had so that it's the same as in\n\"initial\", but now it's at a different path, which we can exhaustively\ndo by checking the blob OIDs\n\nSigned-off-by: Labnann <khalid.masum.92@gmail.com>\n---\n t/t3501-revert-cherry-pick.sh | 8 +++-----\n 1 file changed, 3 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh\nindex bd19c272d6..08103923ab 100755\n--- a/t/t3501-revert-cherry-pick.sh\n+++ b/t/t3501-revert-cherry-pick.sh\n@@ -72,8 +72,7 @@ test_expect_success 'cherry-pick after renaming branch' '\n \n \tgit checkout rename2 &&\n \tgit cherry-pick added &&\n-\ttest_cmp_rev_parse HEAD^ rename2 &&\n-\ttest_path_is_file opos &&\n+\ttest_cmp_rev_parse rename2 HEAD^ &&\n \tgrep \"Add extra line at the end\" opos &&\n \tgit reflog -1 | grep cherry-pick\n \n@@ -83,9 +82,8 @@ test_expect_success 'revert after renaming branch' '\n \n \tgit checkout rename1 &&\n \tgit revert added &&\n-\ttest_cmp_rev_parse HEAD^ rename1 &&\n-\ttest_path_is_file spoo &&\n-\t! grep \"Add extra line at the end\" spoo &&\n+\ttest_cmp_rev_parse rename1 HEAD^ &&\n+\ttest_cmp_rev_parse initial:oops HEAD:spoo &&\n \tgit reflog -1 | grep revert\n \n '\n-- \n2.35.1\n\n"},{"id":"452914","messageId":"xmqqilrswu76.fsf@gitster.g","threadId":"57639","inReplyTo":"20220331191525.17927-3-khalid.masum.92@gmail.com","subject":"Re: [PATCH v2 2/3] t3501: don't ignore \"git <cmd>\" exit code","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-04-01T18:10:37Z","receivedAt":"2022-04-01T18:10:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Labnann <khalid.masum.92@gmail.com> writes:\n\n> without this change this test will become flaky e.g under\n> SANITIZE=leak if some (but not all) memory leaks revealed by\n> these commands, according to c419562860e\n\nThe body of the proposed log message is *not* a continuation (the\nlatter half) of a sentence that the title started, but should be a\nfull sentence on its own.  Capitalize the first word as usual.\n\nAlso, the leak check should not be the primary reason to do this\nchange.  We do not want to miss \"git rev-parse\" used in these tests\nstarts to fail, regardless of why they fail.  Perhaps like this\ninstead?\n\n    Otherwise, we would not notice when \"git rev-parse\" starts\n    segfaulting without emitting any output.  The 'test' command\n    would end up being just \"test =\", which yields success.\n\n> +test_cmp_rev_parse () {\n> +\tgit rev-parse $1 >expect &&\n> +\tgit rev-parse $2 >actual &&\n> +\ttest_cmp expect actual\n> +}\n\nTo me, it looks like we can just use test_cmp_rev that already\nexists, but am I missing some subtlety?\n\nIf not (then we need to explain why in the proposed commit log\nmessage), at least we should place $1 and $2 inside a pair of\ndouble-quotes.  For the current callers, what they pass happen not\nto need such quoting, but once we write a helper function, we are\nhelping _future_ callers as well, and we should be reasonably\nprepared for them.\n\n>  test_expect_success 'cherry-pick --nonsense' '\n>  \n>  \tpos=$(git rev-parse HEAD) &&\n> @@ -66,7 +72,7 @@ test_expect_success 'cherry-pick after renaming branch' '\n>  \n>  \tgit checkout rename2 &&\n>  \tgit cherry-pick added &&\n> -\ttest $(git rev-parse HEAD^) = $(git rev-parse rename2) &&\n> +\ttest_cmp_rev_parse HEAD^ rename2 &&\n\nOK.\n\n>  \ttest_path_is_file opos &&\n>  \tgrep \"Add extra line at the end\" opos &&\n>  \tgit reflog -1 | grep cherry-pick\n> @@ -77,7 +83,7 @@ test_expect_success 'revert after renaming branch' '\n>  \n>  \tgit checkout rename1 &&\n>  \tgit revert added &&\n> -\ttest $(git rev-parse HEAD^) = $(git rev-parse rename1) &&\n> +\ttest_cmp_rev_parse HEAD^ rename1 &&\n\nOK.\n\n>  \ttest_path_is_file spoo &&\n>  \t! grep \"Add extra line at the end\" spoo &&\n>  \tgit reflog -1 | grep revert\n\nThanks.\n"},{"id":"452915","messageId":"xmqq7d88wtjw.fsf@gitster.g","threadId":"57639","inReplyTo":"20220331191525.17927-4-khalid.masum.92@gmail.com","subject":"Re: [PATCH v2 3/3] t3501: test cherry-pick revert with oids","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-04-01T18:24:35Z","receivedAt":"2022-04-01T18:24:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Labnann <khalid.masum.92@gmail.com> writes:\n\n>  we did a revert of a file we had so that it's the same as in\n> \"initial\", but now it's at a different path, which we can exhaustively\n> do by checking the blob OIDs\n\nDon't indent the first line by a space, and capitalize the first\nletter of the full sentence as usual.  End the sentence with a full\nstop.\n\nI did not quite get the \"blob OIDs\" reference; does that refer to\nthe changes in both hunks?\n\n> Signed-off-by: Labnann <khalid.masum.92@gmail.com>\n> ---\n>  t/t3501-revert-cherry-pick.sh | 8 +++-----\n>  1 file changed, 3 insertions(+), 5 deletions(-)\n>\n> diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh\n> index bd19c272d6..08103923ab 100755\n> --- a/t/t3501-revert-cherry-pick.sh\n> +++ b/t/t3501-revert-cherry-pick.sh\n> @@ -72,8 +72,7 @@ test_expect_success 'cherry-pick after renaming branch' '\n>  \n>  \tgit checkout rename2 &&\n>  \tgit cherry-pick added &&\n> -\ttest_cmp_rev_parse HEAD^ rename2 &&\n> -\ttest_path_is_file opos &&\n> +\ttest_cmp_rev_parse rename2 HEAD^ &&\n>  \tgrep \"Add extra line at the end\" opos &&\n\nThis change is not quite explained anywhere.  Why do we have to swap\nHEAD^ and rename2?  I agree that we do not have to make sure that\nopos exists since we are going to run \"grep\" on it in a later step\nanyway, and the lack of that file will be detected as a failure,\nbut these two changes deserve mention in the proposed log message.\n\n>  \tgit reflog -1 | grep cherry-pick\n>  \n> @@ -83,9 +82,8 @@ test_expect_success 'revert after renaming branch' '\n>  \n>  \tgit checkout rename1 &&\n>  \tgit revert added &&\n> -\ttest_cmp_rev_parse HEAD^ rename1 &&\n> -\ttest_path_is_file spoo &&\n> -\t! grep \"Add extra line at the end\" spoo &&\n> +\ttest_cmp_rev_parse rename1 HEAD^ &&\n> +\ttest_cmp_rev_parse initial:oops HEAD:spoo &&\n\nAgain, did we need to swap and if so why?\n\nSo instead of allowing spoo to be some random garbage as long as it\ndoes not have the \"Add extra line\" message, we can exactly predict\nwhat the contents of the correct result, which is to end up with the\nidentical contents in oops of the initial commit.  OK, that makes\nsense.\n\n    Swap the order of revisions being compared in two tests for such\n    and such reasons.  In one test, stop checking for the presence\n    of a file (opos) because we are going to \"grep\" in it in the\n    same test and the lack of it will be noticed as a failure\n    anyway.  In the other test, instead of allowing any random\n    contents as long as a known phrase is not there in it, we can\n    expect the exact outcome---after the successful revert of\n    \"added\", the contents of file \"spoo\" should become identical to\n    what was in file \"oops\" in the \"initial\" commit.\n\nor something like that, perhaps.\n\nStrictly speaking, this used to check the working tree files but now\nit checks the contents of the resulting commit.  I wonder if we need\nto ensure that the HEAD, the index and the working tree are all\nupdated the same way, or if that is too basic, obvious, and (most\nimportantly) already tested elsewhere?\n\nThanks.\n\n>  \tgit reflog -1 | grep revert\n>  \n>  '\n"},{"id":"452967","messageId":"20220402192415.19023-1-khalid.masum.92@gmail.com","threadId":"57639","inReplyTo":"20220331191525.17927-1-khalid.masum.92@gmail.com","subject":"[PATCH v3 0/1][GSoC] t3501: remove redundant file checking and stop ignoring git <cmd> exit code","fromName":"Labnann","fromEmail":"khalid.masum.92@gmail.com","sentAt":"2022-04-02T19:24:14Z","receivedAt":"2022-04-02T19:25:02Z","isPatch":true,"sender":{"key":"khalid.masum.92@gmail.com","avatar":"https://avatars.githubusercontent.com/u/55809005?v=4"},"body":"From: Labnan <khalidmasum@iut-dhaka.edu>\n\nT3501 contains two test cases where we used to check if a file exists using\ntest -f which is not necessary because we already have grep after it. Also \nthe use of git rev-parse can be hidden using test_cmp_rev.\n\nThis patch tries to resolve these two issues in T3501.\n\nSorry for the extra email that was not sent to the right place. I am still\nstruggling to work with the mailing list.\n\nLabnann (1):\n  t3501: remove redundant file check and stop ignoring git <cmd> exit\n    code\n\n t/t3501-revert-cherry-pick.sh | 8 +++-----\n 1 file changed, 3 insertions(+), 5 deletions(-)\n\nRange-diff against v2:\n1:  8b7d38a66f < -:  ---------- t3501: use test_path_is_* functions\n2:  04b0bf1c5d < -:  ---------- t3501: don't ignore \"git <cmd>\" exit code\n3:  36445b40fb < -:  ---------- t3501: test cherry-pick revert with oids\n-:  ---------- > 1:  aeb28e3249 t3501: remove redundant file check and stop ignoring git <cmd> exit code\n-- \n2.35.1.windows.2\n\n"},{"id":"452968","messageId":"20220402192415.19023-2-khalid.masum.92@gmail.com","threadId":"57639","inReplyTo":"20220402192415.19023-1-khalid.masum.92@gmail.com","subject":"[PATCH v3 1/1] t3501: remove redundant file check and stop ignoring git <cmd> exit code","fromName":"Labnann","fromEmail":"khalid.masum.92@gmail.com","sentAt":"2022-04-02T19:24:15Z","receivedAt":"2022-04-02T19:25:05Z","isPatch":true,"sender":{"key":"khalid.masum.92@gmail.com","avatar":"https://avatars.githubusercontent.com/u/55809005?v=4"},"body":"In the test 'cherry-pick after renaming branch', stop checking for\nthe presence of a file (opos) because we are going to \"grep\" in it in\nthe same test and the lack of it will be noticed as a failure anyway.\n\nIn the test 'revert after renaming branch', instead of allowing any\nrandom contents as long as a known phrase is not there in it, we can\nexpect the exact outcome---after the successful revert of\n\"added\", the contents of file \"spoo\" should become identical to\nwhat was in file \"oops\" in the \"initial\" commit.\n\nIn both tests, we would not notice when \"git rev-parse\" starts\nsegfaulting without emitting any output.  The 'test' command would\nend up being just \"test =\", which yields success. Therefore we could\nuse test_cmp_rev\n\nSigned-off-by: Labnann <khalid.masum.92@gmail.com>\n\n---\n t/t3501-revert-cherry-pick.sh | 8 +++-----\n 1 file changed, 3 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh\nindex 8617efaaf1..ad8f0cae5a 100755\n--- a/t/t3501-revert-cherry-pick.sh\n+++ b/t/t3501-revert-cherry-pick.sh\n@@ -66,8 +66,7 @@ test_expect_success 'cherry-pick after renaming branch' '\n \n \tgit checkout rename2 &&\n \tgit cherry-pick added &&\n-\ttest $(git rev-parse HEAD^) = $(git rev-parse rename2) &&\n-\ttest -f opos &&\n+\ttest_cmp_rev rename2 HEAD^ &&\n \tgrep \"Add extra line at the end\" opos &&\n \tgit reflog -1 | grep cherry-pick\n \n@@ -77,9 +76,8 @@ test_expect_success 'revert after renaming branch' '\n \n \tgit checkout rename1 &&\n \tgit revert added &&\n-\ttest $(git rev-parse HEAD^) = $(git rev-parse rename1) &&\n-\ttest -f spoo &&\n-\t! grep \"Add extra line at the end\" spoo &&\n+\ttest_cmp_rev rename1 HEAD^ &&\n+\ttest_cmp_rev initial:oops HEAD:spoo &&\n \tgit reflog -1 | grep revert\n \n '\n-- \n2.35.1.windows.2\n\n"},{"id":"453040","messageId":"xmqqfsmssv1z.fsf@gitster.g","threadId":"57639","inReplyTo":"20220402192415.19023-2-khalid.masum.92@gmail.com","subject":"Re: [PATCH v3 1/1] t3501: remove redundant file check and stop ignoring git <cmd> exit code","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-04-04T15:54:48Z","receivedAt":"2022-04-04T15:54:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Labnann <khalid.masum.92@gmail.com> writes:\n\n> In the test 'cherry-pick after renaming branch', stop checking for\n> the presence of a file (opos) because we are going to \"grep\" in it in\n> the same test and the lack of it will be noticed as a failure anyway.\n\nOK.\n\n> In the test 'revert after renaming branch', instead of allowing any\n> random contents as long as a known phrase is not there in it, we can\n> expect the exact outcome---after the successful revert of\n> \"added\", the contents of file \"spoo\" should become identical to\n> what was in file \"oops\" in the \"initial\" commit.\n\nMakes sense.  The code removes \"test -f spoo\", which is not\nexplained in the above, and I do not think we want to.  We'd\nnotice a breakage where revert leaves the right result in HEAD\nbut fails to match the working tree files if we leave it there.\n\n> In both tests, we would not notice when \"git rev-parse\" starts\n> segfaulting without emitting any output.  The 'test' command would\n> end up being just \"test =\", which yields success. Therefore we could\n> use test_cmp_rev\n\nThe two sentences are right.  The conclusion is a bit iffy; it is\nnot \"we could\", which implies that the current one is OK but it is\nOK to also rewrite it to use test_cmp_rev.\n\n    Use the 'test_cmp_rev' helper to make sure we will notice such a\n    breakage.\n\nor something like that, perhaps?\n\n> Signed-off-by: Labnann <khalid.masum.92@gmail.com>\n\nIs Labnann your name?  I do not know where you are from so forgive\nme if it is.  We prefer people to sign with their real names (cf.\nDocumentation/SubmittingPatches[[real-name]]) on this line (and\nbecause the name/e-mail on this line must match the author's ident,\nthe same preference applies there, too).\n\n> ---\n>  t/t3501-revert-cherry-pick.sh | 8 +++-----\n>  1 file changed, 3 insertions(+), 5 deletions(-)\n>\n> diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh\n> index 8617efaaf1..ad8f0cae5a 100755\n> --- a/t/t3501-revert-cherry-pick.sh\n> +++ b/t/t3501-revert-cherry-pick.sh\n> @@ -66,8 +66,7 @@ test_expect_success 'cherry-pick after renaming branch' '\n>  \n>  \tgit checkout rename2 &&\n>  \tgit cherry-pick added &&\n> -\ttest $(git rev-parse HEAD^) = $(git rev-parse rename2) &&\n> -\ttest -f opos &&\n> +\ttest_cmp_rev rename2 HEAD^ &&\n>  \tgrep \"Add extra line at the end\" opos &&\n>  \tgit reflog -1 | grep cherry-pick\n>  \n> @@ -77,9 +76,8 @@ test_expect_success 'revert after renaming branch' '\n>  \n>  \tgit checkout rename1 &&\n>  \tgit revert added &&\n> -\ttest $(git rev-parse HEAD^) = $(git rev-parse rename1) &&\n> -\ttest -f spoo &&\n> -\t! grep \"Add extra line at the end\" spoo &&\n> +\ttest_cmp_rev rename1 HEAD^ &&\n> +\ttest_cmp_rev initial:oops HEAD:spoo &&\n>  \tgit reflog -1 | grep revert\n>  \n>  '\n\nThe diff looks almost perfect here (modulo that \"test -f spoo\" is\nbetter kept, I would think).\n\nThanks.\n"},{"id":"453128","messageId":"20220405150657.19801-1-khalid.masum.92@gmail.com","threadId":"57639","inReplyTo":"20220402192415.19023-1-khalid.masum.92@gmail.com","subject":"[PATCH v4 0/1][GSoC] t3501: remove test -f and stop ignoring git <cmd> exit code","fromName":"Khalid Masum","fromEmail":"khalid.masum.92@gmail.com","sentAt":"2022-04-05T15:06:56Z","receivedAt":"2022-04-05T21:49:50Z","isPatch":true,"sender":{"key":"khalid.masum.92@gmail.com","avatar":"https://avatars.githubusercontent.com/u/55809005?v=4"},"body":"Resending this email because the previous email did not seem to reach \ngitgitgadget's PR and the mailing list.\n\nT3501 contains two test cases where we used to check if a file exists using\ntest -f which is not necessary because we already have helper function for\nit. In fact in one test case test -f is redundant because we grep right\nafter it. Also the use of git rev-parse can cause segfault.\n\nThis patch tries to resolve these two issues in T3501.\n\nKhalid Masum (1):\n  t3501: remove test -f and stop ignoring git <cmd> exit code\n\n t/t3501-revert-cherry-pick.sh | 9 ++++-----\n 1 file changed, 4 insertions(+), 5 deletions(-)\n\nRange-diff against v3:\n1:  aeb28e3249 ! 1:  1090429b86 t3501: remove redundant file check and stop ignoring git <cmd> exit code\n    @@\n      ## Metadata ##\n    -Author: Labnann <khalid.masum.92@gmail.com>\n    +Author: Khalid Masum <khalid.masum.92@gmail.com>\n     \n      ## Commit message ##\n    -    t3501: remove redundant file check and stop ignoring git <cmd> exit code\n    +    t3501: remove test -f and stop ignoring git <cmd> exit code\n     \n         In the test 'cherry-pick after renaming branch', stop checking for\n         the presence of a file (opos) because we are going to \"grep\" in it in\n    @@ Commit message\n     \n         In the test 'revert after renaming branch', instead of allowing any\n         random contents as long as a known phrase is not there in it, we can\n    -    expect the exact outcome---after the successful revert of\n    -    \"added\", the contents of file \"spoo\" should become identical to\n    -    what was in file \"oops\" in the \"initial\" commit.\n    +    expect the exact outcome---after the successful revert of \"added\", the\n    +    contents of file \"spoo\" should become identical to what was in file\n    +    \"oops\" in the \"initial\" commit. This test also contains 'test -f' that\n    +    verifies presence of a file, but we have a helper function to do the same\n    +    thing. Replace it with appropriate helper function 'test_path_is_file'\n    +    for better readability and better error messages.\n     \n    -    In both tests, we would not notice when \"git rev-parse\" starts\n    -    segfaulting without emitting any output.  The 'test' command would\n    -    end up being just \"test =\", which yields success. Therefore we could\n    -    use test_cmp_rev\n    +    In both tests, we will not notice when \"git rev-parse\" starts segfaulting\n    +    without emitting any output.  The 'test' command will end up being just\n    +    \"test =\", which yields success. Use the 'test_cmp_rev' helper to make\n    +    sure we will notice such a breakage.\n     \n    -    Signed-off-by: Labnann <khalid.masum.92@gmail.com>\n    +    Signed-off-by: Khalid Masum <khalid.masum.92@gmail.com>\n     \n      ## t/t3501-revert-cherry-pick.sh ##\n     @@ t/t3501-revert-cherry-pick.sh: test_expect_success 'cherry-pick after renaming branch' '\n    @@ t/t3501-revert-cherry-pick.sh: test_expect_success 'revert after renaming branch\n     -\ttest -f spoo &&\n     -\t! grep \"Add extra line at the end\" spoo &&\n     +\ttest_cmp_rev rename1 HEAD^ &&\n    ++\ttest_path_is_file spoo &&\n     +\ttest_cmp_rev initial:oops HEAD:spoo &&\n      \tgit reflog -1 | grep revert\n      \n-- \n2.35.1.windows.2\n\n"},{"id":"453129","messageId":"20220405134742.17526-1-khalid.masum.92@gmail.com","threadId":"57639","inReplyTo":"20220402192415.19023-1-khalid.masum.92@gmail.com","subject":"[PATCH v4 0/1][GSoC] t3501: remove test -f and stop ignoring git <cmd> exit code","fromName":"Khalid Masum","fromEmail":"khalid.masum.92@gmail.com","sentAt":"2022-04-05T13:47:41Z","receivedAt":"2022-04-05T21:49:57Z","isPatch":true,"sender":{"key":"khalid.masum.92@gmail.com","avatar":"https://avatars.githubusercontent.com/u/55809005?v=4"},"body":"T3501 contains two test cases where we used to check if a file exists using\ntest -f which is not necessary because we already have helper function for\nit. In fact in one test case test -f is redundant because we grep right\nafter it. Also the use of git rev-parse can cause segfault.\n\nThis patch tries to resolve these two issues in T3501.\n\nKhalid Masum (1):\n  t3501: remove test -f and stop ignoring git <cmd> exit code\n\n t/t3501-revert-cherry-pick.sh | 9 ++++-----\n 1 file changed, 4 insertions(+), 5 deletions(-)\n\nRange-diff against v3:\n1:  aeb28e3249 ! 1:  1090429b86 t3501: remove redundant file check and stop ignoring git <cmd> exit code\n    @@\n      ## Metadata ##\n    -Author: Labnann <khalid.masum.92@gmail.com>\n    +Author: Khalid Masum <khalid.masum.92@gmail.com>\n     \n      ## Commit message ##\n    -    t3501: remove redundant file check and stop ignoring git <cmd> exit code\n    +    t3501: remove test -f and stop ignoring git <cmd> exit code\n     \n         In the test 'cherry-pick after renaming branch', stop checking for\n         the presence of a file (opos) because we are going to \"grep\" in it in\n    @@ Commit message\n     \n         In the test 'revert after renaming branch', instead of allowing any\n         random contents as long as a known phrase is not there in it, we can\n    -    expect the exact outcome---after the successful revert of\n    -    \"added\", the contents of file \"spoo\" should become identical to\n    -    what was in file \"oops\" in the \"initial\" commit.\n    +    expect the exact outcome---after the successful revert of \"added\", the\n    +    contents of file \"spoo\" should become identical to what was in file\n    +    \"oops\" in the \"initial\" commit. This test also contains 'test -f' that\n    +    verifies presence of a file, but we have a helper function to do the same\n    +    thing. Replace it with appropriate helper function 'test_path_is_file'\n    +    for better readability and better error messages.\n     \n    -    In both tests, we would not notice when \"git rev-parse\" starts\n    -    segfaulting without emitting any output.  The 'test' command would\n    -    end up being just \"test =\", which yields success. Therefore we could\n    -    use test_cmp_rev\n    +    In both tests, we will not notice when \"git rev-parse\" starts segfaulting\n    +    without emitting any output.  The 'test' command will end up being just\n    +    \"test =\", which yields success. Use the 'test_cmp_rev' helper to make\n    +    sure we will notice such a breakage.\n     \n    -    Signed-off-by: Labnann <khalid.masum.92@gmail.com>\n    +    Signed-off-by: Khalid Masum <khalid.masum.92@gmail.com>\n     \n      ## t/t3501-revert-cherry-pick.sh ##\n     @@ t/t3501-revert-cherry-pick.sh: test_expect_success 'cherry-pick after renaming branch' '\n    @@ t/t3501-revert-cherry-pick.sh: test_expect_success 'revert after renaming branch\n     -\ttest -f spoo &&\n     -\t! grep \"Add extra line at the end\" spoo &&\n     +\ttest_cmp_rev rename1 HEAD^ &&\n    ++\ttest_path_is_file spoo &&\n     +\ttest_cmp_rev initial:oops HEAD:spoo &&\n      \tgit reflog -1 | grep revert\n      \n-- \n2.35.1.windows.2\n\n"},{"id":"453136","messageId":"20220405134742.17526-2-khalid.masum.92@gmail.com","threadId":"57639","inReplyTo":"20220405134742.17526-1-khalid.masum.92@gmail.com","subject":"[PATCH v4 1/1] t3501: remove test -f and stop ignoring git <cmd> exit code","fromName":"Khalid Masum","fromEmail":"khalid.masum.92@gmail.com","sentAt":"2022-04-05T13:47:42Z","receivedAt":"2022-04-05T21:50:09Z","isPatch":true,"sender":{"key":"khalid.masum.92@gmail.com","avatar":"https://avatars.githubusercontent.com/u/55809005?v=4"},"body":"In the test 'cherry-pick after renaming branch', stop checking for\nthe presence of a file (opos) because we are going to \"grep\" in it in\nthe same test and the lack of it will be noticed as a failure anyway.\n\nIn the test 'revert after renaming branch', instead of allowing any\nrandom contents as long as a known phrase is not there in it, we can\nexpect the exact outcome---after the successful revert of \"added\", the\ncontents of file \"spoo\" should become identical to what was in file\n\"oops\" in the \"initial\" commit. This test also contains 'test -f' that\nverifies presence of a file, but we have a helper function to do the same\nthing. Replace it with appropriate helper function 'test_path_is_file'\nfor better readability and better error messages.\n\nIn both tests, we will not notice when \"git rev-parse\" starts segfaulting\nwithout emitting any output.  The 'test' command will end up being just\n\"test =\", which yields success. Use the 'test_cmp_rev' helper to make\nsure we will notice such a breakage.\n\nSigned-off-by: Khalid Masum <khalid.masum.92@gmail.com>\n---\n t/t3501-revert-cherry-pick.sh | 9 ++++-----\n 1 file changed, 4 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh\nindex 8617efaaf1..9eb19204ac 100755\n--- a/t/t3501-revert-cherry-pick.sh\n+++ b/t/t3501-revert-cherry-pick.sh\n@@ -66,8 +66,7 @@ test_expect_success 'cherry-pick after renaming branch' '\n \n \tgit checkout rename2 &&\n \tgit cherry-pick added &&\n-\ttest $(git rev-parse HEAD^) = $(git rev-parse rename2) &&\n-\ttest -f opos &&\n+\ttest_cmp_rev rename2 HEAD^ &&\n \tgrep \"Add extra line at the end\" opos &&\n \tgit reflog -1 | grep cherry-pick\n \n@@ -77,9 +76,9 @@ test_expect_success 'revert after renaming branch' '\n \n \tgit checkout rename1 &&\n \tgit revert added &&\n-\ttest $(git rev-parse HEAD^) = $(git rev-parse rename1) &&\n-\ttest -f spoo &&\n-\t! grep \"Add extra line at the end\" spoo &&\n+\ttest_cmp_rev rename1 HEAD^ &&\n+\ttest_path_is_file spoo &&\n+\ttest_cmp_rev initial:oops HEAD:spoo &&\n \tgit reflog -1 | grep revert\n \n '\n-- \n2.35.1.windows.2\n\n"},{"id":"453140","messageId":"20220405150657.19801-2-khalid.masum.92@gmail.com","threadId":"57639","inReplyTo":"20220405150657.19801-1-khalid.masum.92@gmail.com","subject":"[PATCH v4 1/1] t3501: remove test -f and stop ignoring git <cmd> exit code","fromName":"Khalid Masum","fromEmail":"khalid.masum.92@gmail.com","sentAt":"2022-04-05T15:06:57Z","receivedAt":"2022-04-05T21:50:09Z","isPatch":true,"sender":{"key":"khalid.masum.92@gmail.com","avatar":"https://avatars.githubusercontent.com/u/55809005?v=4"},"body":"In the test 'cherry-pick after renaming branch', stop checking for\nthe presence of a file (opos) because we are going to \"grep\" in it in\nthe same test and the lack of it will be noticed as a failure anyway.\n\nIn the test 'revert after renaming branch', instead of allowing any\nrandom contents as long as a known phrase is not there in it, we can\nexpect the exact outcome---after the successful revert of \"added\", the\ncontents of file \"spoo\" should become identical to what was in file\n\"oops\" in the \"initial\" commit. This test also contains 'test -f' that\nverifies presence of a file, but we have a helper function to do the same\nthing. Replace it with appropriate helper function 'test_path_is_file'\nfor better readability and better error messages.\n\nIn both tests, we will not notice when \"git rev-parse\" starts segfaulting\nwithout emitting any output.  The 'test' command will end up being just\n\"test =\", which yields success. Use the 'test_cmp_rev' helper to make\nsure we will notice such a breakage.\n\nSigned-off-by: Khalid Masum <khalid.masum.92@gmail.com>\n---\n t/t3501-revert-cherry-pick.sh | 9 ++++-----\n 1 file changed, 4 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh\nindex 8617efaaf1..9eb19204ac 100755\n--- a/t/t3501-revert-cherry-pick.sh\n+++ b/t/t3501-revert-cherry-pick.sh\n@@ -66,8 +66,7 @@ test_expect_success 'cherry-pick after renaming branch' '\n \n \tgit checkout rename2 &&\n \tgit cherry-pick added &&\n-\ttest $(git rev-parse HEAD^) = $(git rev-parse rename2) &&\n-\ttest -f opos &&\n+\ttest_cmp_rev rename2 HEAD^ &&\n \tgrep \"Add extra line at the end\" opos &&\n \tgit reflog -1 | grep cherry-pick\n \n@@ -77,9 +76,9 @@ test_expect_success 'revert after renaming branch' '\n \n \tgit checkout rename1 &&\n \tgit revert added &&\n-\ttest $(git rev-parse HEAD^) = $(git rev-parse rename1) &&\n-\ttest -f spoo &&\n-\t! grep \"Add extra line at the end\" spoo &&\n+\ttest_cmp_rev rename1 HEAD^ &&\n+\ttest_path_is_file spoo &&\n+\ttest_cmp_rev initial:oops HEAD:spoo &&\n \tgit reflog -1 | grep revert\n \n '\n-- \n2.35.1.windows.2\n\n"},{"id":"454255","messageId":"pull.1195.v2.git.1650626787628.gitgitgadget@gmail.com","threadId":"57639","inReplyTo":"pull.1195.git.1648676585765.gitgitgadget@gmail.com","subject":"[PATCH v2] t3501: remove test -f and stop ignoring git <cmd> exit code","fromName":"Labnan via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-04-22T11:26:27Z","receivedAt":"2022-04-22T11:26:35Z","isPatch":true,"sender":{"key":"name:Labnan","avatar":null},"body":"From: Khalid Masum <khalid.masum.92@gmail.com>\n\nIn the test 'cherry-pick after renaming branch', stop checking for\nthe presence of a file (opos) because we are going to \"grep\" in it in\nthe same test and the lack of it will be noticed as a failure anyway.\n\nIn the test 'revert after renaming branch', instead of allowing any\nrandom contents as long as a known phrase is not there in it, we can\nexpect the exact outcome---after the successful revert of \"added\", the\ncontents of file \"spoo\" should become identical to what was in file\n\"oops\" in the \"initial\" commit. This test also contains 'test -f' that\nverifies presence of a file, but we have a helper function to do the same\nthing. Replace it with appropriate helper function 'test_path_is_file'\nfor better readability and better error messages.\n\nIn both tests, we will not notice when \"git rev-parse\" starts segfaulting\nwithout emitting any output.  The 'test' command will end up being just\n\"test =\", which yields success. Use the 'test_cmp_rev' helper to make\nsure we will notice such a breakage.\n\nSigned-off-by: Khalid Masum <khalid.masum.92@gmail.com>\n---\n    t3501: remove redundant test -f and use of git rev-parse\n    \n    Two test -f are present in t3501. They can be replaced with appropriate\n    helper function: test_path_is_file. Which makes the script more readable\n    and gives better error messages.\n    \n    Signed-off-by: Labnann khalid.masum.92@gmail.com\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1195%2FLabnann%2Ft3501-helper-functions-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1195/Labnann/t3501-helper-functions-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1195\n\nRange-diff vs v1:\n\n 1:  8b7d38a66f8 ! 1:  1090429b865 t3501: use test_path_is_* functions\n     @@\n       ## Metadata ##\n     -Author: Labnann <khalid.masum.92@gmail.com>\n     +Author: Khalid Masum <khalid.masum.92@gmail.com>\n      \n       ## Commit message ##\n     -    t3501: use test_path_is_* functions\n     +    t3501: remove test -f and stop ignoring git <cmd> exit code\n      \n     -    Two test -f are present in t3501. They can be replaced with appropriate\n     -    helper function: test_path_is_file\n     +    In the test 'cherry-pick after renaming branch', stop checking for\n     +    the presence of a file (opos) because we are going to \"grep\" in it in\n     +    the same test and the lack of it will be noticed as a failure anyway.\n      \n     -    Signed-off-by: Labnann <khalid.masum.92@gmail.com>\n     +    In the test 'revert after renaming branch', instead of allowing any\n     +    random contents as long as a known phrase is not there in it, we can\n     +    expect the exact outcome---after the successful revert of \"added\", the\n     +    contents of file \"spoo\" should become identical to what was in file\n     +    \"oops\" in the \"initial\" commit. This test also contains 'test -f' that\n     +    verifies presence of a file, but we have a helper function to do the same\n     +    thing. Replace it with appropriate helper function 'test_path_is_file'\n     +    for better readability and better error messages.\n     +\n     +    In both tests, we will not notice when \"git rev-parse\" starts segfaulting\n     +    without emitting any output.  The 'test' command will end up being just\n     +    \"test =\", which yields success. Use the 'test_cmp_rev' helper to make\n     +    sure we will notice such a breakage.\n     +\n     +    Signed-off-by: Khalid Masum <khalid.masum.92@gmail.com>\n      \n       ## t/t3501-revert-cherry-pick.sh ##\n      @@ t/t3501-revert-cherry-pick.sh: test_expect_success 'cherry-pick after renaming branch' '\n     + \n       \tgit checkout rename2 &&\n       \tgit cherry-pick added &&\n     - \ttest $(git rev-parse HEAD^) = $(git rev-parse rename2) &&\n     +-\ttest $(git rev-parse HEAD^) = $(git rev-parse rename2) &&\n      -\ttest -f opos &&\n     -+\ttest_path_is_file opos &&\n     ++\ttest_cmp_rev rename2 HEAD^ &&\n       \tgrep \"Add extra line at the end\" opos &&\n       \tgit reflog -1 | grep cherry-pick\n       \n      @@ t/t3501-revert-cherry-pick.sh: test_expect_success 'revert after renaming branch' '\n     + \n       \tgit checkout rename1 &&\n       \tgit revert added &&\n     - \ttest $(git rev-parse HEAD^) = $(git rev-parse rename1) &&\n     +-\ttest $(git rev-parse HEAD^) = $(git rev-parse rename1) &&\n      -\ttest -f spoo &&\n     +-\t! grep \"Add extra line at the end\" spoo &&\n     ++\ttest_cmp_rev rename1 HEAD^ &&\n      +\ttest_path_is_file spoo &&\n     - \t! grep \"Add extra line at the end\" spoo &&\n     ++\ttest_cmp_rev initial:oops HEAD:spoo &&\n       \tgit reflog -1 | grep revert\n       \n     + '\n\n\n t/t3501-revert-cherry-pick.sh | 9 ++++-----\n 1 file changed, 4 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh\nindex 8617efaaf1e..9eb19204ac7 100755\n--- a/t/t3501-revert-cherry-pick.sh\n+++ b/t/t3501-revert-cherry-pick.sh\n@@ -66,8 +66,7 @@ test_expect_success 'cherry-pick after renaming branch' '\n \n \tgit checkout rename2 &&\n \tgit cherry-pick added &&\n-\ttest $(git rev-parse HEAD^) = $(git rev-parse rename2) &&\n-\ttest -f opos &&\n+\ttest_cmp_rev rename2 HEAD^ &&\n \tgrep \"Add extra line at the end\" opos &&\n \tgit reflog -1 | grep cherry-pick\n \n@@ -77,9 +76,9 @@ test_expect_success 'revert after renaming branch' '\n \n \tgit checkout rename1 &&\n \tgit revert added &&\n-\ttest $(git rev-parse HEAD^) = $(git rev-parse rename1) &&\n-\ttest -f spoo &&\n-\t! grep \"Add extra line at the end\" spoo &&\n+\ttest_cmp_rev rename1 HEAD^ &&\n+\ttest_path_is_file spoo &&\n+\ttest_cmp_rev initial:oops HEAD:spoo &&\n \tgit reflog -1 | grep revert\n \n '\n\nbase-commit: 805e0a68082a217f0112db9ee86a022227a9c81b\n-- \ngitgitgadget\n"}]}