From: Trevor Saunders <tbsaunde@tbsaunde.org>
Date: Tue, 12 May 2015 02:10:24 GMT
Subject: Re: [PATCH] bisect: improve output when bad commit is found
Message-ID: <20150512021024.GE18112@tsaunders-iceball.corp.tor1.mozilla.com>
In-Reply-To: <xmqqbnhq1r9t.fsf@gitster.dls.corp.google.com>

On Mon, May 11, 2015 at 07:08:46PM -0700, Junio C Hamano wrote:
> Trevor Saunders <tbsaunde@tbsaunde.org> writes:
> 
> >> > +	file=$1
> >> > +	hash=$2
> >> > +	grep "the first bad commit is" $file || return $?
> >> > +	grep $hash $file || return $?
> >> 
> >> Is it OK to have these strings anywhere in the $file?
> >
> > Its not great, but the test seems to log multiple invokations of git
> > bisect into the same file, so there may be text about previous runs
> > before we are told which commit is bad.
> 
> So if we had a previous entry that happens to match $hash, even if
> the current test stopped and pointed at a different thing, this test
> declares a success?

err yeah, didn't think of that :(

> This function knows how the $file should end, so it might be more
> sensible to craft the expected output and compare the tail end of
> the $file with it, something like:
> 
> 	(
> 		echo "The first bad commit is"
>                 git show -s "$hash"
> 	) >expect &&
>         cnt=$(wc -l <expect) &&
>         tail -n $cnt "$file" >actual &&
>         test_cmp expect actual
> 
> perhaps?

seems about right.

Thanks!

Trev