On Thu, Jun 24, 2021 at 12:19:31PM +0200, Ævar Arnfjörð Bjarmason wrote:
> Some tests will fail under --verbose because while we've unset COLUMNS
> since b1d645b58ac (tests: unset COLUMNS inherited from environment,
> 2012-03-27), we also look for the columns with an ioctl(..,
> TIOCGWINSZ, ...) on some platforms. By setting COLUMNS again we
> preempt the TIOCGWINSZ lookup in pager.c's term_columns(), it'll take
> COLUMNS over TIOCGWINSZ,
>
> This fixes the t0500-progress-display.sh test when run as:
>
> ./t0500-progress-display.sh --verbose
>
> It broke because of a combination of the this issue and the progress
> output reacting to the column width since 545dc345ebd (progress: break
> too long progress bar lines, 2019-04-12). The
> t5324-split-commit-graph.sh fails in a similar manner due to progress
> output, see [1] for details.
This issue is not progress-specific.
I run the test suite with 'export COLUMNS=10' in 'test-lib.sh' and
got:
t0500-progress-display.sh (Wstat: 256 Tests: 12 Failed: 10)
Failed tests: 1-5, 7-11
t4012-diff-binary.sh (Wstat: 256 Tests: 13 Failed: 1)
Failed test: 13
t4052-stat-output.sh (Wstat: 256 Tests: 78 Failed: 14)
Failed tests: 3-5, 13, 17, 21, 38-41, 49, 52, 55, 57
t5510-fetch.sh (Wstat: 256 Tests: 181 Failed: 1)
Failed test: 175
t5324-split-commit-graph.sh (Wstat: 256 Tests: 36 Failed: 1)
Failed test: 22
t5150-request-pull.sh (Wstat: 256 Tests: 10 Failed: 1)
Failed test: 6
t4016-diff-quote.sh (Wstat: 256 Tests: 5 Failed: 1)
Failed test: 5
Then with COLUMNS=238:
t0500-progress-display.sh (Wstat: 256 Tests: 12 Failed: 4)
Failed tests: 3-6
t4012-diff-binary.sh (Wstat: 256 Tests: 13 Failed: 1)
Failed test: 13
t4052-stat-output.sh (Wstat: 256 Tests: 78 Failed: 3)
Failed tests: 3-5
>From these only the failures in t0500 and t5324 are caused by the
progress display, the rest fail with things like:
--- expect 2021-06-26 17:07:58.489958439 +0000
+++ actual 2021-06-26 17:07:58.957964694 +0000
@@ -1,2 +1,2 @@
binfile | Bin 0 -> 1026 bytes
- textfile | 10000 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+ textfile | 10000 ++++++++
--- expect80 2021-06-26 17:07:58.321956193 +0000
+++ actual 2021-06-26 17:07:58.333956354 +0000
@@ -1 +1 @@
- ...aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1 +
+ ...aaaaaaaaaaaa | 1 +
--- expect 2021-06-26 17:09:05.198849586 +0000
+++ actual 2021-06-26 17:09:05.194849532 +0000
@@ -1,2 +1,2 @@
-main -> origin/main
+main -> origin/main
looooooooooooong-tag -> looooooooooooong-tag
--- expect 2021-06-26 17:14:31.819199273 +0000
+++ request.fuzzy 2021-06-26 17:14:31.951201026 +0000
@@ -16,4 +16,5 @@
----------------------------------------------------------------
SHORTLOG
-DIFFSTAT
+ ...nic.txt | 5 +++++
+ 1 file changed, 5 insertions(+)
--- expect 2021-06-26 16:55:41.632510754 +0000
+++ actual 2021-06-26 16:55:42.052516149 +0000
@@ -1,2 +1,2 @@
binfile | Bin 0 -> 1026 bytes
- textfile | 10000 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+ textfile | 10000 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
--- expect80 2021-06-26 16:55:41.416507979 +0000
+++ actual 2021-06-26 16:55:41.436508236 +0000
@@ -1 +1 @@
- ...aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1 +
+ aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1 +
So there are more diffstat-related failures than progress-related.
> A more narrow fix here would be to only do this in the --verbose mode,
> but there's no harm in setting this for everything. If we're not
> connected to a TTY the COLUMNS setting won't matter.
This is wrong, we check the terminal width even when not connected to
a TTY, therefore COLUMNS definitely matters; all the failures reported
above were with '--verbose-log'.
> See ea77e675e56 (Make "git help" react to window size correctly,
> 2005-12-18) and ad6c3739a33 (pager: find out the terminal width before
> spawning the pager, 2012-02-12) for how the TIOCGWINSZ code ended up
> in pager.c
>
> 1. http://lore.kernel.org/git/20210624051253.GG6312@xxxxxxxxxx
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
> ---
>
> This replaces a more narrow fix in
> https://lore.kernel.org/git/patch-1.1-cba5d88ca35-20210621T070114Z-avarab@xxxxxxxxx/,
> which as SZEDER points out was also needed by another test using the
> progress.c code.
>
> t/test-lib.sh | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 54938c64279..1a6ca772d6c 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -406,14 +406,15 @@ LANG=C
> LC_ALL=C
> PAGER=cat
> TZ=UTC
> -export LANG LC_ALL PAGER TZ
> +COLUMNS=80
> +export LANG LC_ALL PAGER TZ COLUMNS
> EDITOR=:
>
> # A call to "unset" with no arguments causes at least Solaris 10
> # /usr/xpg4/bin/sh and /bin/ksh to bail out. So keep the unsets
> # deriving from the command substitution clustered with the other
> # ones.
> -unset VISUAL EMAIL LANGUAGE COLUMNS $("$PERL_PATH" -e '
> +unset VISUAL EMAIL LANGUAGE $("$PERL_PATH" -e '
> my @env = keys %ENV;
> my $ok = join("|", qw(
> TRACE
> --
> 2.32.0.605.g06ef811e153
>
