On Thu 18-11-21 17:05:41, Taylor Blau wrote: > On Thu, Nov 18, 2021 at 05:49:15PM +0100, Jan Kara wrote: > > > Test 17 from t6030-bisect-porcelain.sh assumes that bisection algorithm > > suggests first HASH3 where HASH2 and HASH3 are equivalent choices. Make > > sure test correctly handles both choices, add test variant to properly > > test commit skipping in the second case. > > OK, makes sense-ish: at least in the context of preparing for the > bisection algorithm to change. The subject line leaves a bit to be > desired, though. Perhaps: > > t6030: handle equivalent bisection points gracefully Sure, I can improve all subjects to test updates like this. > > diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh > > index 1be85d064e76..f8cfdd3c36d2 100755 > > --- a/t/t6030-bisect-porcelain.sh > > +++ b/t/t6030-bisect-porcelain.sh > > @@ -197,11 +197,27 @@ test_expect_success 'bisect skip: successful result' ' > > test_when_finished git bisect reset && > > git bisect reset && > > git bisect start $HASH4 $HASH1 && > > - git bisect skip && > > + if [ $(git rev-parse HEAD) == $HASH3 ]; then > > This is somewhat uncommon style for Git's test suite. It might be more > appropriate to write instead: > > if test "$HASH3" = "$(git rev-parse HEAD)" > then > git bisect skip > fi && > # ... Sure. Will do. > > +# $HASH1 is good, $HASH4 is bad, we skip $HASH2 > > +# but $HASH3 is good, > > It looks like this comment should have gone above the start of the test > in the previous hunk. > > But it looks like you accidentally duplicated this test in its entirety > (with the addition of the misplaced comment) below instead. No, I think the comment and the test are correct. The first test tests situation H1--H2--H3--H4 ^ ^ ^ ^ | bad | bad good skipped the second test tests situation H1--H2--H3--H4 ^ ^ ^ ^ | skipped bad good good So in both cases we can decide about the bad commit besides the skipped commit. And if the bisection algorithm picks H2 out of H2/H3 (which are equivalent) then second test tests this situation fully, if the bisection algorithm picks H3, then the first test tests this situation fully. Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
