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

["SZEDER Gábor" <szeder.dev@gmail.com>]

On Wed, Feb 05, 2020 at 04:40:56PM -0800, Emily Shaffer wrote:
> diff --git a/t/t0091-bugreport.sh b/t/t0091-bugreport.sh
> new file mode 100755
> index 0000000000..451badff0c
> --- /dev/null
> +++ b/t/t0091-bugreport.sh
> @@ -0,0 +1,49 @@
> +#!/bin/sh
> +
> +test_description='git bugreport'
> +
> +. ./test-lib.sh
> +
> +# Headers "[System Info]" will be followed by a non-empty line if we put some
> +# 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
> +			echo "$line"
> +			read -r nextline
> +			if test -z "$nextline"; then
> +				return 1;
> +			fi
> +		fi
> +	done
> +}
> +
> +test_expect_success 'creates a report with content in the right places' '
> +	git bugreport &&
> +	REPORT="$(ls git-bugreport-*)" &&

What if the globbing were to match more than one file?

> +	check_all_headers_populated <$REPORT &&
> +	rm $REPORT

Please register a cleanup commands like this with 'test_when_finished'.

> +'
> +
> +test_expect_success 'dies if file with same name as report already exists' '
> +	touch git-bugreport-duplicate.txt &&
> +	test_must_fail git bugreport --suffix duplicate &&
> +	rm git-bugreport-duplicate.txt
> +'
> +
> +test_expect_success '--output-directory puts the report in the provided dir' '
> +	mkdir foo/ &&

Is it really necessary to create the directory in advance?  Or to put
it in another way: shouldn't 'git bugreport' create any missing
leading directories of the path given for its '-o' option?  FWIW, 'git
format-patch -o dir ...' does create all necessary directories.

> +	git bugreport -o foo/ &&
> +	test_path_is_file foo/git-bugreport-* &&

What if the globbing were to match more than one file? :)

> +	rm -fr foo/
> +'
> +
> +test_expect_success 'incorrect arguments abort with usage' '
> +	test_must_fail git bugreport --false 2>output &&
> +	grep usage output &&

This breaks the GETTEXT_POISON CI job, because "usage" is translated,
so the test should not look for it with plain 'grep'.  Please use
'test_i18ngrep' instead.

> +	test_path_is_missing git-bugreport-*
> +'
> +
> +test_done
> -- 
> 2.25.0.341.g760bfbb309-goog
>