Subject: Re: [PATCH v6 03/15] bugreport: add tool to generate debugging info

From: Emily Shaffer <[email-masked]>
Date: Wed, 12 Feb 2020 14:36:42 -0800

On Wed, Feb 12, 2020 at 10:06:44AM -0800, Junio C Hamano wrote:
> As this topic seems to break under GIT_TEST_GETTEXT_POISON=yes, I'll
> apply the following band-aid on top before merging it to 'pu'.  
> 
> Most of them are style fixes, but quoting $REPORT, not just makes
> the redirection honor the coding guidelines, will ensure that an
> error is caught if git-bugreport-* glob matches no paths or more
> than on paths.

Thanks. Most of these I have locally, but not all. I'll apply this as a
fixup on patch 3/n (the one that introduces the tests) and add a
Helped-by line.

I'm hoping to get a reroll out today; it was the first thing on my list
this week, and then a high-priority internal issue shows up (so it
goes). Sorry for the inconvenience with the CI failures.

 - Emily

>  t/t0091-bugreport.sh | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/t/t0091-bugreport.sh b/t/t0091-bugreport.sh
> index 451badff0c..f2186941ce 100755
> --- a/t/t0091-bugreport.sh
> +++ b/t/t0091-bugreport.sh
> @@ -8,9 +8,12 @@ test_description='git bugreport'
>  # information there; we can make sure all our headers were followed by some
>  # information to check if the command was successful.
>  HEADER_PATTERN="^\[.*\]$"
> -check_all_headers_populated() {
> -	while read -r line; do
> -		if test "$(grep "$HEADER_PATTERN" "$line")"; then
> +
> +check_all_headers_populated () {
> +	while read -r line
> +	do
> +		if test "$(grep "$HEADER_PATTERN" "$line")"
> +		then
>  			echo "$line"
>  			read -r nextline
>  			if test -z "$nextline"; then
> @@ -23,12 +26,12 @@ check_all_headers_populated() {
>  test_expect_success 'creates a report with content in the right places' '
>  	git bugreport &&
>  	REPORT="$(ls git-bugreport-*)" &&
> -	check_all_headers_populated <$REPORT &&
> -	rm $REPORT
> +	check_all_headers_populated <"$REPORT" &&
> +	rm "$REPORT"
>  '
>  
>  test_expect_success 'dies if file with same name as report already exists' '
> -	touch git-bugreport-duplicate.txt &&
> +	>>git-bugreport-duplicate.txt &&
>  	test_must_fail git bugreport --suffix duplicate &&
>  	rm git-bugreport-duplicate.txt
>  '
> @@ -42,7 +45,7 @@ test_expect_success '--output-directory puts the report in the provided dir' '
>  
>  test_expect_success 'incorrect arguments abort with usage' '
>  	test_must_fail git bugreport --false 2>output &&
> -	grep usage output &&
> +	test_i18ngrep usage output &&
>  	test_path_is_missing git-bugreport-*
>  '
>  
> -- 
> 2.25.0-455-gcf3c3a5ab4
>