From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
"Alex Henrie" <alexhenrie24@gmail.com>,
"Felipe Contreras" <felipe.contreras@gmail.com>,
"Fabian Stelzer" <fabian.stelzer@campoint.net>,
"Jeff King" <peff@peff.net>,
"Zbigniew Jędrzejewski-Szmek" <zbyszek@in.waw.pl>,
"SZEDER Gábor" <szeder.dev@gmail.com>,
"Eric Sunshine" <sunshine@sunshineco.com>,
"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH v3 3/3] test-lib.sh: use GIT_TEST_COLUMNS over COLUMNS
Date: Thu, 5 Aug 2021 01:05:46 +0200 [thread overview]
Message-ID: <patch-v3-3.3-74acba0f9ca-20210804T230335Z-avarab@gmail.com> (raw)
In-Reply-To: <cover-v3-0.3-00000000000-20210804T230335Z-avarab@gmail.com>
In my c49a177beca (test-lib.sh: set COLUMNS=80 for --verbose
repeatability, 2021-06-29) the test suite started breaking on recent
versions of bash.
This is because it sets "shopt -s checkwinsize" starting with version
5.0, furthermore it started setting COLUMNS under "shopt -s
checkwinsize" for non-interactive use around version 4.3.
A narrow fix for that issue would be to add this just above our
setting of COLUMNS in test-lib.sh:
shopt -u checkwinsize >/dev/null 2>&1
But we'd then be at the mercy of the next shell or terminal that wants
to be clever about COLUMNS.
Let's instead solve this more thoroughly. We'll now take
GIT_TEST_COLUMNS over COLUMNS, and furthermore intentionally spoil the
COLUMNS variable to break any tests that rely on it being set to a
sane value.
If something breaks because we have a codepath that's not
term_columns() checking COLUMNS we'd like to know about it, the narrow
"shopt -u checkwinsize" fix won't give us that.
The "shopt" fix won't future-poof us against other 3rd party software
changes either. If that third-party software e.g. takes TIOCGWINSZ
over columns on some platforms, our tests would be flaky and break
there even without this change.
This approach does mean that any tests of ours that expected to test
term_columns() behavior by setting COLUMNS will need to explicitly
unset GIT_TEST_COLUMNS, or set it to the empty string. Let's do that
in the new test_with_columns() helper, which we previously changed all
the tests that set COLUMNS to use.
Reported-by: Fabian Stelzer <fabian.stelzer@campoint.net>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
| 7 +++++++
t/test-lib-functions.sh | 1 +
t/test-lib.sh | 13 +++++++++++--
3 files changed, 19 insertions(+), 2 deletions(-)
--git a/pager.c b/pager.c
index 52f27a6765c..cfcc6dc04bd 100644
--- a/pager.c
+++ b/pager.c
@@ -165,6 +165,13 @@ int term_columns(void)
term_columns_at_startup = 80;
term_columns_guessed = 1;
+ col_string = getenv("GIT_TEST_COLUMNS");
+ if (col_string && (n_cols = atoi(col_string)) > 0) {
+ term_columns_at_startup = n_cols;
+ term_columns_guessed = 0;
+ return term_columns_at_startup;
+ }
+
col_string = getenv("COLUMNS");
if (col_string && (n_cols = atoi(col_string)) > 0) {
term_columns_at_startup = n_cols;
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index adc853109e4..0970c1293a8 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -1730,5 +1730,6 @@ test_with_columns () {
return 1
fi
+ GIT_TEST_COLUMNS= \
COLUMNS=$columns "$@" 2>&7
} 7>&2 2>&4
diff --git a/t/test-lib.sh b/t/test-lib.sh
index db61081d6b8..fc589226189 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -415,10 +415,19 @@ LANG=C
LC_ALL=C
PAGER=cat
TZ=UTC
-COLUMNS=80
-export LANG LC_ALL PAGER TZ COLUMNS
+export LANG LC_ALL PAGER TZ
EDITOR=:
+# For repeatability we need to set term_columns()'s idea of
+# columns. We do this via GIT_TEST_COLUMNS and not COLUMNS because
+# e.g. versions of bash >= 5.0 have "shopt -s checkwinsize" on by
+# default. We could do "shopt -u checkwinsize >/dev/null 2>&1" here to
+# fix that particular issue, but this is not shell specific, and
+# future-proof the tests.
+GIT_TEST_COLUMNS=80
+COLUMNS=10
+export GIT_TEST_COLUMNS COLUMNS
+
# 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
--
2.33.0.rc0.597.gc569a812f0a
prev parent reply other threads:[~2021-08-04 23:05 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-14 12:39 progress test failure on fedora34 Fabian Stelzer
2021-07-14 15:35 ` Ævar Arnfjörð Bjarmason
2021-07-14 16:35 ` Alex Henrie
2021-07-18 8:05 ` Ævar Arnfjörð Bjarmason
2021-07-19 9:00 ` Jeff King
2021-07-19 17:18 ` Alex Henrie
2021-07-19 18:21 ` Alex Henrie
2021-07-19 18:43 ` Felipe Contreras
2021-07-19 19:34 ` Felipe Contreras
2021-07-19 20:42 ` Alex Henrie
2021-07-20 0:40 ` Felipe Contreras
2021-07-21 0:45 ` ZheNing Hu
2021-07-21 2:50 ` Felipe Contreras
2021-07-26 23:57 ` [PATCH] test-lib.sh: use GIT_TEST_COLUMNS over COLUMNS Ævar Arnfjörð Bjarmason
2021-07-27 17:38 ` Jeff King
2021-07-28 0:53 ` Junio C Hamano
2021-08-02 13:46 ` [PATCH v2 0/3] " Ævar Arnfjörð Bjarmason
2021-08-02 13:46 ` [PATCH v2 1/3] test-lib-functions.sh: rename test_must_fail_acceptable() Ævar Arnfjörð Bjarmason
2021-08-02 13:46 ` [PATCH v2 2/3] test-lib-functions.sh: add a test_with_columns function Ævar Arnfjörð Bjarmason
2021-08-02 17:14 ` SZEDER Gábor
2021-08-02 17:24 ` Eric Sunshine
2021-08-02 13:46 ` [PATCH v2 3/3] test-lib.sh: use GIT_TEST_COLUMNS over COLUMNS Ævar Arnfjörð Bjarmason
2021-08-04 23:05 ` [PATCH v3 0/3] " Ævar Arnfjörð Bjarmason
2021-08-04 23:05 ` [PATCH v3 1/3] test-lib-functions.sh: rename test_must_fail_acceptable() Ævar Arnfjörð Bjarmason
2021-08-04 23:05 ` [PATCH v3 2/3] test-lib-functions.sh: add a test_with_columns function Ævar Arnfjörð Bjarmason
2021-08-04 23:05 ` Ævar Arnfjörð Bjarmason [this message]
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=patch-v3-3.3-74acba0f9ca-20210804T230335Z-avarab@gmail.com \
--to=avarab@gmail.com \
--cc=alexhenrie24@gmail.com \
--cc=fabian.stelzer@campoint.net \
--cc=felipe.contreras@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=peff@peff.net \
--cc=sunshine@sunshineco.com \
--cc=szeder.dev@gmail.com \
--cc=zbyszek@in.waw.pl \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).