{"thread":{"id":"55575","subject":"[PATCH v3] t6030: add test for git bisect skip started with --term* arguments","startedAt":"2021-04-28T11:39:39Z","lastAt":"2021-04-28T16:55:47Z","messageCount":2,"participants":["Bagas Sanjaya","Eric Sunshine"],"isPatch":true,"patchVersion":3,"patchTotal":null},"messages":[{"id":"423192","messageId":"20210428113805.109528-1-bagasdotme@gmail.com","threadId":"55575","inReplyTo":null,"subject":"[PATCH v3] t6030: add test for git bisect skip started with --term* arguments","fromName":"Bagas Sanjaya","fromEmail":"bagasdotme@gmail.com","sentAt":"2021-04-28T11:38:06Z","receivedAt":"2021-04-28T11:39:39Z","isPatch":true,"sender":{"key":"bagasdotme@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40219486?v=4"},"body":"Trygve Aaberge reported git bisect breakage when the bisection\nis started with --term* arguments (--term-new and --term-old).\n\nFor example, suppose that we have repository with 10 commits, and we\nstart the bisection from HEAD to first commit (HEAD~9) with:\n\n  $ git bisect start --term-new=fixed --term-old=unfixed HEAD HEAD~9\n\nThe bisection then stopped at HEAD~5 (fifth commit), and we choose\nto skip (git bisect skip). The HEAD should now at HEAD~4 (sixth commit).\nIn the breakage, however, the HEAD after skipping stayed at HEAD~5\n(not changed).\n\nThe breakage is caused by forgetting to read '.git/BISECT_TERMS' during\nimplementation of `'bisect skip' subcommand in C.\n\nLet's add the test to catch the breakage. Now that the corresponding\nfix had been integrated, flip the switch to test_expect_success.\n\nReported-by: Trygve Aaberge <trygveaa@gmail.com>\nSigned-off-by: Bagas Sanjaya <bagasdotme@gmail.com>\n---\n\n Changes from v2 [1]:\n   * remove double quotes inside test name\n   * double-quote HASH_SKIPPED_FROM and HASH_SKIPPED_TO in the\n     test comparison line\n   * rename test name to be simpler\n   * commit message now includes proper explanation why git bisect skip\n     is currently broken\n   * because the fix to the breakage had just been landed on seen, flip\n     the switch to test_expect_success.\n   * give credit to Trygve in form of Reported-by\n\n[1]: https://lore.kernel.org/git/20210425080508.154159-1-bagasdotme@gmail.com/\n\n t/t6030-bisect-porcelain.sh | 11 +++++++++++\n 1 file changed, 11 insertions(+)\n\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex 32bb66e1ed..ca3a1de433 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -922,6 +922,17 @@ test_expect_success 'bisect start takes options and revs in any order' '\n \ttest_cmp expected actual\n '\n \n+# Bisect is started with --term-new and --term-old arguments,\n+# then skip. The HEAD should be changed.\n+test_expect_success 'bisect skip works with --term*' '\n+\tgit bisect reset &&\n+\tgit bisect start --term-new=fixed --term-old=unfixed HEAD $HASH1 &&\n+\tHASH_SKIPPED_FROM=$(git rev-parse --verify HEAD) &&\n+\tgit bisect skip &&\n+\tHASH_SKIPPED_TO=$(git rev-parse --verify HEAD) &&\n+\ttest \"$HASH_SKIPPED_FROM\" != \"$HASH_SKIPPED_TO\"\n+'\n+\n test_expect_success 'git bisect reset cleans bisection state properly' '\n \tgit bisect reset &&\n \tgit bisect start &&\n\nbase-commit: 311531c9de557d25ac087c1637818bd2aad6eb3a\n-- \n2.25.1\n\n"},{"id":"423204","messageId":"CAPig+cSNWdUmPCQiErm41fzvDFRdegfkExUveYYN61Mrj_X2gQ@mail.gmail.com","threadId":"55575","inReplyTo":"20210428113805.109528-1-bagasdotme@gmail.com","subject":"Re: [PATCH v3] t6030: add test for git bisect skip started with --term* arguments","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-04-28T16:55:33Z","receivedAt":"2021-04-28T16:55:47Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Apr 28, 2021 at 7:39 AM Bagas Sanjaya <bagasdotme@gmail.com> wrote:\n> Trygve Aaberge reported git bisect breakage when the bisection\n> is started with --term* arguments (--term-new and --term-old).\n>\n> For example, suppose that we have repository with 10 commits, and we\n> start the bisection from HEAD to first commit (HEAD~9) with:\n>\n>   $ git bisect start --term-new=fixed --term-old=unfixed HEAD HEAD~9\n>\n> The bisection then stopped at HEAD~5 (fifth commit), and we choose\n> to skip (git bisect skip). The HEAD should now at HEAD~4 (sixth commit).\n> In the breakage, however, the HEAD after skipping stayed at HEAD~5\n> (not changed).\n>\n> The breakage is caused by forgetting to read '.git/BISECT_TERMS' during\n> implementation of `'bisect skip' subcommand in C.\n>\n> Let's add the test to catch the breakage. Now that the corresponding\n> fix had been integrated, flip the switch to test_expect_success.\n\nThe final sentence about flipping the switch should probably be\ndropped since this patch now introduces the new test in its \"success\"\nstate.\n\n> Reported-by: Trygve Aaberge <trygveaa@gmail.com>\n> Signed-off-by: Bagas Sanjaya <bagasdotme@gmail.com>\n> ---\n>\n>  Changes from v2 [1]:\n>    * remove double quotes inside test name\n>    * double-quote HASH_SKIPPED_FROM and HASH_SKIPPED_TO in the\n>      test comparison line\n>    * rename test name to be simpler\n>    * commit message now includes proper explanation why git bisect skip\n>      is currently broken\n>    * because the fix to the breakage had just been landed on seen, flip\n>      the switch to test_expect_success.\n\nHere in the patch commentary, it does indeed make sense to mention\nthat you flipped the state from \"failure\" to \"success\" between\niterations of the patch.\n"}]}