Re: [RFC PATCH v4] Implement --first-parent for git rev-list --bisect

[Junio C Hamano <gitster@pobox.com>]

Tiago Botelho <tiagonbotelho@gmail.com> writes:

> diff --git a/bisect.c b/bisect.c
> index 4eafc8262..e58cb8d62 100644
> --- a/bisect.c
> +++ b/bisect.c
> @@ -33,6 +33,8 @@ static const char *term_good;
>   *
>   * We care just barely enough to avoid recursing for
>   * non-merge entries.
> + *
> + * Note: This function does not support the usage --first-parent.
>   */

Hmph, is this because we know --first-parent codepath currently does
not call this function, so we do not bother to prepare this function
to be called from --first-parent codepath?

I am not saying that we must prepare this function to be callable
with --first-parent; if I have to wonder why the above comment is
there and what it is trying to say, I suspect most other readers
would, too, so...

> diff --git a/t/t6002-rev-list-bisect.sh b/t/t6002-rev-list-bisect.sh
> index a66140803..774d9a4fd 100755
> --- a/t/t6002-rev-list-bisect.sh
> +++ b/t/t6002-rev-list-bisect.sh
> @@ -263,4 +263,41 @@ test_expect_success 'rev-parse --bisect can default to good/bad refs' '
>  	test_cmp expect.sorted actual.sorted
>  '
>  
> +# We generate the following commit graph:
> +#
> +#   B - C
> +#  /      \
> +# A        FX
> +#  \      /
> +#   D - EX
> +
> +test_expect_success 'setup' '
> +  test_commit A &&
> +  test_commit B &&
> +  test_commit C &&
> +  git reset --hard A &&
> +  test_commit D &&
> +  test_commit EX &&
> +  test_merge FX C
> +'
> +
> +test_output_expect_success "--bisect --first-parent" 'git rev-list --bisect --first-parent FX ^A' <<EOF
> +$(git rev-parse EX)
> +EOF
> +
> +test_output_expect_success "--bisect-vars --first-parent" 'git rev-list --bisect-vars --first-parent FX ^A' <<EOF
> +bisect_rev='$(git rev-parse EX)'
> +bisect_nr=1
> +bisect_good=0
> +bisect_bad=1
> +bisect_all=3
> +bisect_steps=1
> +EOF
> +
> +test_output_expect_success "--bisect-all --first-parent" 'git rev-list --bisect-all --first-parent FX ^A' <<EOF
> +$(git rev-parse EX) (dist=1)
> +$(git rev-parse D) (dist=1)
> +$(git rev-parse FX) (dist=0)
> +EOF
> +

These are all good basic tests, but can you come up with a test that
demonstrates breakage in the previous round that has been fixed in
this version of the patch?

Thanks.