Re: [PATCH v6 2/2] hook API: fix v2.36.0 regression: hooks should be connected to a TTY

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 7, 2022, 17:08 UTC
Message-ID: <xmqq7d5sz9iy.fsf@gitster.g>
In-Reply-To: <patch-v6-2.2-503ef241a52-20220606T170356Z-avarab@gmail.com>

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

Show 5 quoted lines

> @@ -120,4 +121,34 @@ test_expect_success 'git -c core.hooksPath=<PATH> hook run' '
>  	test_cmp expect actual
>  '
>  
> +test_hook_tty() {

Style.

Show 16 quoted lines

> +	cat >expect <<-\EOF
> +	STDOUT TTY
> +	STDERR TTY
> +	EOF
> +
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +
> +	test_commit -C repo A &&
> +	test_commit -C repo B &&
> +	git -C repo reset --soft HEAD^ &&
> +
> +	test_hook -C repo pre-commit <<-EOF &&
> +	test -t 1 && echo STDOUT TTY >>actual || echo STDOUT NO TTY >>actual &&
> +	test -t 2 && echo STDERR TTY >>actual || echo STDERR NO TTY >>actual
> +	EOF

So, when this hook is run, we'd see if STDOUT and STDERR are connected to a tty in the "actual" file.

> +	test_terminal git "$@" &&

And we run the test and see

> +	test_cmp expect repo/actual

what happens. The test_cmp knows that the git command runs in "repo" by hardcoding repo/actual, and this helper is full of the same knowledge, so it would be easier to see what is going on if you removed "-C repo" from the two callers (below) and instead added it to where you run "git" under test_terminal (above).

Show 11 quoted lines

> +}
> +
> +test_expect_success TTY 'git hook run: stdout and stderr are connected to a TTY' '
> +	test_hook_tty -C repo hook run pre-commit
> +'
> +
> +test_expect_success TTY 'git commit: stdout and stderr are connected to a TTY' '
> +	test_hook_tty -C repo commit -m"B.new"
> +'
> +
>  test_done

Other than that, looking good.

Thanks.