Re: [PATCH v3] pager: fix crash when pager program doesn't exist

[Eric Sunshine <sunshine@sunshineco.com>]

On Wed, Nov 24, 2021 at 9:54 AM Enzo Matsumiya <ematsumiya@suse.de> wrote:
> [...]
> Reproducer:
> $ git config pager.show INVALID_PAGER
> $ git show $VALID_COMMIT
> error: cannot run INVALID_PAGER: No such file or directory
> [1]    3619 segmentation fault (core dumped)  git show $VALID_COMMIT
>
> Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
> ---
> diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
> @@ -786,4 +786,9 @@ test_expect_success TTY 'git returns SIGPIPE on propagated signals from pager' '
> +test_expect_success TTY 'handle attempt to run an invalid pager' '
> +       test_config pager.show invalid-pager &&
> +       test_terminal git show
> +'

This is a minor observation (so you decide what value it might have),
but the terminology "handle ... invalid pager" in the test title
doesn't convey very much information to some future reader of this
test, and that person will be forced to consult the commit message --
which does a good job of explaining the problem -- to really
understand what this test is checking. If you change the title to, for
instance:

    non-existent pager doesn't cause crash

then the reader will have an easier time understanding what this test
is about. It's true that the reader will still need to consult the
commit message for a detailed picture of the problem, but won't be
left head-scratching, as might be the case with the current test
title.