diff mbox series

[v3] t6030: add test for git bisect skip started with --term* arguments

Message ID 20210428113805.109528-1-bagasdotme@gmail.com (mailing list archive)
State Superseded
Headers show
Series [v3] t6030: add test for git bisect skip started with --term* arguments | expand

Commit Message

Bagas Sanjaya April 28, 2021, 11:38 a.m. UTC
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.

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.
   * give credit to Trygve in form of Reported-by

[1]: https://lore.kernel.org/git/20210425080508.154159-1-bagasdotme@gmail.com/

 t/t6030-bisect-porcelain.sh | 11 +++++++++++
 1 file changed, 11 insertions(+)


base-commit: 311531c9de557d25ac087c1637818bd2aad6eb3a

Comments

Eric Sunshine April 28, 2021, 4:55 p.m. UTC | #1
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.
diff mbox series

Patch

diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 32bb66e1ed..ca3a1de433 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -922,6 +922,17 @@  test_expect_success 'bisect start takes options and revs in any order' '
 	test_cmp expected actual
 '
 
+# Bisect is started with --term-new and --term-old arguments,
+# then skip. The HEAD should be changed.
+test_expect_success 'bisect skip works with --term*' '
+	git bisect reset &&
+	git bisect start --term-new=fixed --term-old=unfixed HEAD $HASH1 &&
+	HASH_SKIPPED_FROM=$(git rev-parse --verify HEAD) &&
+	git bisect skip &&
+	HASH_SKIPPED_TO=$(git rev-parse --verify HEAD) &&
+	test "$HASH_SKIPPED_FROM" != "$HASH_SKIPPED_TO"
+'
+
 test_expect_success 'git bisect reset cleans bisection state properly' '
 	git bisect reset &&
 	git bisect start &&