All of lore.kernel.org
 help / color / mirror / Atom feed
From: "SZEDER Gábor" <szeder.dev@gmail.com>
To: Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org,
	Johannes Schindelin <johannes.schindelin@gmx.de>,
	Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH 2/7] t3701: avoid depending on the TTY prerequisite
Date: Mon, 16 Dec 2019 13:18:59 +0100	[thread overview]
Message-ID: <20191216121859.GP6527@szeder.dev> (raw)
In-Reply-To: <ed870d34a8479366df786e76e2770df344469a41.1575637705.git.gitgitgadget@gmail.com>

On Fri, Dec 06, 2019 at 01:08:20PM +0000, Johannes Schindelin via GitGitGadget wrote:
> 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.

> +# 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.


  reply	other threads:[~2019-12-16 12:19 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-06 13:08 [PATCH 0/7] add -i: close some regression test gaps Johannes Schindelin via GitGitGadget
2019-12-06 13:08 ` [PATCH 1/7] t3701: add a test for advanced split-hunk editing Johannes Schindelin via GitGitGadget
2019-12-06 13:08 ` [PATCH 2/7] t3701: avoid depending on the TTY prerequisite Johannes Schindelin via GitGitGadget
2019-12-16 12:18   ` SZEDER Gábor [this message]
2019-12-17  5:53     ` Jeff King
2019-12-06 13:08 ` [PATCH 3/7] t3701: add a test for the different `add -p` prompts Johannes Schindelin via GitGitGadget
2019-12-06 13:08 ` [PATCH 4/7] t3701: verify the shown messages when nothing can be added Johannes Schindelin via GitGitGadget
2019-12-06 13:08 ` [PATCH 5/7] t3701: verify that the diff.algorithm config setting is handled Johannes Schindelin via GitGitGadget
2019-12-06 13:08 ` [PATCH 6/7] git add -p: use non-zero exit code when the diff generation failed Johannes Schindelin via GitGitGadget
2019-12-06 20:48   ` Junio C Hamano
2019-12-06 13:08 ` [PATCH 7/7] apply --allow-overlap: fix a corner case Johannes Schindelin via GitGitGadget
2019-12-06 13:45   ` Junio C Hamano
2019-12-06 14:11     ` Johannes Schindelin
2019-12-06 16:40       ` Junio C Hamano
2019-12-06 17:56         ` Johannes Schindelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191216121859.GP6527@szeder.dev \
    --to=szeder.dev@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.