Re: [PATCH 02/16] bisect: add test for the bisect algorithm

[Christian Couder <christian.couder@gmail.com>]

On Fri, Feb 26, 2016 at 3:04 AM, Stephan Beyer <s-beyer@gmx.net> wrote:
> Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
> ---
>
> To be honest: the test could be better, it could be more "targeted",
> i.e. the example commit history could be smaller and just consider
> all the cases and corner cases and whatever.
> However, I made it first to understand the algorithm and verify the
> description of it in the documentation. Then I was too lazy to
> improve it. I am sorry that this is no better advertising text. ;)
>
> Moreover, the test does not test one important thing that is
> always cared about in the bisect code: TREESAME commits.
> Perhaps I got the concept wrong. I tried to obtain TREESAME commits
> using 'git commit --allow-empty -m "same tree"'. However, those
> commits were never considered being TREESAME. So I gave up (I did
> not care much.)

I didn't care enough to test TREESAME either.

> Anyone has an idea how to obtain them?
> Or is this a bug that should be fixed?
>
> (Also UNINTERESTING commits are never found by the DEBUG_BISECT
> output, but I think this is because they are just filtered out.)

Yeah, I think so.

>  t/t8010-bisect-algorithm.sh | 162 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 162 insertions(+)
>  create mode 100755 t/t8010-bisect-algorithm.sh
>
> diff --git a/t/t8010-bisect-algorithm.sh b/t/t8010-bisect-algorithm.sh
> new file mode 100755
> index 0000000..bda59da
> --- /dev/null
> +++ b/t/t8010-bisect-algorithm.sh
> @@ -0,0 +1,162 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2016 Stephan Beyer
> +#
> +test_description='Tests git bisect algorithm'
> +
> +exec </dev/null
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'set up a history for the test' '
> +       test_commit A1 A 1 &&
> +       test_commit A2 A 2 &&
> +       test_commit A3 A 3 &&
> +       test_commit A4 A 4 &&
> +       test_commit A5 A 5 &&
> +       test_commit A6 A 6 &&
> +       git checkout -b b A5 &&
> +       test_commit B1 B 1 &&
> +       git checkout master &&
> +       test_commit A7 A 7 &&
> +       git checkout b &&
> +       test_commit B2 B 2 &&
> +       git checkout master &&
> +       test_commit A8 A 8 &&
> +       test_merge Bmerge b &&
> +       git checkout b &&
> +       test_commit B3 B 3 &&
> +       git checkout -b c A7 &&
> +       test_commit C1 C 1 &&
> +       git checkout -b d A3 &&
> +       test_commit D1 D 1 &&
> +       git checkout c &&
> +       test_commit C2 C 2 &&
> +       git checkout d &&
> +       test_commit D2 D 2 &&
> +       git checkout c &&
> +       test_commit C3 C 3 &&
> +       git checkout master &&
> +       git merge -m BCDmerge b c d &&
> +       git tag BCDmerge &&
> +       test_commit A9 A 9 &&
> +       git checkout d &&
> +       test_commit D3 &&
> +       git checkout master
> +'
> +
> +test_expect_success 'bisect algorithm works in linear history with an odd number of commits' '
> +       git bisect start A7 &&
> +       git bisect next &&
> +       test "$(git rev-parse HEAD)" = "$(git rev-parse A3)" \
> +         -o "$(git rev-parse HEAD)" = "$(git rev-parse A4)"

I thought that we should not use "-o" and "-a" but instead "|| test"
and "&& test".

> +'
> +
> +test_expect_success 'bisect algorithm works in linear history with an even number of commits' '
> +       git bisect reset &&
> +       git bisect start A8 &&
> +       git bisect next &&
> +       test "$(git rev-parse HEAD)" = "$(git rev-parse A4)"
> +'
> +
> +test_expect_success 'bisect algorithm works with a merge' '
> +       git bisect reset &&
> +       git bisect start Bmerge &&
> +       git bisect next &&
> +       test "$(git rev-parse HEAD)" = "$(git rev-parse A5)" &&
> +       git bisect good &&
> +       test "$(git rev-parse HEAD)" = "$(git rev-parse A8)" &&
> +       git bisect good &&
> +       test "$(git rev-parse HEAD)" = "$(git rev-parse B2)" \
> +         -o "$(git rev-parse HEAD)" = "$(git rev-parse B1)"

Here and in other places too...