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

From: Stephan Beyer <s-beyer@gmx.net>
Date: Feb 26, 2016, 21:38 UTC
Message-ID: <56D0C5E0.2020703@gmx.net>
In-Reply-To: <CAP8UFD2szf46skWmgZi3kSkh3D0aeMPw4TagUQa7KZ-z6pHdAA@mail.gmail.com>

Hi Christian,

On 02/26/2016 07:53 AM, Christian Couder wrote:

Show 8 quoted lines

>> +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".

Why is this? I understand the && instead of -a thing (test atomicity), however, for || this results in an ugly

+       git bisect next &&
+       ( test "$(git rev-parse HEAD)" = "$(git rev-parse A3)" ||
+         test "$(git rev-parse HEAD)" = "$(git rev-parse A4)" )

Right? (Otherwise a failure of e.g. "git bisect start A7" would run the command after || (which may still be fine in some cases but is wrong in most of the other cases).

However, what do you think about this?

diff --git a/t/t8010-bisect-algorithm.sh b/t/t8010-bisect-algorithm.sh
index bda59da..ae50e7c 100755
--- a/t/t8010-bisect-algorithm.sh
+++ b/t/t8010-bisect-algorithm.sh
@@ -8,6 +8,16 @@ exec </dev/null

 . ./test-lib.sh

+test_compare_rev () {
+       arg="$(git rev-parse "$1")"
+       shift
+       for rev
+       do
+               test "$arg" = "$(git rev-parse "$rev")" && return 0
+       done
+       return 1
+}
+
 test_expect_success 'set up a history for the test' '
        test_commit A1 A 1 &&
        test_commit A2 A 2 &&
@@ -48,27 +58,25 @@ test_expect_success 'set up a history for the test' '
 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)"
+       test_compare_rev HEAD A3 A4
 '

and so on...
See
https://github.com/sbeyer/git/commit/2c224093ccee837a7f0f62f6af6a0a804d07c022

(test_compare_rev() could also go into test-lib.sh)

Cheers
Stephan