From: Eric Sunshine <sunshine@sunshineco.com>
To: Bagas Sanjaya <bagasdotme@gmail.com>
Cc: Git List <git@vger.kernel.org>,
Junio C Hamano <gitster@pobox.com>,
Christian Couder <chriscool@tuxfamily.org>,
Pranit Bauva <pranit.bauva@gmail.com>,
Ramsay Jones <ramsay@ramsayjones.plus.com>,
Trygve Aaberge <trygveaa@gmail.com>
Subject: Re: [PATCH v3] t6030: add test for git bisect skip started with --term* arguments
Date: Wed, 28 Apr 2021 12:55:33 -0400 [thread overview]
Message-ID: <CAPig+cSNWdUmPCQiErm41fzvDFRdegfkExUveYYN61Mrj_X2gQ@mail.gmail.com> (raw)
In-Reply-To: <20210428113805.109528-1-bagasdotme@gmail.com>
On Wed, Apr 28, 2021 at 7:39 AM Bagas Sanjaya <bagasdotme@gmail.com> wrote:
> Trygve Aaberge reported git bisect breakage when the bisection
> is started with --term* arguments (--term-new and --term-old).
>
> For example, suppose that we have repository with 10 commits, and we
> start the bisection from HEAD to first commit (HEAD~9) with:
>
> $ git bisect start --term-new=fixed --term-old=unfixed HEAD HEAD~9
>
> The bisection then stopped at HEAD~5 (fifth commit), and we choose
> to skip (git bisect skip). The HEAD should now at HEAD~4 (sixth commit).
> In the breakage, however, the HEAD after skipping stayed at HEAD~5
> (not changed).
>
> The breakage is caused by forgetting to read '.git/BISECT_TERMS' during
> implementation of `'bisect skip' subcommand in C.
>
> Let's add the test to catch the breakage. Now that the corresponding
> fix had been integrated, flip the switch to test_expect_success.
The final sentence about flipping the switch should probably be
dropped since this patch now introduces the new test in its "success"
state.
> Reported-by: Trygve Aaberge <trygveaa@gmail.com>
> Signed-off-by: Bagas Sanjaya <bagasdotme@gmail.com>
> ---
>
> Changes from v2 [1]:
> * remove double quotes inside test name
> * double-quote HASH_SKIPPED_FROM and HASH_SKIPPED_TO in the
> test comparison line
> * rename test name to be simpler
> * commit message now includes proper explanation why git bisect skip
> is currently broken
> * because the fix to the breakage had just been landed on seen, flip
> the switch to test_expect_success.
Here in the patch commentary, it does indeed make sense to mention
that you flipped the state from "failure" to "success" between
iterations of the patch.
prev parent reply other threads:[~2021-04-28 16:55 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-28 11:38 [PATCH v3] t6030: add test for git bisect skip started with --term* arguments Bagas Sanjaya
2021-04-28 16:55 ` Eric Sunshine [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAPig+cSNWdUmPCQiErm41fzvDFRdegfkExUveYYN61Mrj_X2gQ@mail.gmail.com \
--to=sunshine@sunshineco.com \
--cc=bagasdotme@gmail.com \
--cc=chriscool@tuxfamily.org \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=pranit.bauva@gmail.com \
--cc=ramsay@ramsayjones.plus.com \
--cc=trygveaa@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).