Re: [PATCH 2/7] t3701: avoid depending on the TTY prerequisite

From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Dec 16, 2019, 12:18 UTC
Message-ID: <20191216121859.GP6527@szeder.dev>
In-Reply-To: <ed870d34a8479366df786e76e2770df344469a41.1575637705.git.gitgitgadget@gmail.com>

On Fri, Dec 06, 2019 at 01:08:20PM +0000, Johannes Schindelin via GitGitGadget wrote:

Show 17 quoted lines

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> The TTY prerequisite is a rather heavy one: it not only requires Perl to
> work, but also the IO/Pty.pm module (with native support, and it
> requires pseudo terminals, too).
> 
> In particular, test cases marked with the TTY prerequisite would be
> skipped in Git for Windows' SDK.
> 
> In the case of `git add -p`, we do not actually need that big a hammer,
> as we do not want to test any functionality that requires a pseudo
> terminal; all we want is for the interactive add command to use color,
> even when being called from within the test suite.
> 
> And we found exactly such a trick earlier already: when we added a test
> case to verify that the main loop of `git add -i` is colored
> appropriately. Let's use that trick instead of the TTY prerequisite.

It's much more interesting _what_ that trick is than when it was found. Is it setting TERM=vt100, or is it setting both TERM=vt100 and GIT_PAGER_IN_USE=true? I'm inclined to think the latter, but I'm not sure I interpreted the comment below right.

> +# This function uses a trick to manipulate the interactive add to use color:
> +# the `want_color()` function special-cases the situation where a pager was
> +# spawned and Git now wants to output colored text: to detect that situation,
> +# the environment variable `GIT_PAGER_IN_USE` is set. However, color is

Perhaps a s/is set/has to be set/ would have helped my interpreter, dunno.

Show 6 quoted lines

> +# suppressed despite that environment variable if the `TERM` variable
> +# indicates a dumb terminal, so we set that variable, too.
> +
> +force_color () {
> +	env GIT_PAGER_IN_USE=true TERM=vt100 "$@"
> +}

In any case, there are a couple of tests in other test scripts that test color relying on the TTY prereq. So maybe it would be worth to make this into a "global" helper function by adding it to 'test-lib-functions.sh', so we can drop a few more prereqs.

OTOH, some of those other tests have descriptions like:

  t3203-branch-output.sh:test_expect_success TTY '%(color) present with tty'
  t7004-tag.sh:test_expect_success TTY '%(color) present with tty'

i.e. their description is specific about checking the behaviour with a tty, so I'm not entirely sure.