Re: [PATCH v2] bugreport: collect list of populated hooks

From: Emily Shaffer <emilyshaffer@google.com>
Date: Apr 30, 2020, 00:01 UTC
Message-ID: <20200430000133.GD77802@google.com>
In-Reply-To: <20200427234510.GC36078@google.com>

On Mon, Apr 27, 2020 at 04:45:10PM -0700, Jonathan Nieder wrote:

Show 17 quoted lines

> 
> Emily Shaffer wrote:
> 
> > --- a/t/t0091-bugreport.sh
> > +++ b/t/t0091-bugreport.sh
> > @@ -57,5 +57,15 @@ test_expect_success 'can create leading directories outside of a git dir' '
> >  	nongit git bugreport -o foo/bar/baz
> >  '
> >  
> > +test_expect_success 'indicates populated hooks' '
> > +	test_when_finished rm git-bugreport-hooks.txt &&
> > +	test_when_finished rm -fr .git/hooks &&
> > +	mkdir .git/hooks &&
> > +	touch .git/hooks/applypatch-msg &&
> > +	chmod +x .git/hooks/applypatch-msg &&
> 
> optional: could use write_script for this

ACK, especially given Junio's comment in reply here.

Show 10 quoted lines

> 
> > +	git bugreport -s hooks &&
> > +	test_i18ngrep applypatch-msg git-bugreport-hooks.txt
> 
> This should work even when translated, so it can use "grep" instead of
> test_i18ngrep.
> 
> A few more things to test:
> - that it doesn't include hooks we *haven't* installed. :)
> - that it isn't confused by the default *.sample hooks

I'll lump these together by adding a .sample hook and ensuring that hook was excluded.

> - what happens when outside a git repository?

The earlier test to make sure it doesn't crash seems satisfactory to me
- is there some other behavior you'd like to ensure? I'm not keen to
check the lines below the header are empty, since the spacing there
could change and break an unrelated test.

Reroll coming today. These changes are straightforward and passing for me locally.

 - Emily