Re: [PATCH 03/27] bisect: Fixup test bisect-porcelain/20

[Jan Kara <jack@suse.cz>]

On Thu 18-11-21 17:10:43, Taylor Blau wrote:
> On Thu, Nov 18, 2021 at 12:13:21PM -0800, Chris Torek wrote:
> > On Thu, Nov 18, 2021 at 10:39 AM Jan Kara <jack@suse.cz> wrote:
> > > diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
> > > index f8cfdd3c36d2..13f7deea4d81 100755
> > > --- a/t/t6030-bisect-porcelain.sh
> > > +++ b/t/t6030-bisect-porcelain.sh
> > > @@ -240,8 +240,13 @@ test_expect_success 'bisect skip: cannot tell between 3 commits' '
> > >  test_expect_success 'bisect skip: cannot tell between 2 commits' '
> > >         test_when_finished git bisect reset &&
> > >         git bisect start $HASH4 $HASH1 &&
> > > -       git bisect skip &&
> > > -       test_expect_code 2 git bisect good >my_bisect_log.txt &&
> > > +       if [ $(git rev-parse HEAD) == $HASH2 ]; then
> > > +               results=('good' 'skip')
> > > +       else
> > > +               results=('skip' 'good')
> > > +       fi &&
> > > +       git bisect ${results[0]} &&
> > > +       test_expect_code 2 git bisect ${results[1]} >my_bisect_log.txt &&
> >
> > These are also not available in old POSIX shell - consider using two
> > separate variables to hold the two strings.
> 
> Or just inlining the commands that you actually want to run inside of
> the if statement above:
> 
>     if test "$HASH2" = "$(git rev-parse HEAD)
>     then
>       git bisect good &&
>       test_expect_code 2 git bisect skip >my_bisect_log.txt
>     else
>       git bisect skip &&
>       test_expect_code 2 git bisect good >my_bisect_log.txt
>     fi && #...
> 
> Here (and in the previous patch) it might be helpful to add a short note
> in these conditionals, maybe along the lines of:
> 
>     # HASH2 and HASH3 are equivalent choices, but we only want to mark
>     # HASH2 as "good". Handle either ordering:
> 
> Same note on the brevity of the subject line applies here, too.

Sure, I'll fix these. Thanks for review.

								Honza

-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR