progress test failure on fedora34

expecting success of 0500.3 'progress display breaks long lines #1':
     sed -e "s/Z$//" >expect <<\EOF &&
Working hard.......2.........3.........4.........5.........6:   0% 
(100/100000)<CR>
Working hard.......2.........3.........4.........5.........6:   1% 
(1000/100000)<CR>
Working hard.......2.........3.........4.........5.........6: Z
    10% (10000/100000)<CR>
   100% (100000/100000)<CR>
   100% (100000/100000), done.
EOF

     cat >in <<-\EOF &&
     progress 100
     progress 1000
     progress 10000
     progress 100000
     EOF
     test-tool progress --total=100000 \
         "Working hard.......2.........3.........4.........5.........6" \
         <in 2>stderr &&

     show_cr <stderr >out &&
     test_cmp expect out

@@ -1,6 +1,5 @@
  Working hard.......2.........3.........4.........5.........6:   0% 
(100/100000)<CR>
  Working hard.......2.........3.........4.........5.........6:   1% 
(1000/100000)<CR>
-Working hard.......2.........3.........4.........5.........6:
-   10% (10000/100000)<CR>
-  100% (100000/100000)<CR>
-  100% (100000/100000), done.
+Working hard.......2.........3.........4.........5.........6:  10% 
(10000/100000)<CR>
+Working hard.......2.........3.........4.........5.........6: 100% 
(100000/100000)<CR>
+Working hard.......2.........3.........4.........5.........6: 100% 
(100000/100000), done.
error: last command exited with $?=1
not ok 3 - progress display breaks long lines #1
#
#        sed -e "s/Z$//" >expect <<\EOF &&
#    Working hard.......2.........3.........4.........5.........6: 0% 
(100/100000)<CR>
#    Working hard.......2.........3.........4.........5.........6: 1% 
(1000/100000)<CR>
#    Working hard.......2.........3.........4.........5.........6: Z
#       10% (10000/100000)<CR>
#      100% (100000/100000)<CR>
#      100% (100000/100000), done.
#    EOF
#
#        cat >in <<-\EOF &&
#        progress 100
#        progress 1000
#        progress 10000
#        progress 100000
#        EOF
#        test-tool progress --total=100000 \
#            "Working 
hard.......2.........3.........4.........5.........6" \
#            <in 2>stderr &&
#
#        show_cr <stderr >out &&
#        test_cmp expect out
#

Ævar Arnfjörð Bjarmason· Jul 14, 2021, 15:35 UTC · re: Fabian Stelzer · lore

Re: progress test failure on fedora34

On Wed, Jul 14 2021, Fabian Stelzer wrote:

> Hi,
> The test t0500-progress-display.sh in current master fails on latest
> fedora34.
> The break was introduced with:
>
> 83ae1edff7ee0b7674bd556955d2cf1706bddb21
> ab/fix-columns-to-80-during-tests (2021-06-29) 1 commit
>
> Kind regards,
> Fabian

Alex Henrie· Jul 14, 2021, 16:35 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: progress test failure on fedora34

>
>
> On Wed, Jul 14 2021, Fabian Stelzer wrote:
>
> > Hi,
> > The test t0500-progress-display.sh in current master fails on latest
> > fedora34.
> > The break was introduced with:
> >
> > 83ae1edff7ee0b7674bd556955d2cf1706bddb21
> > ab/fix-columns-to-80-during-tests (2021-06-29) 1 commit
> >
> > Kind regards,
> > Fabian
>
> I have not been able to reproduce this, it seems the below E-Mail was
> word-wrapped by your mailer, which is especially bad here since getting
> to the bottom of this requires looking at the whitespace.
>
> Is there a way you could tar that up and send it (to me personally is
> fine, or some pastebin or whatever).
>
> I am able to reproduce something that looks like this if I
> s/COLUMNS=80/COLUMNS=79/g in the test-lib, but given that we set it to
> 80, and that the progress.c code just ends up with an
> atoi(getenv("COLUMNS")), and we do our own wrapping (with no other fancy
> logic) in progress.c, I'm not seeing right now how this could happen...

-Alex

Ævar Arnfjörð Bjarmason· Jul 18, 2021, 08:05 UTC · re: Alex Henrie · lore

Re: progress test failure on fedora34

On Wed, Jul 14 2021, Alex Henrie wrote:

> On Wed, Jul 14, 2021 at 9:39 AM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>>
>>
>> On Wed, Jul 14 2021, Fabian Stelzer wrote:
>>
>> > Hi,
>> > The test t0500-progress-display.sh in current master fails on latest
>> > fedora34.
>> > The break was introduced with:
>> >
>> > 83ae1edff7ee0b7674bd556955d2cf1706bddb21
>> > ab/fix-columns-to-80-during-tests (2021-06-29) 1 commit
>> >
>> > Kind regards,
>> > Fabian
>>
>> I have not been able to reproduce this, it seems the below E-Mail was
>> word-wrapped by your mailer, which is especially bad here since getting
>> to the bottom of this requires looking at the whitespace.
>>
>> Is there a way you could tar that up and send it (to me personally is
>> fine, or some pastebin or whatever).
>>
>> I am able to reproduce something that looks like this if I
>> s/COLUMNS=80/COLUMNS=79/g in the test-lib, but given that we set it to
>> 80, and that the progress.c code just ends up with an
>> atoi(getenv("COLUMNS")), and we do our own wrapping (with no other fancy
>> logic) in progress.c, I'm not seeing right now how this could happen...
>
> This test also fails for me when using QTerminal or Konsole, but it
> passes on XTerm and LXTerminal.

diff --git a/progress.c b/progress.c
index 680c6a8bf9..dca254b515 100644
--- a/progress.c
+++ b/progress.c
@@ -144,6 +144,7 @@ static void display(struct progress *progress, uint64_t n, const char *done)
                        size_t progress_line_len = progress->title_len +
                                                counters_sb->len + 2;
                        int cols = term_columns();
+                       fprintf(stderr, "cols = %d\n", cols);
 
                        if (progress->split) {
                                fprintf(stderr, "  %s%*s", counters_sb->buf,

Jeff King· Jul 19, 2021, 09:00 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: progress test failure on fedora34

On Sun, Jul 18, 2021 at 10:05:44AM +0200, Ævar Arnfjörð Bjarmason wrote:

> > This test also fails for me when using QTerminal or Konsole, but it
> > passes on XTerm and LXTerminal.
> 
> I tried this on Debian 11 with QTerminal 0.16.1 and can't reproduce it,
> resized the window etc., always get COLUMNS=80 if I add some printf
> debugging.
> 
> Do you mind testing with an ad-hoc patch like this on top? It will fail
> right away, but should say COLUMNS = 80 in the output.
> 
> The only thing I can think of right now is that some terminals are doing
> some evil trickery to LD_PRELOAD or whatever and intercept getenv() for
> COLUMNS and the like, but that's an entirely unfounded hunch.

That would be truly evil. :)

  $ echo $COLUMNS
  119
  $ COLUMNS=80 bash -c 'echo $COLUMNS'
  80
  $ COLUMNS=80 zsh -c 'echo $COLUMNS'
  119

  $ zsh -c 'COLUMNS=80; echo $COLUMNS; COLUMNS=foo; echo $COLUMNS'
  80
  119

> diff --git a/progress.c b/progress.c
> index 680c6a8bf9..dca254b515 100644
> --- a/progress.c
> +++ b/progress.c
> @@ -144,6 +144,7 @@ static void display(struct progress *progress, uint64_t n, const char *done)
>                         size_t progress_line_len = progress->title_len +
>                                                 counters_sb->len + 2;
>                         int cols = term_columns();
> +                       fprintf(stderr, "cols = %d\n", cols);
>  
>                         if (progress->split) {
>                                 fprintf(stderr, "  %s%*s", counters_sb->buf,

-Peff

Alex Henrie· Jul 19, 2021, 17:18 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: progress test failure on fedora34

>
> On Wed, Jul 14 2021, Alex Henrie wrote:
>
> > On Wed, Jul 14, 2021 at 9:39 AM Ævar Arnfjörð Bjarmason
> > <avarab@gmail.com> wrote:
> >>
> >> On Wed, Jul 14 2021, Fabian Stelzer wrote:
> >>
> >> > Hi,
> >> > The test t0500-progress-display.sh in current master fails on latest
> >> > fedora34.
> >> > The break was introduced with:
> >> >
> >> > 83ae1edff7ee0b7674bd556955d2cf1706bddb21
> >> > ab/fix-columns-to-80-during-tests (2021-06-29) 1 commit
> >> >
> >> > Kind regards,
> >> > Fabian
> >>
> >> I have not been able to reproduce this, it seems the below E-Mail was
> >> word-wrapped by your mailer, which is especially bad here since getting
> >> to the bottom of this requires looking at the whitespace.
> >>
> >> Is there a way you could tar that up and send it (to me personally is
> >> fine, or some pastebin or whatever).
> >>
> >> I am able to reproduce something that looks like this if I
> >> s/COLUMNS=80/COLUMNS=79/g in the test-lib, but given that we set it to
> >> 80, and that the progress.c code just ends up with an
> >> atoi(getenv("COLUMNS")), and we do our own wrapping (with no other fancy
> >> logic) in progress.c, I'm not seeing right now how this could happen...
> >
> > This test also fails for me when using QTerminal or Konsole, but it
> > passes on XTerm and LXTerminal.
>
> I tried this on Debian 11 with QTerminal 0.16.1 and can't reproduce it,
> resized the window etc., always get COLUMNS=80 if I add some printf
> debugging.

-Alex

Alex Henrie· Jul 19, 2021, 18:21 UTC · re: Alex Henrie · lore

Re: progress test failure on fedora34

On Mon, Jul 19, 2021 at 11:18 AM Alex Henrie <alexhenrie24@gmail.com> wrote:

>
> On Sun, Jul 18, 2021 at 2:08 AM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
> >
> > On Wed, Jul 14 2021, Alex Henrie wrote:
> >
> > > On Wed, Jul 14, 2021 at 9:39 AM Ævar Arnfjörð Bjarmason
> > > <avarab@gmail.com> wrote:
> > >>
> > >> On Wed, Jul 14 2021, Fabian Stelzer wrote:
> > >>
> > >> > Hi,
> > >> > The test t0500-progress-display.sh in current master fails on latest
> > >> > fedora34.
> > >> > The break was introduced with:
> > >> >
> > >> > 83ae1edff7ee0b7674bd556955d2cf1706bddb21
> > >> > ab/fix-columns-to-80-during-tests (2021-06-29) 1 commit
> > >> >
> > >> > Kind regards,
> > >> > Fabian
> > >>
> > >> I have not been able to reproduce this, it seems the below E-Mail was
> > >> word-wrapped by your mailer, which is especially bad here since getting
> > >> to the bottom of this requires looking at the whitespace.
> > >>
> > >> Is there a way you could tar that up and send it (to me personally is
> > >> fine, or some pastebin or whatever).
> > >>
> > >> I am able to reproduce something that looks like this if I
> > >> s/COLUMNS=80/COLUMNS=79/g in the test-lib, but given that we set it to
> > >> 80, and that the progress.c code just ends up with an
> > >> atoi(getenv("COLUMNS")), and we do our own wrapping (with no other fancy
> > >> logic) in progress.c, I'm not seeing right now how this could happen...
> > >
> > > This test also fails for me when using QTerminal or Konsole, but it
> > > passes on XTerm and LXTerminal.
> >
> > I tried this on Debian 11 with QTerminal 0.16.1 and can't reproduce it,
> > resized the window etc., always get COLUMNS=80 if I add some printf
> > debugging.
>
> Actually, it looks like the difference was that I didn't resize the
> XTerm or LXTerminal windows. The tests pass on all four if the
> terminal emulator window is exactly 80 columns wide, and they fail on
> all four if the window is any wider or narrower.

-Alex

Felipe Contreras· Jul 19, 2021, 18:43 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: progress test failure on fedora34

Ævar Arnfjörð Bjarmason wrote:

> 
> On Wed, Jul 14 2021, Alex Henrie wrote:
> 
> > On Wed, Jul 14, 2021 at 9:39 AM Ævar Arnfjörð Bjarmason
> > <avarab@gmail.com> wrote:
> >>
> >>
> >> On Wed, Jul 14 2021, Fabian Stelzer wrote:
> >>
> >> > Hi,
> >> > The test t0500-progress-display.sh in current master fails on latest
> >> > fedora34.
> >> > The break was introduced with:
> >> >
> >> > 83ae1edff7ee0b7674bd556955d2cf1706bddb21
> >> > ab/fix-columns-to-80-during-tests (2021-06-29) 1 commit
> >> >
> >> > Kind regards,
> >> > Fabian
> >>
> >> I have not been able to reproduce this, it seems the below E-Mail was
> >> word-wrapped by your mailer, which is especially bad here since getting
> >> to the bottom of this requires looking at the whitespace.
> >>
> >> Is there a way you could tar that up and send it (to me personally is
> >> fine, or some pastebin or whatever).
> >>
> >> I am able to reproduce something that looks like this if I
> >> s/COLUMNS=80/COLUMNS=79/g in the test-lib, but given that we set it to
> >> 80, and that the progress.c code just ends up with an
> >> atoi(getenv("COLUMNS")), and we do our own wrapping (with no other fancy
> >> logic) in progress.c, I'm not seeing right now how this could happen...
> >
> > This test also fails for me when using QTerminal or Konsole, but it
> > passes on XTerm and LXTerminal.
> 
> I tried this on Debian 11 with QTerminal 0.16.1 and can't reproduce it,
> resized the window etc., always get COLUMNS=80 if I add some printf
> debugging.
> 
> Do you mind testing with an ad-hoc patch like this on top? It will fail
> right away, but should say COLUMNS = 80 in the output.
> 
> The only thing I can think of right now is that some terminals are doing
> some evil trickery to LD_PRELOAD or whatever and intercept getenv() for
> COLUMNS and the like, but that's an entirely unfounded hunch.

And it seems to be a bug in bash:

  export COLUMNS=80

  echo "COLUMNS: $COLUMNS"
  cat > /tmp/expect <<EOF
  foobar
  EOF
  echo "COLUMNS: $COLUMNS"

I get:

  COLUMNS: 80
  COLUMNS: 115

Even directly in the console.

-- 
Felipe Contreras

Felipe Contreras· Jul 19, 2021, 19:34 UTC · re: Felipe Contreras · lore

Re: progress test failure on fedora34

Felipe Contreras wrote:

> Ævar Arnfjörð Bjarmason wrote:
> > 
> > On Wed, Jul 14 2021, Alex Henrie wrote:
> > 
> > > On Wed, Jul 14, 2021 at 9:39 AM Ævar Arnfjörð Bjarmason
> > > <avarab@gmail.com> wrote:
> > >>
> > >>
> > >> On Wed, Jul 14 2021, Fabian Stelzer wrote:
> > >>
> > >> > Hi,
> > >> > The test t0500-progress-display.sh in current master fails on latest
> > >> > fedora34.
> > >> > The break was introduced with:
> > >> >
> > >> > 83ae1edff7ee0b7674bd556955d2cf1706bddb21
> > >> > ab/fix-columns-to-80-during-tests (2021-06-29) 1 commit
> > >> >
> > >> > Kind regards,
> > >> > Fabian
> > >>
> > >> I have not been able to reproduce this, it seems the below E-Mail was
> > >> word-wrapped by your mailer, which is especially bad here since getting
> > >> to the bottom of this requires looking at the whitespace.
> > >>
> > >> Is there a way you could tar that up and send it (to me personally is
> > >> fine, or some pastebin or whatever).
> > >>
> > >> I am able to reproduce something that looks like this if I
> > >> s/COLUMNS=80/COLUMNS=79/g in the test-lib, but given that we set it to
> > >> 80, and that the progress.c code just ends up with an
> > >> atoi(getenv("COLUMNS")), and we do our own wrapping (with no other fancy
> > >> logic) in progress.c, I'm not seeing right now how this could happen...
> > >
> > > This test also fails for me when using QTerminal or Konsole, but it
> > > passes on XTerm and LXTerminal.
> > 
> > I tried this on Debian 11 with QTerminal 0.16.1 and can't reproduce it,
> > resized the window etc., always get COLUMNS=80 if I add some printf
> > debugging.
> > 
> > Do you mind testing with an ad-hoc patch like this on top? It will fail
> > right away, but should say COLUMNS = 80 in the output.
> > 
> > The only thing I can think of right now is that some terminals are doing
> > some evil trickery to LD_PRELOAD or whatever and intercept getenv() for
> > COLUMNS and the like, but that's an entirely unfounded hunch.
> 
> I'm able to reproduce this. The test fails when running directly with
> bash, but not with prove.
> 
> And it seems to be a bug in bash:
> 
>   export COLUMNS=80
> 
>   echo "COLUMNS: $COLUMNS"
>   cat > /tmp/expect <<EOF
>   foobar
>   EOF
>   echo "COLUMNS: $COLUMNS"
> 
> I get:
> 
>   COLUMNS: 80
>   COLUMNS: 115
> 
> Even directly in the console.

Hmm, from man bash:

  checkwinsize
    If  set, bash checks the window size after each external (non‐builtin) com‐
    mand and, if necessary, updates the values of LINES and COLUMNS.  This  op‐
    tion is enabled by default.

Seems like since bash 5.0 this is on by default.

-- 
Felipe Contreras

Alex Henrie· Jul 19, 2021, 20:42 UTC · re: Felipe Contreras · lore

Re: progress test failure on fedora34

>
> Felipe Contreras wrote:
> > Ævar Arnfjörð Bjarmason wrote:
> > >
> > > On Wed, Jul 14 2021, Alex Henrie wrote:
> > >
> > > > On Wed, Jul 14, 2021 at 9:39 AM Ævar Arnfjörð Bjarmason
> > > > <avarab@gmail.com> wrote:
> > > >>
> > > >>
> > > >> On Wed, Jul 14 2021, Fabian Stelzer wrote:
> > > >>
> > > >> > Hi,
> > > >> > The test t0500-progress-display.sh in current master fails on latest
> > > >> > fedora34.
> > > >> > The break was introduced with:
> > > >> >
> > > >> > 83ae1edff7ee0b7674bd556955d2cf1706bddb21
> > > >> > ab/fix-columns-to-80-during-tests (2021-06-29) 1 commit
> > > >> >
> > > >> > Kind regards,
> > > >> > Fabian
> > > >>
> > > >> I have not been able to reproduce this, it seems the below E-Mail was
> > > >> word-wrapped by your mailer, which is especially bad here since getting
> > > >> to the bottom of this requires looking at the whitespace.
> > > >>
> > > >> Is there a way you could tar that up and send it (to me personally is
> > > >> fine, or some pastebin or whatever).
> > > >>
> > > >> I am able to reproduce something that looks like this if I
> > > >> s/COLUMNS=80/COLUMNS=79/g in the test-lib, but given that we set it to
> > > >> 80, and that the progress.c code just ends up with an
> > > >> atoi(getenv("COLUMNS")), and we do our own wrapping (with no other fancy
> > > >> logic) in progress.c, I'm not seeing right now how this could happen...
> > > >
> > > > This test also fails for me when using QTerminal or Konsole, but it
> > > > passes on XTerm and LXTerminal.
> > >
> > > I tried this on Debian 11 with QTerminal 0.16.1 and can't reproduce it,
> > > resized the window etc., always get COLUMNS=80 if I add some printf
> > > debugging.
> > >
> > > Do you mind testing with an ad-hoc patch like this on top? It will fail
> > > right away, but should say COLUMNS = 80 in the output.
> > >
> > > The only thing I can think of right now is that some terminals are doing
> > > some evil trickery to LD_PRELOAD or whatever and intercept getenv() for
> > > COLUMNS and the like, but that's an entirely unfounded hunch.
> >
> > I'm able to reproduce this. The test fails when running directly with
> > bash, but not with prove.
> >
> > And it seems to be a bug in bash:
> >
> >   export COLUMNS=80
> >
> >   echo "COLUMNS: $COLUMNS"
> >   cat > /tmp/expect <<EOF
> >   foobar
> >   EOF
> >   echo "COLUMNS: $COLUMNS"
> >
> > I get:
> >
> >   COLUMNS: 80
> >   COLUMNS: 115
> >
> > Even directly in the console.
>
> Hmm, from man bash:
>
>   checkwinsize
>     If  set, bash checks the window size after each external (non‐builtin) com‐
>     mand and, if necessary, updates the values of LINES and COLUMNS.  This  op‐
>     tion is enabled by default.
>
> Seems like since bash 5.0 this is on by default.

-Alex

Felipe Contreras· Jul 20, 2021, 00:40 UTC · re: Alex Henrie · lore

Re: progress test failure on fedora34

Alex Henrie wrote:

> On Mon, Jul 19, 2021 at 1:34 PM Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
> >
> > Felipe Contreras wrote:
> > > Ævar Arnfjörð Bjarmason wrote:
> > > >
> > > > On Wed, Jul 14 2021, Alex Henrie wrote:
> > > >
> > > > > On Wed, Jul 14, 2021 at 9:39 AM Ævar Arnfjörð Bjarmason
> > > > > <avarab@gmail.com> wrote:
> > > > >>
> > > > >>
> > > > >> On Wed, Jul 14 2021, Fabian Stelzer wrote:
> > > > >>
> > > > >> > Hi,
> > > > >> > The test t0500-progress-display.sh in current master fails on latest
> > > > >> > fedora34.
> > > > >> > The break was introduced with:
> > > > >> >
> > > > >> > 83ae1edff7ee0b7674bd556955d2cf1706bddb21
> > > > >> > ab/fix-columns-to-80-during-tests (2021-06-29) 1 commit
> > > > >> >
> > > > >> > Kind regards,
> > > > >> > Fabian
> > > > >>
> > > > >> I have not been able to reproduce this, it seems the below E-Mail was
> > > > >> word-wrapped by your mailer, which is especially bad here since getting
> > > > >> to the bottom of this requires looking at the whitespace.
> > > > >>
> > > > >> Is there a way you could tar that up and send it (to me personally is
> > > > >> fine, or some pastebin or whatever).
> > > > >>
> > > > >> I am able to reproduce something that looks like this if I
> > > > >> s/COLUMNS=80/COLUMNS=79/g in the test-lib, but given that we set it to
> > > > >> 80, and that the progress.c code just ends up with an
> > > > >> atoi(getenv("COLUMNS")), and we do our own wrapping (with no other fancy
> > > > >> logic) in progress.c, I'm not seeing right now how this could happen...
> > > > >
> > > > > This test also fails for me when using QTerminal or Konsole, but it
> > > > > passes on XTerm and LXTerminal.
> > > >
> > > > I tried this on Debian 11 with QTerminal 0.16.1 and can't reproduce it,
> > > > resized the window etc., always get COLUMNS=80 if I add some printf
> > > > debugging.
> > > >
> > > > Do you mind testing with an ad-hoc patch like this on top? It will fail
> > > > right away, but should say COLUMNS = 80 in the output.
> > > >
> > > > The only thing I can think of right now is that some terminals are doing
> > > > some evil trickery to LD_PRELOAD or whatever and intercept getenv() for
> > > > COLUMNS and the like, but that's an entirely unfounded hunch.
> > >
> > > I'm able to reproduce this. The test fails when running directly with
> > > bash, but not with prove.
> > >
> > > And it seems to be a bug in bash:
> > >
> > >   export COLUMNS=80
> > >
> > >   echo "COLUMNS: $COLUMNS"
> > >   cat > /tmp/expect <<EOF
> > >   foobar
> > >   EOF
> > >   echo "COLUMNS: $COLUMNS"
> > >
> > > I get:
> > >
> > >   COLUMNS: 80
> > >   COLUMNS: 115
> > >
> > > Even directly in the console.
> >
> > Hmm, from man bash:
> >
> >   checkwinsize
> >     If  set, bash checks the window size after each external (non‐builtin) com‐
> >     mand and, if necessary, updates the values of LINES and COLUMNS.  This  op‐
> >     tion is enabled by default.
> >
> > Seems like since bash 5.0 this is on by default.
> 
> Indeed, running `shopt -u checkwinsize` right after exporting COLUMNS
> in test-lib.sh fixes the tests. Great work!

-- 
Felipe Contreras

ZheNing Hu· Jul 21, 2021, 00:45 UTC · re: Felipe Contreras · lore

Re: progress test failure on fedora34

Hi, I met this bug too on my ArchLinux.

Felipe Contreras <felipe.contreras@gmail.com> 于2021年7月20日周二 上午9:04写道：

>
>
> Yeah, this fixes it, but it doesn't seem we are setting any
> bash-specific options right now, and additionally I don't think bash
> should be doing that in the first place. If the shell is
> non-interactive, why is checkwinsize being honored?
>
> Moreover, why does it work with prove? I'm investigating that right now,
> but so far I haven't found any reason.
>

> --
> Felipe Contreras

Felipe Contreras· Jul 21, 2021, 02:50 UTC · re: ZheNing Hu · lore

Re: progress test failure on fedora34

ZheNing Hu wrote:

> Hi, I met this bug too on my ArchLinux.
> 
> Felipe Contreras <felipe.contreras@gmail.com> 于2021年7月20日周二 上午9:04写道：
> > Yeah, this fixes it, but it doesn't seem we are setting any
> > bash-specific options right now, and additionally I don't think bash
> > should be doing that in the first place. If the shell is
> > non-interactive, why is checkwinsize being honored?
> >
> > Moreover, why does it work with prove? I'm investigating that right now,
> > but so far I haven't found any reason.
> 
> I ask this question on IRC #git, and ikke said that after bisecting,
> he thought that
> this bug was introduced in c49a177be.

But I don't think the bug is in git, it's in bash.

[1] https://lore.kernel.org/git/49498ed0-cfd5-2305-cee7-5c5939a19bcf@campoint.net/

-- 
Felipe Contreras

Ævar Arnfjörð Bjarmason· Jul 26, 2021, 23:57 UTC · re: Alex Henrie · lore

[PATCH] test-lib.sh: use GIT_TEST_COLUMNS over COLUMNS

	shopt -u checkwinsize >/dev/null 2>&1

Reported-by: Fabian Stelzer <fabian.stelzer@campoint.net>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---

 pager.c                       |  7 +++++++
 t/t3200-branch.sh             |  8 ++++----
 t/t4052-stat-output.sh        | 22 +++++++++++-----------
 t/t4205-log-pretty-formats.sh |  6 +++---
 t/t7004-tag.sh                |  6 +++---
 t/t7006-pager.sh              |  2 +-
 t/t7508-status.sh             |  4 ++--
 t/t9002-column.sh             | 23 ++++++++++-------------
 t/test-lib.sh                 | 13 +++++++++++--
 9 files changed, 52 insertions(+), 39 deletions(-)

diff --git a/pager.c b/pager.c
index 52f27a6765..cfcc6dc04b 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/t3200-branch.sh b/t/t3200-branch.sh
index cc4b10236e..3e0b71a908 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -328,7 +328,7 @@ test_expect_success 'git branch --list -v with --abbrev' '
 '
 
 test_expect_success 'git branch --column' '
-	COLUMNS=81 git branch --column=column >actual &&
+	GIT_TEST_COLUMNS= COLUMNS=81 git branch --column=column >actual &&
 	cat >expect <<\EOF &&
   a/b/c   bam     foo     l     * main    n       o/p     r
   abc     bar     j/k     m/m     mb      o/o     q       topic
@@ -341,7 +341,7 @@ test_expect_success 'git branch --column with an extremely long branch name' '
 	long=z$long/$long/$long/$long &&
 	test_when_finished "git branch -d $long" &&
 	git branch $long &&
-	COLUMNS=80 git branch --column=column >actual &&
+	GIT_TEST_COLUMNS= COLUMNS=80 git branch --column=column >actual &&
 	cat >expect <<EOF &&
   a/b/c
   abc
@@ -367,7 +367,7 @@ EOF
 test_expect_success 'git branch with column.*' '
 	git config column.ui column &&
 	git config column.branch "dense" &&
-	COLUMNS=80 git branch >actual &&
+	GIT_TEST_COLUMNS= COLUMNS=80 git branch >actual &&
 	git config --unset column.branch &&
 	git config --unset column.ui &&
 	cat >expect <<\EOF &&
@@ -383,7 +383,7 @@ test_expect_success 'git branch --column -v should fail' '
 
 test_expect_success 'git branch -v with column.ui ignored' '
 	git config column.ui column &&
-	COLUMNS=80 git branch -v | cut -c -8 | sed "s/ *$//" >actual &&
+	GIT_TEST_COLUMNS= COLUMNS=80 git branch -v | cut -c -8 | sed "s/ *$//" >actual &&
 	git config --unset column.ui &&
 	cat >expect <<\EOF &&
   a/b/c
diff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh
index 9eba436211..08f8615809 100755
--- a/t/t4052-stat-output.sh
+++ b/t/t4052-stat-output.sh
@@ -111,7 +111,7 @@ cat >expect72 <<'EOF'
  abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 EOF
 test_expect_success "format-patch --cover-letter ignores COLUMNS (big change)" '
-	COLUMNS=200 git format-patch -1 --stdout --cover-letter >output &&
+	GIT_TEST_COLUMNS= COLUMNS=200 git format-patch -1 --stdout --cover-letter >output &&
 	grep " | " output >actual &&
 	test_cmp expect72 actual
 '
@@ -131,7 +131,7 @@ EOF
 while read verb expect cmd args
 do
 	test_expect_success "$cmd $verb COLUMNS (big change)" '
-		COLUMNS=200 git $cmd $args >output &&
+		GIT_TEST_COLUMNS= COLUMNS=200 git $cmd $args >output &&
 		grep " | " output >actual &&
 		test_cmp "$expect" actual
 	'
@@ -139,7 +139,7 @@ do
 	case "$cmd" in diff|show) continue;; esac
 
 	test_expect_success "$cmd --graph $verb COLUMNS (big change)" '
-		COLUMNS=200 git $cmd $args --graph >output &&
+		GIT_TEST_COLUMNS= COLUMNS=200 git $cmd $args --graph >output &&
 		grep " | " output >actual &&
 		test_cmp "$expect-graph" actual
 	'
@@ -159,7 +159,7 @@ EOF
 while read verb expect cmd args
 do
 	test_expect_success "$cmd $verb not enough COLUMNS (big change)" '
-		COLUMNS=40 git $cmd $args >output &&
+		GIT_TEST_COLUMNS= COLUMNS=40 git $cmd $args >output &&
 		grep " | " output >actual &&
 		test_cmp "$expect" actual
 	'
@@ -167,7 +167,7 @@ do
 	case "$cmd" in diff|show) continue;; esac
 
 	test_expect_success "$cmd --graph $verb not enough COLUMNS (big change)" '
-		COLUMNS=40 git $cmd $args --graph >output &&
+		GIT_TEST_COLUMNS= COLUMNS=40 git $cmd $args --graph >output &&
 		grep " | " output >actual &&
 		test_cmp "$expect-graph" actual
 	'
@@ -302,7 +302,7 @@ EOF
 while read verb expect cmd args
 do
 	test_expect_success "$cmd $verb COLUMNS (long filename)" '
-		COLUMNS=200 git $cmd $args >output &&
+		GIT_TEST_COLUMNS= COLUMNS=200 git $cmd $args >output &&
 		grep " | " output >actual &&
 		test_cmp "$expect" actual
 	'
@@ -310,7 +310,7 @@ do
 	case "$cmd" in diff|show) continue;; esac
 
 	test_expect_success "$cmd --graph $verb COLUMNS (long filename)" '
-		COLUMNS=200 git $cmd $args --graph >output &&
+		GIT_TEST_COLUMNS= COLUMNS=200 git $cmd $args --graph >output &&
 		grep " | " output >actual &&
 		test_cmp "$expect-graph" actual
 	'
@@ -331,7 +331,7 @@ while read verb expect cmd args
 do
 	test_expect_success COLUMNS_CAN_BE_1 \
 		"$cmd $verb prefix greater than COLUMNS (big change)" '
-		COLUMNS=1 git $cmd $args >output &&
+		GIT_TEST_COLUMNS= COLUMNS=1 git $cmd $args >output &&
 		grep " | " output >actual &&
 		test_cmp "$expect" actual
 	'
@@ -340,7 +340,7 @@ do
 
 	test_expect_success COLUMNS_CAN_BE_1 \
 		"$cmd --graph $verb prefix greater than COLUMNS (big change)" '
-		COLUMNS=1 git $cmd $args --graph >output &&
+		GIT_TEST_COLUMNS= COLUMNS=1 git $cmd $args --graph >output &&
 		grep " | " output >actual &&
 		test_cmp "$expect-graph" actual
 	'
@@ -356,7 +356,7 @@ cat >expect <<'EOF'
 EOF
 test_expect_success 'merge --stat respects COLUMNS (big change)' '
 	git checkout -b branch HEAD^^ &&
-	COLUMNS=100 git merge --stat --no-ff main^ >output &&
+	GIT_TEST_COLUMNS= COLUMNS=100 git merge --stat --no-ff main^ >output &&
 	grep " | " output >actual &&
 	test_cmp expect actual
 '
@@ -365,7 +365,7 @@ cat >expect <<'EOF'
  aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 +++++++++++++++++++++++++++++++++++++++
 EOF
 test_expect_success 'merge --stat respects COLUMNS (long filename)' '
-	COLUMNS=100 git merge --stat --no-ff main >output &&
+	GIT_TEST_COLUMNS= COLUMNS=100 git merge --stat --no-ff main >output &&
 	grep " | " output >actual &&
 	test_cmp expect actual
 '
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 5865daa8f8..6c8e1b3f1a 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -207,7 +207,7 @@ test_expect_success 'left alignment formatting at the nth column' '
 '
 
 test_expect_success 'left alignment formatting at the nth column' '
-	COLUMNS=50 git log --pretty="tformat:%h %<|(-10)%s" >actual &&
+	GIT_TEST_COLUMNS= COLUMNS=50 git log --pretty="tformat:%h %<|(-10)%s" >actual &&
 	qz_to_tab_space <<-EOF >expected &&
 	$head1 message two                    Z
 	$head2 message one                    Z
@@ -350,7 +350,7 @@ test_expect_success 'right alignment formatting at the nth column' '
 '
 
 test_expect_success 'right alignment formatting at the nth column' '
-	COLUMNS=50 git log --pretty="tformat:%h %>|(-10)%s" >actual &&
+	GIT_TEST_COLUMNS= COLUMNS=50 git log --pretty="tformat:%h %>|(-10)%s" >actual &&
 	qz_to_tab_space <<-EOF >expected &&
 	$head1                      message two
 	$head2                      message one
@@ -450,7 +450,7 @@ test_expect_success 'center alignment formatting at the nth column' '
 '
 
 test_expect_success 'center alignment formatting at the nth column' '
-	COLUMNS=70 git log --pretty="tformat:%h %><|(-30)%s" >actual &&
+	GIT_TEST_COLUMNS= COLUMNS=70 git log --pretty="tformat:%h %><|(-30)%s" >actual &&
 	qz_to_tab_space <<-EOF >expected &&
 	$head1           message two          Z
 	$head2           message one          Z
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 2f72c5c688..1c3d8cfd16 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -372,7 +372,7 @@ test_expect_success 'tag -l <pattern> -l <pattern> works, as our buggy documenta
 '
 
 test_expect_success 'listing tags in column' '
-	COLUMNS=41 git tag -l --column=row >actual &&
+	GIT_TEST_COLUMNS= COLUMNS=41 git tag -l --column=row >actual &&
 	cat >expected <<\EOF &&
 a1      aa1     cba     t210    t211
 v0.2.1  v1.0    v1.0.1  v1.1.3
@@ -383,7 +383,7 @@ EOF
 test_expect_success 'listing tags in column with column.*' '
 	test_config column.tag row &&
 	test_config column.ui dense &&
-	COLUMNS=40 git tag -l >actual &&
+	GIT_TEST_COLUMNS= COLUMNS=40 git tag -l >actual &&
 	cat >expected <<\EOF &&
 a1      aa1   cba     t210    t211
 v0.2.1  v1.0  v1.0.1  v1.1.3
@@ -397,7 +397,7 @@ test_expect_success 'listing tag with -n --column should fail' '
 
 test_expect_success 'listing tags -n in column with column.ui ignored' '
 	test_config column.ui "row dense" &&
-	COLUMNS=40 git tag -l -n >actual &&
+	GIT_TEST_COLUMNS= COLUMNS=40 git tag -l -n >actual &&
 	cat >expected <<\EOF &&
 a1              Foo
 aa1             Foo
diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index 0e7cf75435..1b116366a3 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -651,7 +651,7 @@ test_expect_success TTY 'git tag with auto-columns ' '
 	cat >expect <<-\EOF &&
 	initial  one      two      three    four     five
 	EOF
-	test_terminal env PAGER="cat >actual" COLUMNS=80 \
+	test_terminal env PAGER="cat >actual" GIT_TEST_COLUMNS= COLUMNS=80 \
 		git -c column.ui=auto tag --sort=authordate &&
 	test_cmp expect actual
 '
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index 2b72451ba3..669a3c7150 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -108,13 +108,13 @@ test_expect_success 'status --column' '
 #	dir2/modified  untracked
 #
 EOF
-	COLUMNS=50 git -c status.displayCommentPrefix=true status --column="column dense" >output &&
+	GIT_TEST_COLUMNS= COLUMNS=50 git -c status.displayCommentPrefix=true status --column="column dense" >output &&
 	test_cmp expect output
 '
 
 test_expect_success 'status --column status.displayCommentPrefix=false' '
 	strip_comments expect &&
-	COLUMNS=49 git -c status.displayCommentPrefix=false status --column="column dense" >output &&
+	GIT_TEST_COLUMNS= COLUMNS=49 git -c status.displayCommentPrefix=false status --column="column dense" >output &&
 	test_cmp expect output
 '
 
diff --git a/t/t9002-column.sh b/t/t9002-column.sh
index 89983527b6..50cf3e7b42 100755
--- a/t/t9002-column.sh
+++ b/t/t9002-column.sh
@@ -46,7 +46,7 @@ test_expect_success '80 columns' '
 	cat >expected <<\EOF &&
 one    two    three  four   five   six    seven  eight  nine   ten    eleven
 EOF
-	COLUMNS=80 git column --mode=column <lista >actual &&
+	GIT_TEST_COLUMNS= COLUMNS=80 git column --mode=column <lista >actual &&
 	test_cmp expected actual
 '
 
@@ -65,7 +65,7 @@ eleven
 EOF
 
 test_expect_success COLUMNS_CAN_BE_1 'COLUMNS = 1' '
-	COLUMNS=1 git column --mode=column <lista >actual &&
+	GIT_TEST_COLUMNS= COLUMNS=1 git column --mode=column <lista >actual &&
 	test_cmp expected actual
 '
 
@@ -74,9 +74,6 @@ test_expect_success 'width = 1' '
 	test_cmp expected actual
 '
 
-COLUMNS=20
-export COLUMNS
-
 test_expect_success '20 columns' '
 	cat >expected <<\EOF &&
 one    seven
@@ -86,7 +83,7 @@ four   ten
 five   eleven
 six
 EOF
-	git column --mode=column <lista >actual &&
+	GIT_TEST_COLUMNS= COLUMNS=20 git column --mode=column <lista >actual &&
 	test_cmp expected actual
 '
 
@@ -99,7 +96,7 @@ four   ten
 five   eleven
 six
 EOF
-	git column --mode=column,nodense < lista > actual &&
+	GIT_TEST_COLUMNS= COLUMNS=20 git column --mode=column,nodense < lista > actual &&
 	test_cmp expected actual
 '
 
@@ -110,7 +107,7 @@ two   six   ten
 three seven eleven
 four  eight
 EOF
-	git column --mode=column,dense < lista > actual &&
+	GIT_TEST_COLUMNS= COLUMNS=20 git column --mode=column,dense < lista > actual &&
 	test_cmp expected actual
 '
 
@@ -123,7 +120,7 @@ four    ten
 five    eleven
 six
 EOF
-	git column --mode=column --padding 2 <lista >actual &&
+	GIT_TEST_COLUMNS= COLUMNS=20 git column --mode=column --padding 2 <lista >actual &&
 	test_cmp expected actual
 '
 
@@ -136,7 +133,7 @@ test_expect_success '20 columns, indented' '
   five   eleven
   six
 EOF
-	git column --mode=column --indent="  " <lista >actual &&
+	GIT_TEST_COLUMNS= COLUMNS=20 git column --mode=column --indent="  " <lista >actual &&
 	test_cmp expected actual
 '
 
@@ -149,7 +146,7 @@ seven  eight
 nine   ten
 eleven
 EOF
-	git column --mode=row <lista >actual &&
+	GIT_TEST_COLUMNS= COLUMNS=20 git column --mode=row <lista >actual &&
 	test_cmp expected actual
 '
 
@@ -162,7 +159,7 @@ seven  eight
 nine   ten
 eleven
 EOF
-	git column --mode=row,nodense <lista >actual &&
+	GIT_TEST_COLUMNS= COLUMNS=20 git column --mode=row,nodense <lista >actual &&
 	test_cmp expected actual
 '
 
@@ -173,7 +170,7 @@ four  five   six
 seven eight  nine
 ten   eleven
 EOF
-	git column --mode=row,dense <lista >actual &&
+	GIT_TEST_COLUMNS= COLUMNS=20 git column --mode=row,dense <lista >actual &&
 	test_cmp expected actual
 '
 
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 9e26860544..82771643ba 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -406,10 +406,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.32.0.988.g1a6a4b2c5f

Jeff King· Jul 27, 2021, 17:38 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH] test-lib.sh: use GIT_TEST_COLUMNS over COLUMNS

On Tue, Jul 27, 2021 at 01:57:09AM +0200, Ævar Arnfjörð Bjarmason wrote:

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

> 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. I'm doing the
> latter in all the tests changed here.

-Peff

Junio C Hamano· Jul 28, 2021, 00:53 UTC · re: Jeff King · lore

Re: [PATCH] test-lib.sh: use GIT_TEST_COLUMNS over COLUMNS

Jeff King <peff@peff.net> writes:

>> 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.
>
> I guess people running with bash won't see the test breakage (because
> bash will quietly "fix" the COLUMNS setting). But enough people run with
> /bin/sh that we'll eventually notice.
>
>> 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. I'm doing the
>> latter in all the tests changed here.
>
> This is rather ugly, and I'm not in general a fan of adding more
> test-only code into the bowels of Git itself. But it may be the least
> bad solution.

Perhaps with something like

	test_with_columns () {
        	local columns=$1
		shift
		GIT_TEST_COLUMNS= COLUMNS=$columns "$@"
	}

we may be able to hide the ugly implementation detail like this:

	test_with_columns 81 git branch --column=column >actual

    s/^(\s*)COLUMNS=(\d+)/$1test_with_columns $2/;

plus addition of the helper to test-lib, perhaps like so:

	test_with_columns () {
        	local columns=$1
		shift
		COLUMNS=$columns "$@"
	}

and the whole GIT_TEST_COLUMNS stuff being the second step.

Ævar Arnfjörð Bjarmason· Aug 2, 2021, 13:46 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH v2 0/3] test-lib.sh: use GIT_TEST_COLUMNS over COLUMNS

Ævar Arnfjörð Bjarmason (3):
  test-lib-functions.sh: rename test_must_fail_acceptable()
  test-lib-functions.sh: add a test_with_columns function
  test-lib.sh: use GIT_TEST_COLUMNS over COLUMNS

 pager.c                       |  7 +++++++
 t/t3200-branch.sh             |  8 ++++----
 t/t4052-stat-output.sh        | 22 +++++++++++-----------
 t/t4205-log-pretty-formats.sh |  6 +++---
 t/t7004-tag.sh                |  6 +++---
 t/t7006-pager.sh              |  2 +-
 t/t7508-status.sh             |  4 ++--
 t/t9002-column.sh             | 23 ++++++++++-------------
 t/test-lib-functions.sh       | 21 ++++++++++++++++++---
 t/test-lib.sh                 | 13 +++++++++++--
 10 files changed, 70 insertions(+), 42 deletions(-)

Range-diff against v1:
-:  ----------- > 1:  739457b992f test-lib-functions.sh: rename test_must_fail_acceptable()
1:  f81f3911d52 ! 2:  36c57178c55 test-lib.sh: use GIT_TEST_COLUMNS over COLUMNS
    @@ Metadata
     Author: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## Commit message ##
    -    test-lib.sh: use GIT_TEST_COLUMNS over COLUMNS
    +    test-lib-functions.sh: add a test_with_columns function
     
    -    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.
    +    Add a helper function to wrap patterns of "COLUMNS=N <command>" as
    +    "test_with_columns N <command>". This will be used and extended in the
    +    next commit.
     
    -    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. I'm doing the
    -    latter in all the tests changed here.
    -
    -    Reported-by: Fabian Stelzer <fabian.stelzer@campoint.net>
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
    - ## pager.c ##
    -@@ pager.c: 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;
    -
      ## t/t3200-branch.sh ##
     @@ t/t3200-branch.sh: test_expect_success 'git branch --list -v with --abbrev' '
      '
      
      test_expect_success 'git branch --column' '
     -	COLUMNS=81 git branch --column=column >actual &&
    -+	GIT_TEST_COLUMNS= COLUMNS=81 git branch --column=column >actual &&
    ++	test_with_columns 81 git branch --column=column >actual &&
      	cat >expect <<\EOF &&
        a/b/c   bam     foo     l     * main    n       o/p     r
        abc     bar     j/k     m/m     mb      o/o     q       topic
    @@ t/t3200-branch.sh: test_expect_success 'git branch --column with an extremely lo
      	test_when_finished "git branch -d $long" &&
      	git branch $long &&
     -	COLUMNS=80 git branch --column=column >actual &&
    -+	GIT_TEST_COLUMNS= COLUMNS=80 git branch --column=column >actual &&
    ++	test_with_columns 80 git branch --column=column >actual &&
      	cat >expect <<EOF &&
        a/b/c
        abc
    @@ t/t3200-branch.sh: EOF
      	git config column.ui column &&
      	git config column.branch "dense" &&
     -	COLUMNS=80 git branch >actual &&
    -+	GIT_TEST_COLUMNS= COLUMNS=80 git branch >actual &&
    ++	test_with_columns 80 git branch >actual &&
      	git config --unset column.branch &&
      	git config --unset column.ui &&
      	cat >expect <<\EOF &&
    @@ t/t3200-branch.sh: test_expect_success 'git branch --column -v should fail' '
      test_expect_success 'git branch -v with column.ui ignored' '
      	git config column.ui column &&
     -	COLUMNS=80 git branch -v | cut -c -8 | sed "s/ *$//" >actual &&
    -+	GIT_TEST_COLUMNS= COLUMNS=80 git branch -v | cut -c -8 | sed "s/ *$//" >actual &&
    ++	test_with_columns 80 git branch -v | cut -c -8 | sed "s/ *$//" >actual &&
      	git config --unset column.ui &&
      	cat >expect <<\EOF &&
        a/b/c
    @@ t/t4052-stat-output.sh: cat >expect72 <<'EOF'
      EOF
      test_expect_success "format-patch --cover-letter ignores COLUMNS (big change)" '
     -	COLUMNS=200 git format-patch -1 --stdout --cover-letter >output &&
    -+	GIT_TEST_COLUMNS= COLUMNS=200 git format-patch -1 --stdout --cover-letter >output &&
    ++	test_with_columns 200 git format-patch -1 --stdout --cover-letter >output &&
      	grep " | " output >actual &&
      	test_cmp expect72 actual
      '
    @@ t/t4052-stat-output.sh: EOF
      do
      	test_expect_success "$cmd $verb COLUMNS (big change)" '
     -		COLUMNS=200 git $cmd $args >output &&
    -+		GIT_TEST_COLUMNS= COLUMNS=200 git $cmd $args >output &&
    ++		test_with_columns 200 git $cmd $args >output &&
      		grep " | " output >actual &&
      		test_cmp "$expect" actual
      	'
    @@ t/t4052-stat-output.sh: do
      
      	test_expect_success "$cmd --graph $verb COLUMNS (big change)" '
     -		COLUMNS=200 git $cmd $args --graph >output &&
    -+		GIT_TEST_COLUMNS= COLUMNS=200 git $cmd $args --graph >output &&
    ++		test_with_columns 200 git $cmd $args --graph >output &&
      		grep " | " output >actual &&
      		test_cmp "$expect-graph" actual
      	'
    @@ t/t4052-stat-output.sh: EOF
      do
      	test_expect_success "$cmd $verb not enough COLUMNS (big change)" '
     -		COLUMNS=40 git $cmd $args >output &&
    -+		GIT_TEST_COLUMNS= COLUMNS=40 git $cmd $args >output &&
    ++		test_with_columns 40 git $cmd $args >output &&
      		grep " | " output >actual &&
      		test_cmp "$expect" actual
      	'
    @@ t/t4052-stat-output.sh: do
      
      	test_expect_success "$cmd --graph $verb not enough COLUMNS (big change)" '
     -		COLUMNS=40 git $cmd $args --graph >output &&
    -+		GIT_TEST_COLUMNS= COLUMNS=40 git $cmd $args --graph >output &&
    ++		test_with_columns 40 git $cmd $args --graph >output &&
      		grep " | " output >actual &&
      		test_cmp "$expect-graph" actual
      	'
    @@ t/t4052-stat-output.sh: EOF
      do
      	test_expect_success "$cmd $verb COLUMNS (long filename)" '
     -		COLUMNS=200 git $cmd $args >output &&
    -+		GIT_TEST_COLUMNS= COLUMNS=200 git $cmd $args >output &&
    ++		test_with_columns 200 git $cmd $args >output &&
      		grep " | " output >actual &&
      		test_cmp "$expect" actual
      	'
    @@ t/t4052-stat-output.sh: do
      
      	test_expect_success "$cmd --graph $verb COLUMNS (long filename)" '
     -		COLUMNS=200 git $cmd $args --graph >output &&
    -+		GIT_TEST_COLUMNS= COLUMNS=200 git $cmd $args --graph >output &&
    ++		test_with_columns 200 git $cmd $args --graph >output &&
      		grep " | " output >actual &&
      		test_cmp "$expect-graph" actual
      	'
    @@ t/t4052-stat-output.sh: while read verb expect cmd args
      	test_expect_success COLUMNS_CAN_BE_1 \
      		"$cmd $verb prefix greater than COLUMNS (big change)" '
     -		COLUMNS=1 git $cmd $args >output &&
    -+		GIT_TEST_COLUMNS= COLUMNS=1 git $cmd $args >output &&
    ++		test_with_columns 1 git $cmd $args >output &&
      		grep " | " output >actual &&
      		test_cmp "$expect" actual
      	'
    @@ t/t4052-stat-output.sh: do
      	test_expect_success COLUMNS_CAN_BE_1 \
      		"$cmd --graph $verb prefix greater than COLUMNS (big change)" '
     -		COLUMNS=1 git $cmd $args --graph >output &&
    -+		GIT_TEST_COLUMNS= COLUMNS=1 git $cmd $args --graph >output &&
    ++		test_with_columns 1 git $cmd $args --graph >output &&
      		grep " | " output >actual &&
      		test_cmp "$expect-graph" actual
      	'
    @@ t/t4052-stat-output.sh: cat >expect <<'EOF'
      test_expect_success 'merge --stat respects COLUMNS (big change)' '
      	git checkout -b branch HEAD^^ &&
     -	COLUMNS=100 git merge --stat --no-ff main^ >output &&
    -+	GIT_TEST_COLUMNS= COLUMNS=100 git merge --stat --no-ff main^ >output &&
    ++	test_with_columns 100 git merge --stat --no-ff main^ >output &&
      	grep " | " output >actual &&
      	test_cmp expect actual
      '
    @@ t/t4052-stat-output.sh: cat >expect <<'EOF'
      EOF
      test_expect_success 'merge --stat respects COLUMNS (long filename)' '
     -	COLUMNS=100 git merge --stat --no-ff main >output &&
    -+	GIT_TEST_COLUMNS= COLUMNS=100 git merge --stat --no-ff main >output &&
    ++	test_with_columns 100 git merge --stat --no-ff main >output &&
      	grep " | " output >actual &&
      	test_cmp expect actual
      '
    @@ t/t4205-log-pretty-formats.sh: test_expect_success 'left alignment formatting at
      
      test_expect_success 'left alignment formatting at the nth column' '
     -	COLUMNS=50 git log --pretty="tformat:%h %<|(-10)%s" >actual &&
    -+	GIT_TEST_COLUMNS= COLUMNS=50 git log --pretty="tformat:%h %<|(-10)%s" >actual &&
    ++	test_with_columns 50 git log --pretty="tformat:%h %<|(-10)%s" >actual &&
      	qz_to_tab_space <<-EOF >expected &&
      	$head1 message two                    Z
      	$head2 message one                    Z
    @@ t/t4205-log-pretty-formats.sh: test_expect_success 'right alignment formatting a
      
      test_expect_success 'right alignment formatting at the nth column' '
     -	COLUMNS=50 git log --pretty="tformat:%h %>|(-10)%s" >actual &&
    -+	GIT_TEST_COLUMNS= COLUMNS=50 git log --pretty="tformat:%h %>|(-10)%s" >actual &&
    ++	test_with_columns 50 git log --pretty="tformat:%h %>|(-10)%s" >actual &&
      	qz_to_tab_space <<-EOF >expected &&
      	$head1                      message two
      	$head2                      message one
    @@ t/t4205-log-pretty-formats.sh: test_expect_success 'center alignment formatting
      
      test_expect_success 'center alignment formatting at the nth column' '
     -	COLUMNS=70 git log --pretty="tformat:%h %><|(-30)%s" >actual &&
    -+	GIT_TEST_COLUMNS= COLUMNS=70 git log --pretty="tformat:%h %><|(-30)%s" >actual &&
    ++	test_with_columns 70 git log --pretty="tformat:%h %><|(-30)%s" >actual &&
      	qz_to_tab_space <<-EOF >expected &&
      	$head1           message two          Z
      	$head2           message one          Z
    @@ t/t7004-tag.sh: test_expect_success 'tag -l <pattern> -l <pattern> works, as our
      
      test_expect_success 'listing tags in column' '
     -	COLUMNS=41 git tag -l --column=row >actual &&
    -+	GIT_TEST_COLUMNS= COLUMNS=41 git tag -l --column=row >actual &&
    ++	test_with_columns 41 git tag -l --column=row >actual &&
      	cat >expected <<\EOF &&
      a1      aa1     cba     t210    t211
      v0.2.1  v1.0    v1.0.1  v1.1.3
    @@ t/t7004-tag.sh: EOF
      	test_config column.tag row &&
      	test_config column.ui dense &&
     -	COLUMNS=40 git tag -l >actual &&
    -+	GIT_TEST_COLUMNS= COLUMNS=40 git tag -l >actual &&
    ++	test_with_columns 40 git tag -l >actual &&
      	cat >expected <<\EOF &&
      a1      aa1   cba     t210    t211
      v0.2.1  v1.0  v1.0.1  v1.1.3
    @@ t/t7004-tag.sh: test_expect_success 'listing tag with -n --column should fail' '
      test_expect_success 'listing tags -n in column with column.ui ignored' '
      	test_config column.ui "row dense" &&
     -	COLUMNS=40 git tag -l -n >actual &&
    -+	GIT_TEST_COLUMNS= COLUMNS=40 git tag -l -n >actual &&
    ++	test_with_columns 40 git tag -l -n >actual &&
      	cat >expected <<\EOF &&
      a1              Foo
      aa1             Foo
    @@ t/t7006-pager.sh: test_expect_success TTY 'git tag with auto-columns ' '
      	initial  one      two      three    four     five
      	EOF
     -	test_terminal env PAGER="cat >actual" COLUMNS=80 \
    -+	test_terminal env PAGER="cat >actual" GIT_TEST_COLUMNS= COLUMNS=80 \
    ++	test_with_columns 80 test_terminal env PAGER="cat >actual" \
      		git -c column.ui=auto tag --sort=authordate &&
      	test_cmp expect actual
      '
    @@ t/t7508-status.sh: test_expect_success 'status --column' '
      #
      EOF
     -	COLUMNS=50 git -c status.displayCommentPrefix=true status --column="column dense" >output &&
    -+	GIT_TEST_COLUMNS= COLUMNS=50 git -c status.displayCommentPrefix=true status --column="column dense" >output &&
    ++	test_with_columns 50 git -c status.displayCommentPrefix=true status --column="column dense" >output &&
      	test_cmp expect output
      '
      
      test_expect_success 'status --column status.displayCommentPrefix=false' '
      	strip_comments expect &&
     -	COLUMNS=49 git -c status.displayCommentPrefix=false status --column="column dense" >output &&
    -+	GIT_TEST_COLUMNS= COLUMNS=49 git -c status.displayCommentPrefix=false status --column="column dense" >output &&
    ++	test_with_columns 49 git -c status.displayCommentPrefix=false status --column="column dense" >output &&
      	test_cmp expect output
      '
      
    @@ t/t9002-column.sh: test_expect_success '80 columns' '
      one    two    three  four   five   six    seven  eight  nine   ten    eleven
      EOF
     -	COLUMNS=80 git column --mode=column <lista >actual &&
    -+	GIT_TEST_COLUMNS= COLUMNS=80 git column --mode=column <lista >actual &&
    ++	test_with_columns 80 git column --mode=column <lista >actual &&
      	test_cmp expected actual
      '
      
    @@ t/t9002-column.sh: eleven
      
      test_expect_success COLUMNS_CAN_BE_1 'COLUMNS = 1' '
     -	COLUMNS=1 git column --mode=column <lista >actual &&
    -+	GIT_TEST_COLUMNS= COLUMNS=1 git column --mode=column <lista >actual &&
    ++	test_with_columns 1 git column --mode=column <lista >actual &&
      	test_cmp expected actual
      '
      
    @@ t/t9002-column.sh: four   ten
      six
      EOF
     -	git column --mode=column <lista >actual &&
    -+	GIT_TEST_COLUMNS= COLUMNS=20 git column --mode=column <lista >actual &&
    ++	test_with_columns 20 git column --mode=column <lista >actual &&
      	test_cmp expected actual
      '
      
    @@ t/t9002-column.sh: four   ten
      six
      EOF
     -	git column --mode=column,nodense < lista > actual &&
    -+	GIT_TEST_COLUMNS= COLUMNS=20 git column --mode=column,nodense < lista > actual &&
    ++	test_with_columns 20 git column --mode=column,nodense < lista > actual &&
      	test_cmp expected actual
      '
      
    @@ t/t9002-column.sh: two   six   ten
      four  eight
      EOF
     -	git column --mode=column,dense < lista > actual &&
    -+	GIT_TEST_COLUMNS= COLUMNS=20 git column --mode=column,dense < lista > actual &&
    ++	test_with_columns 20 git column --mode=column,dense < lista > actual &&
      	test_cmp expected actual
      '
      
    @@ t/t9002-column.sh: four    ten
      six
      EOF
     -	git column --mode=column --padding 2 <lista >actual &&
    -+	GIT_TEST_COLUMNS= COLUMNS=20 git column --mode=column --padding 2 <lista >actual &&
    ++	test_with_columns 20 git column --mode=column --padding 2 <lista >actual &&
      	test_cmp expected actual
      '
      
    @@ t/t9002-column.sh: test_expect_success '20 columns, indented' '
        six
      EOF
     -	git column --mode=column --indent="  " <lista >actual &&
    -+	GIT_TEST_COLUMNS= COLUMNS=20 git column --mode=column --indent="  " <lista >actual &&
    ++	test_with_columns 20 git column --mode=column --indent="  " <lista >actual &&
      	test_cmp expected actual
      '
      
    @@ t/t9002-column.sh: seven  eight
      eleven
      EOF
     -	git column --mode=row <lista >actual &&
    -+	GIT_TEST_COLUMNS= COLUMNS=20 git column --mode=row <lista >actual &&
    ++	test_with_columns 20 git column --mode=row <lista >actual &&
      	test_cmp expected actual
      '
      
    @@ t/t9002-column.sh: seven  eight
      eleven
      EOF
     -	git column --mode=row,nodense <lista >actual &&
    -+	GIT_TEST_COLUMNS= COLUMNS=20 git column --mode=row,nodense <lista >actual &&
    ++	test_with_columns 20 git column --mode=row,nodense <lista >actual &&
      	test_cmp expected actual
      '
      
    @@ t/t9002-column.sh: four  five   six
      ten   eleven
      EOF
     -	git column --mode=row,dense <lista >actual &&
    -+	GIT_TEST_COLUMNS= COLUMNS=20 git column --mode=row,dense <lista >actual &&
    ++	test_with_columns 20 git column --mode=row,dense <lista >actual &&
      	test_cmp expected actual
      '
      
     
    - ## t/test-lib.sh ##
    -@@ t/test-lib.sh: 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
    + ## t/test-lib-functions.sh ##
    +@@ t/test-lib-functions.sh: test_region () {
    + test_readlink () {
    + 	perl -le 'print readlink($_) for @ARGV' "$@"
    + }
    ++
    ++# Test a with a given number of COLUMNS in the environment.
    ++test_with_columns () {
    ++	local columns=$1
    ++	shift
     +
    - # 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
    ++	COLUMNS=$columns "$@"
    ++}
-:  ----------- > 3:  6cbbb955e9a test-lib.sh: use GIT_TEST_COLUMNS over COLUMNS

-- 
2.32.0.1070.gec115ccd780

Ævar Arnfjörð Bjarmason· Aug 2, 2021, 13:46 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH v2 1/3] test-lib-functions.sh: rename test_must_fail_acceptable()

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/test-lib-functions.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index e28411bb75a..37da7d9a99a 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -895,7 +895,7 @@ list_contains () {
 # accepted by test_must_fail(). If the command is run with env, the env
 # and its corresponding variable settings will be stripped before we
 # test the command being run.
-test_must_fail_acceptable () {
+is_git_command_name () {
 	if test "$1" = "env"
 	then
 		shift
@@ -943,7 +943,7 @@ test_must_fail_acceptable () {
 #     (Don't use 'success', use 'test_might_fail' instead.)
 #
 # Do not use this to run anything but "git" and other specific testable
-# commands (see test_must_fail_acceptable()).  We are not in the
+# commands (see is_git_command_name()).  We are not in the
 # business of vetting system supplied commands -- in other words, this
 # is wrong:
 #
@@ -963,7 +963,7 @@ test_must_fail () {
 		_test_ok=
 		;;
 	esac
-	if ! test_must_fail_acceptable "$@"
+	if ! is_git_command_name "$@"
 	then
 		echo >&7 "test_must_fail: only 'git' is allowed: $*"
 		return 1

-- 
2.32.0.1070.gec115ccd780

Ævar Arnfjörð Bjarmason· Aug 2, 2021, 13:46 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH v2 3/3] test-lib.sh: use GIT_TEST_COLUMNS over COLUMNS

	shopt -u checkwinsize >/dev/null 2>&1

Reported-by: Fabian Stelzer <fabian.stelzer@campoint.net>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 pager.c                 |  7 +++++++
 t/test-lib-functions.sh |  7 +++++++
 t/test-lib.sh           | 13 +++++++++++--
 3 files changed, 25 insertions(+), 2 deletions(-)

diff --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 536339faaa2..959354e0c6e 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -1724,5 +1724,12 @@ test_with_columns () {
 	local columns=$1
 	shift
 
+	if ! is_git_command_name "$@"
+	then
+		echo >&7 "test_with_columns: only 'git' is allowed: $*"
+		return 1
+	fi
+
+	GIT_TEST_COLUMNS= \
 	COLUMNS=$columns "$@"
 }
diff --git a/t/test-lib.sh b/t/test-lib.sh
index da13190970f..4bb231e5729 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.32.0.1070.gec115ccd780

Ævar Arnfjörð Bjarmason· Aug 2, 2021, 13:46 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH v2 2/3] test-lib-functions.sh: add a test_with_columns function

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t3200-branch.sh             |  8 ++++----
 t/t4052-stat-output.sh        | 22 +++++++++++-----------
 t/t4205-log-pretty-formats.sh |  6 +++---
 t/t7004-tag.sh                |  6 +++---
 t/t7006-pager.sh              |  2 +-
 t/t7508-status.sh             |  4 ++--
 t/t9002-column.sh             | 23 ++++++++++-------------
 t/test-lib-functions.sh       |  8 ++++++++
 8 files changed, 42 insertions(+), 37 deletions(-)

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index cc4b10236e2..e568156ca83 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -328,7 +328,7 @@ test_expect_success 'git branch --list -v with --abbrev' '
 '
 
 test_expect_success 'git branch --column' '
-	COLUMNS=81 git branch --column=column >actual &&
+	test_with_columns 81 git branch --column=column >actual &&
 	cat >expect <<\EOF &&
   a/b/c   bam     foo     l     * main    n       o/p     r
   abc     bar     j/k     m/m     mb      o/o     q       topic
@@ -341,7 +341,7 @@ test_expect_success 'git branch --column with an extremely long branch name' '
 	long=z$long/$long/$long/$long &&
 	test_when_finished "git branch -d $long" &&
 	git branch $long &&
-	COLUMNS=80 git branch --column=column >actual &&
+	test_with_columns 80 git branch --column=column >actual &&
 	cat >expect <<EOF &&
   a/b/c
   abc
@@ -367,7 +367,7 @@ EOF
 test_expect_success 'git branch with column.*' '
 	git config column.ui column &&
 	git config column.branch "dense" &&
-	COLUMNS=80 git branch >actual &&
+	test_with_columns 80 git branch >actual &&
 	git config --unset column.branch &&
 	git config --unset column.ui &&
 	cat >expect <<\EOF &&
@@ -383,7 +383,7 @@ test_expect_success 'git branch --column -v should fail' '
 
 test_expect_success 'git branch -v with column.ui ignored' '
 	git config column.ui column &&
-	COLUMNS=80 git branch -v | cut -c -8 | sed "s/ *$//" >actual &&
+	test_with_columns 80 git branch -v | cut -c -8 | sed "s/ *$//" >actual &&
 	git config --unset column.ui &&
 	cat >expect <<\EOF &&
   a/b/c
diff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh
index 9eba436211f..3a91df50dd8 100755
--- a/t/t4052-stat-output.sh
+++ b/t/t4052-stat-output.sh
@@ -111,7 +111,7 @@ cat >expect72 <<'EOF'
  abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 EOF
 test_expect_success "format-patch --cover-letter ignores COLUMNS (big change)" '
-	COLUMNS=200 git format-patch -1 --stdout --cover-letter >output &&
+	test_with_columns 200 git format-patch -1 --stdout --cover-letter >output &&
 	grep " | " output >actual &&
 	test_cmp expect72 actual
 '
@@ -131,7 +131,7 @@ EOF
 while read verb expect cmd args
 do
 	test_expect_success "$cmd $verb COLUMNS (big change)" '
-		COLUMNS=200 git $cmd $args >output &&
+		test_with_columns 200 git $cmd $args >output &&
 		grep " | " output >actual &&
 		test_cmp "$expect" actual
 	'
@@ -139,7 +139,7 @@ do
 	case "$cmd" in diff|show) continue;; esac
 
 	test_expect_success "$cmd --graph $verb COLUMNS (big change)" '
-		COLUMNS=200 git $cmd $args --graph >output &&
+		test_with_columns 200 git $cmd $args --graph >output &&
 		grep " | " output >actual &&
 		test_cmp "$expect-graph" actual
 	'
@@ -159,7 +159,7 @@ EOF
 while read verb expect cmd args
 do
 	test_expect_success "$cmd $verb not enough COLUMNS (big change)" '
-		COLUMNS=40 git $cmd $args >output &&
+		test_with_columns 40 git $cmd $args >output &&
 		grep " | " output >actual &&
 		test_cmp "$expect" actual
 	'
@@ -167,7 +167,7 @@ do
 	case "$cmd" in diff|show) continue;; esac
 
 	test_expect_success "$cmd --graph $verb not enough COLUMNS (big change)" '
-		COLUMNS=40 git $cmd $args --graph >output &&
+		test_with_columns 40 git $cmd $args --graph >output &&
 		grep " | " output >actual &&
 		test_cmp "$expect-graph" actual
 	'
@@ -302,7 +302,7 @@ EOF
 while read verb expect cmd args
 do
 	test_expect_success "$cmd $verb COLUMNS (long filename)" '
-		COLUMNS=200 git $cmd $args >output &&
+		test_with_columns 200 git $cmd $args >output &&
 		grep " | " output >actual &&
 		test_cmp "$expect" actual
 	'
@@ -310,7 +310,7 @@ do
 	case "$cmd" in diff|show) continue;; esac
 
 	test_expect_success "$cmd --graph $verb COLUMNS (long filename)" '
-		COLUMNS=200 git $cmd $args --graph >output &&
+		test_with_columns 200 git $cmd $args --graph >output &&
 		grep " | " output >actual &&
 		test_cmp "$expect-graph" actual
 	'
@@ -331,7 +331,7 @@ while read verb expect cmd args
 do
 	test_expect_success COLUMNS_CAN_BE_1 \
 		"$cmd $verb prefix greater than COLUMNS (big change)" '
-		COLUMNS=1 git $cmd $args >output &&
+		test_with_columns 1 git $cmd $args >output &&
 		grep " | " output >actual &&
 		test_cmp "$expect" actual
 	'
@@ -340,7 +340,7 @@ do
 
 	test_expect_success COLUMNS_CAN_BE_1 \
 		"$cmd --graph $verb prefix greater than COLUMNS (big change)" '
-		COLUMNS=1 git $cmd $args --graph >output &&
+		test_with_columns 1 git $cmd $args --graph >output &&
 		grep " | " output >actual &&
 		test_cmp "$expect-graph" actual
 	'
@@ -356,7 +356,7 @@ cat >expect <<'EOF'
 EOF
 test_expect_success 'merge --stat respects COLUMNS (big change)' '
 	git checkout -b branch HEAD^^ &&
-	COLUMNS=100 git merge --stat --no-ff main^ >output &&
+	test_with_columns 100 git merge --stat --no-ff main^ >output &&
 	grep " | " output >actual &&
 	test_cmp expect actual
 '
@@ -365,7 +365,7 @@ cat >expect <<'EOF'
  aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 +++++++++++++++++++++++++++++++++++++++
 EOF
 test_expect_success 'merge --stat respects COLUMNS (long filename)' '
-	COLUMNS=100 git merge --stat --no-ff main >output &&
+	test_with_columns 100 git merge --stat --no-ff main >output &&
 	grep " | " output >actual &&
 	test_cmp expect actual
 '
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 5865daa8f8d..a035f749537 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -207,7 +207,7 @@ test_expect_success 'left alignment formatting at the nth column' '
 '
 
 test_expect_success 'left alignment formatting at the nth column' '
-	COLUMNS=50 git log --pretty="tformat:%h %<|(-10)%s" >actual &&
+	test_with_columns 50 git log --pretty="tformat:%h %<|(-10)%s" >actual &&
 	qz_to_tab_space <<-EOF >expected &&
 	$head1 message two                    Z
 	$head2 message one                    Z
@@ -350,7 +350,7 @@ test_expect_success 'right alignment formatting at the nth column' '
 '
 
 test_expect_success 'right alignment formatting at the nth column' '
-	COLUMNS=50 git log --pretty="tformat:%h %>|(-10)%s" >actual &&
+	test_with_columns 50 git log --pretty="tformat:%h %>|(-10)%s" >actual &&
 	qz_to_tab_space <<-EOF >expected &&
 	$head1                      message two
 	$head2                      message one
@@ -450,7 +450,7 @@ test_expect_success 'center alignment formatting at the nth column' '
 '
 
 test_expect_success 'center alignment formatting at the nth column' '
-	COLUMNS=70 git log --pretty="tformat:%h %><|(-30)%s" >actual &&
+	test_with_columns 70 git log --pretty="tformat:%h %><|(-30)%s" >actual &&
 	qz_to_tab_space <<-EOF >expected &&
 	$head1           message two          Z
 	$head2           message one          Z
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 2f72c5c6883..e893c43d705 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -372,7 +372,7 @@ test_expect_success 'tag -l <pattern> -l <pattern> works, as our buggy documenta
 '
 
 test_expect_success 'listing tags in column' '
-	COLUMNS=41 git tag -l --column=row >actual &&
+	test_with_columns 41 git tag -l --column=row >actual &&
 	cat >expected <<\EOF &&
 a1      aa1     cba     t210    t211
 v0.2.1  v1.0    v1.0.1  v1.1.3
@@ -383,7 +383,7 @@ EOF
 test_expect_success 'listing tags in column with column.*' '
 	test_config column.tag row &&
 	test_config column.ui dense &&
-	COLUMNS=40 git tag -l >actual &&
+	test_with_columns 40 git tag -l >actual &&
 	cat >expected <<\EOF &&
 a1      aa1   cba     t210    t211
 v0.2.1  v1.0  v1.0.1  v1.1.3
@@ -397,7 +397,7 @@ test_expect_success 'listing tag with -n --column should fail' '
 
 test_expect_success 'listing tags -n in column with column.ui ignored' '
 	test_config column.ui "row dense" &&
-	COLUMNS=40 git tag -l -n >actual &&
+	test_with_columns 40 git tag -l -n >actual &&
 	cat >expected <<\EOF &&
 a1              Foo
 aa1             Foo
diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index 0e7cf75435e..5189699debf 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -651,7 +651,7 @@ test_expect_success TTY 'git tag with auto-columns ' '
 	cat >expect <<-\EOF &&
 	initial  one      two      three    four     five
 	EOF
-	test_terminal env PAGER="cat >actual" COLUMNS=80 \
+	test_with_columns 80 test_terminal env PAGER="cat >actual" \
 		git -c column.ui=auto tag --sort=authordate &&
 	test_cmp expect actual
 '
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index 2b72451ba3e..808b8f50576 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -108,13 +108,13 @@ test_expect_success 'status --column' '
 #	dir2/modified  untracked
 #
 EOF
-	COLUMNS=50 git -c status.displayCommentPrefix=true status --column="column dense" >output &&
+	test_with_columns 50 git -c status.displayCommentPrefix=true status --column="column dense" >output &&
 	test_cmp expect output
 '
 
 test_expect_success 'status --column status.displayCommentPrefix=false' '
 	strip_comments expect &&
-	COLUMNS=49 git -c status.displayCommentPrefix=false status --column="column dense" >output &&
+	test_with_columns 49 git -c status.displayCommentPrefix=false status --column="column dense" >output &&
 	test_cmp expect output
 '
 
diff --git a/t/t9002-column.sh b/t/t9002-column.sh
index 89983527b62..9151d69bcf1 100755
--- a/t/t9002-column.sh
+++ b/t/t9002-column.sh
@@ -46,7 +46,7 @@ test_expect_success '80 columns' '
 	cat >expected <<\EOF &&
 one    two    three  four   five   six    seven  eight  nine   ten    eleven
 EOF
-	COLUMNS=80 git column --mode=column <lista >actual &&
+	test_with_columns 80 git column --mode=column <lista >actual &&
 	test_cmp expected actual
 '
 
@@ -65,7 +65,7 @@ eleven
 EOF
 
 test_expect_success COLUMNS_CAN_BE_1 'COLUMNS = 1' '
-	COLUMNS=1 git column --mode=column <lista >actual &&
+	test_with_columns 1 git column --mode=column <lista >actual &&
 	test_cmp expected actual
 '
 
@@ -74,9 +74,6 @@ test_expect_success 'width = 1' '
 	test_cmp expected actual
 '
 
-COLUMNS=20
-export COLUMNS
-
 test_expect_success '20 columns' '
 	cat >expected <<\EOF &&
 one    seven
@@ -86,7 +83,7 @@ four   ten
 five   eleven
 six
 EOF
-	git column --mode=column <lista >actual &&
+	test_with_columns 20 git column --mode=column <lista >actual &&
 	test_cmp expected actual
 '
 
@@ -99,7 +96,7 @@ four   ten
 five   eleven
 six
 EOF
-	git column --mode=column,nodense < lista > actual &&
+	test_with_columns 20 git column --mode=column,nodense < lista > actual &&
 	test_cmp expected actual
 '
 
@@ -110,7 +107,7 @@ two   six   ten
 three seven eleven
 four  eight
 EOF
-	git column --mode=column,dense < lista > actual &&
+	test_with_columns 20 git column --mode=column,dense < lista > actual &&
 	test_cmp expected actual
 '
 
@@ -123,7 +120,7 @@ four    ten
 five    eleven
 six
 EOF
-	git column --mode=column --padding 2 <lista >actual &&
+	test_with_columns 20 git column --mode=column --padding 2 <lista >actual &&
 	test_cmp expected actual
 '
 
@@ -136,7 +133,7 @@ test_expect_success '20 columns, indented' '
   five   eleven
   six
 EOF
-	git column --mode=column --indent="  " <lista >actual &&
+	test_with_columns 20 git column --mode=column --indent="  " <lista >actual &&
 	test_cmp expected actual
 '
 
@@ -149,7 +146,7 @@ seven  eight
 nine   ten
 eleven
 EOF
-	git column --mode=row <lista >actual &&
+	test_with_columns 20 git column --mode=row <lista >actual &&
 	test_cmp expected actual
 '
 
@@ -162,7 +159,7 @@ seven  eight
 nine   ten
 eleven
 EOF
-	git column --mode=row,nodense <lista >actual &&
+	test_with_columns 20 git column --mode=row,nodense <lista >actual &&
 	test_cmp expected actual
 '
 
@@ -173,7 +170,7 @@ four  five   six
 seven eight  nine
 ten   eleven
 EOF
-	git column --mode=row,dense <lista >actual &&
+	test_with_columns 20 git column --mode=row,dense <lista >actual &&
 	test_cmp expected actual
 '
 
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 37da7d9a99a..536339faaa2 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -1718,3 +1718,11 @@ test_region () {
 test_readlink () {
 	perl -le 'print readlink($_) for @ARGV' "$@"
 }
+
+# Test a with a given number of COLUMNS in the environment.
+test_with_columns () {
+	local columns=$1
+	shift
+
+	COLUMNS=$columns "$@"
+}

-- 
2.32.0.1070.gec115ccd780

SZEDER Gábor· Aug 2, 2021, 17:14 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH v2 2/3] test-lib-functions.sh: add a test_with_columns function

On Mon, Aug 02, 2021 at 03:46:27PM +0200, Ævar Arnfjörð Bjarmason wrote:

> Add a helper function to wrap patterns of "COLUMNS=N <command>" as
> "test_with_columns N <command>". This will be used and extended in the
> next commit.

> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index 37da7d9a99a..536339faaa2 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -1718,3 +1718,11 @@ test_region () {
>  test_readlink () {
>  	perl -le 'print readlink($_) for @ARGV' "$@"
>  }
> +
> +# Test a with a given number of COLUMNS in the environment.
> +test_with_columns () {
> +	local columns=$1
> +	shift
> +
> +	COLUMNS=$columns "$@"
> +}

Eric Sunshine· Aug 2, 2021, 17:24 UTC · re: SZEDER Gábor · lore

Re: [PATCH v2 2/3] test-lib-functions.sh: add a test_with_columns function

On Mon, Aug 2, 2021 at 1:14 PM SZEDER Gábor <szeder.dev@gmail.com> wrote:

> On Mon, Aug 02, 2021 at 03:46:27PM +0200, Ævar Arnfjörð Bjarmason wrote:
> > +# Test a with a given number of COLUMNS in the environment.
> > +test_with_columns () {
> > +     local columns=$1
> > +     shift
> > +
> > +     COLUMNS=$columns "$@"
> > +}
>
> This function needs some redirections to separate the tested command's
> standard error from the function's '-x' trace, see a5bf824f3b (t:
> prevent '-x' tracing from interfering with test helpers' stderr,
> 2018-02-25).

    FOO=bar command

idiom and instead use:

    test_must_fail env FOO=bar command

Ævar Arnfjörð Bjarmason· Aug 4, 2021, 23:05 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH v3 0/3] test-lib.sh: use GIT_TEST_COLUMNS over COLUMNS

Ævar Arnfjörð Bjarmason (3):
  test-lib-functions.sh: rename test_must_fail_acceptable()
  test-lib-functions.sh: add a test_with_columns function
  test-lib.sh: use GIT_TEST_COLUMNS over COLUMNS

 pager.c                       |  7 +++++++
 t/t3200-branch.sh             |  8 ++++----
 t/t4052-stat-output.sh        | 22 +++++++++++-----------
 t/t4205-log-pretty-formats.sh |  6 +++---
 t/t7004-tag.sh                |  6 +++---
 t/t7006-pager.sh              |  2 +-
 t/t7508-status.sh             |  4 ++--
 t/t9002-column.sh             | 23 ++++++++++-------------
 t/test-lib-functions.sh       | 21 ++++++++++++++++++---
 t/test-lib.sh                 | 13 +++++++++++--
 10 files changed, 70 insertions(+), 42 deletions(-)

Range-diff against v2:
1:  739457b992f = 1:  f45590a76d5 test-lib-functions.sh: rename test_must_fail_acceptable()
2:  36c57178c55 ! 2:  53e6e25ece6 test-lib-functions.sh: add a test_with_columns function
    @@ t/test-lib-functions.sh: test_region () {
     +	local columns=$1
     +	shift
     +
    -+	COLUMNS=$columns "$@"
    -+}
    ++	if ! is_git_command_name "$@"
    ++	then
    ++		echo >&7 "test_with_columns: only 'git' is allowed: $*"
    ++		return 1
    ++	fi
    ++
    ++	COLUMNS=$columns "$@" 2>&7
    ++} 7>&2 2>&4
3:  6cbbb955e9a ! 3:  74acba0f9ca test-lib.sh: use GIT_TEST_COLUMNS over COLUMNS
    @@ pager.c: int term_columns(void)
     
      ## t/test-lib-functions.sh ##
     @@ t/test-lib-functions.sh: test_with_columns () {
    - 	local columns=$1
    - 	shift
    + 		return 1
    + 	fi
      
    -+	if ! is_git_command_name "$@"
    -+	then
    -+		echo >&7 "test_with_columns: only 'git' is allowed: $*"
    -+		return 1
    -+	fi
    -+
     +	GIT_TEST_COLUMNS= \
    - 	COLUMNS=$columns "$@"
    - }
    + 	COLUMNS=$columns "$@" 2>&7
    + } 7>&2 2>&4
     
      ## t/test-lib.sh ##
     @@ t/test-lib.sh: LANG=C

-- 
2.33.0.rc0.597.gc569a812f0a

Ævar Arnfjörð Bjarmason· Aug 4, 2021, 23:05 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH v3 1/3] test-lib-functions.sh: rename test_must_fail_acceptable()

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/test-lib-functions.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index e28411bb75a..37da7d9a99a 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -895,7 +895,7 @@ list_contains () {
 # accepted by test_must_fail(). If the command is run with env, the env
 # and its corresponding variable settings will be stripped before we
 # test the command being run.
-test_must_fail_acceptable () {
+is_git_command_name () {
 	if test "$1" = "env"
 	then
 		shift
@@ -943,7 +943,7 @@ test_must_fail_acceptable () {
 #     (Don't use 'success', use 'test_might_fail' instead.)
 #
 # Do not use this to run anything but "git" and other specific testable
-# commands (see test_must_fail_acceptable()).  We are not in the
+# commands (see is_git_command_name()).  We are not in the
 # business of vetting system supplied commands -- in other words, this
 # is wrong:
 #
@@ -963,7 +963,7 @@ test_must_fail () {
 		_test_ok=
 		;;
 	esac
-	if ! test_must_fail_acceptable "$@"
+	if ! is_git_command_name "$@"
 	then
 		echo >&7 "test_must_fail: only 'git' is allowed: $*"
 		return 1

-- 
2.33.0.rc0.597.gc569a812f0a

Ævar Arnfjörð Bjarmason· Aug 4, 2021, 23:05 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH v3 3/3] test-lib.sh: use GIT_TEST_COLUMNS over COLUMNS

	shopt -u checkwinsize >/dev/null 2>&1

Reported-by: Fabian Stelzer <fabian.stelzer@campoint.net>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 pager.c                 |  7 +++++++
 t/test-lib-functions.sh |  1 +
 t/test-lib.sh           | 13 +++++++++++--
 3 files changed, 19 insertions(+), 2 deletions(-)

diff --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

Ævar Arnfjörð Bjarmason· Aug 4, 2021, 23:05 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH v3 2/3] test-lib-functions.sh: add a test_with_columns function

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t3200-branch.sh             |  8 ++++----
 t/t4052-stat-output.sh        | 22 +++++++++++-----------
 t/t4205-log-pretty-formats.sh |  6 +++---
 t/t7004-tag.sh                |  6 +++---
 t/t7006-pager.sh              |  2 +-
 t/t7508-status.sh             |  4 ++--
 t/t9002-column.sh             | 23 ++++++++++-------------
 t/test-lib-functions.sh       | 14 ++++++++++++++
 8 files changed, 48 insertions(+), 37 deletions(-)

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index cc4b10236e2..e568156ca83 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -328,7 +328,7 @@ test_expect_success 'git branch --list -v with --abbrev' '
 '
 
 test_expect_success 'git branch --column' '
-	COLUMNS=81 git branch --column=column >actual &&
+	test_with_columns 81 git branch --column=column >actual &&
 	cat >expect <<\EOF &&
   a/b/c   bam     foo     l     * main    n       o/p     r
   abc     bar     j/k     m/m     mb      o/o     q       topic
@@ -341,7 +341,7 @@ test_expect_success 'git branch --column with an extremely long branch name' '
 	long=z$long/$long/$long/$long &&
 	test_when_finished "git branch -d $long" &&
 	git branch $long &&
-	COLUMNS=80 git branch --column=column >actual &&
+	test_with_columns 80 git branch --column=column >actual &&
 	cat >expect <<EOF &&
   a/b/c
   abc
@@ -367,7 +367,7 @@ EOF
 test_expect_success 'git branch with column.*' '
 	git config column.ui column &&
 	git config column.branch "dense" &&
-	COLUMNS=80 git branch >actual &&
+	test_with_columns 80 git branch >actual &&
 	git config --unset column.branch &&
 	git config --unset column.ui &&
 	cat >expect <<\EOF &&
@@ -383,7 +383,7 @@ test_expect_success 'git branch --column -v should fail' '
 
 test_expect_success 'git branch -v with column.ui ignored' '
 	git config column.ui column &&
-	COLUMNS=80 git branch -v | cut -c -8 | sed "s/ *$//" >actual &&
+	test_with_columns 80 git branch -v | cut -c -8 | sed "s/ *$//" >actual &&
 	git config --unset column.ui &&
 	cat >expect <<\EOF &&
   a/b/c
diff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh
index 9eba436211f..3a91df50dd8 100755
--- a/t/t4052-stat-output.sh
+++ b/t/t4052-stat-output.sh
@@ -111,7 +111,7 @@ cat >expect72 <<'EOF'
  abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 EOF
 test_expect_success "format-patch --cover-letter ignores COLUMNS (big change)" '
-	COLUMNS=200 git format-patch -1 --stdout --cover-letter >output &&
+	test_with_columns 200 git format-patch -1 --stdout --cover-letter >output &&
 	grep " | " output >actual &&
 	test_cmp expect72 actual
 '
@@ -131,7 +131,7 @@ EOF
 while read verb expect cmd args
 do
 	test_expect_success "$cmd $verb COLUMNS (big change)" '
-		COLUMNS=200 git $cmd $args >output &&
+		test_with_columns 200 git $cmd $args >output &&
 		grep " | " output >actual &&
 		test_cmp "$expect" actual
 	'
@@ -139,7 +139,7 @@ do
 	case "$cmd" in diff|show) continue;; esac
 
 	test_expect_success "$cmd --graph $verb COLUMNS (big change)" '
-		COLUMNS=200 git $cmd $args --graph >output &&
+		test_with_columns 200 git $cmd $args --graph >output &&
 		grep " | " output >actual &&
 		test_cmp "$expect-graph" actual
 	'
@@ -159,7 +159,7 @@ EOF
 while read verb expect cmd args
 do
 	test_expect_success "$cmd $verb not enough COLUMNS (big change)" '
-		COLUMNS=40 git $cmd $args >output &&
+		test_with_columns 40 git $cmd $args >output &&
 		grep " | " output >actual &&
 		test_cmp "$expect" actual
 	'
@@ -167,7 +167,7 @@ do
 	case "$cmd" in diff|show) continue;; esac
 
 	test_expect_success "$cmd --graph $verb not enough COLUMNS (big change)" '
-		COLUMNS=40 git $cmd $args --graph >output &&
+		test_with_columns 40 git $cmd $args --graph >output &&
 		grep " | " output >actual &&
 		test_cmp "$expect-graph" actual
 	'
@@ -302,7 +302,7 @@ EOF
 while read verb expect cmd args
 do
 	test_expect_success "$cmd $verb COLUMNS (long filename)" '
-		COLUMNS=200 git $cmd $args >output &&
+		test_with_columns 200 git $cmd $args >output &&
 		grep " | " output >actual &&
 		test_cmp "$expect" actual
 	'
@@ -310,7 +310,7 @@ do
 	case "$cmd" in diff|show) continue;; esac
 
 	test_expect_success "$cmd --graph $verb COLUMNS (long filename)" '
-		COLUMNS=200 git $cmd $args --graph >output &&
+		test_with_columns 200 git $cmd $args --graph >output &&
 		grep " | " output >actual &&
 		test_cmp "$expect-graph" actual
 	'
@@ -331,7 +331,7 @@ while read verb expect cmd args
 do
 	test_expect_success COLUMNS_CAN_BE_1 \
 		"$cmd $verb prefix greater than COLUMNS (big change)" '
-		COLUMNS=1 git $cmd $args >output &&
+		test_with_columns 1 git $cmd $args >output &&
 		grep " | " output >actual &&
 		test_cmp "$expect" actual
 	'
@@ -340,7 +340,7 @@ do
 
 	test_expect_success COLUMNS_CAN_BE_1 \
 		"$cmd --graph $verb prefix greater than COLUMNS (big change)" '
-		COLUMNS=1 git $cmd $args --graph >output &&
+		test_with_columns 1 git $cmd $args --graph >output &&
 		grep " | " output >actual &&
 		test_cmp "$expect-graph" actual
 	'
@@ -356,7 +356,7 @@ cat >expect <<'EOF'
 EOF
 test_expect_success 'merge --stat respects COLUMNS (big change)' '
 	git checkout -b branch HEAD^^ &&
-	COLUMNS=100 git merge --stat --no-ff main^ >output &&
+	test_with_columns 100 git merge --stat --no-ff main^ >output &&
 	grep " | " output >actual &&
 	test_cmp expect actual
 '
@@ -365,7 +365,7 @@ cat >expect <<'EOF'
  aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 +++++++++++++++++++++++++++++++++++++++
 EOF
 test_expect_success 'merge --stat respects COLUMNS (long filename)' '
-	COLUMNS=100 git merge --stat --no-ff main >output &&
+	test_with_columns 100 git merge --stat --no-ff main >output &&
 	grep " | " output >actual &&
 	test_cmp expect actual
 '
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 5865daa8f8d..a035f749537 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -207,7 +207,7 @@ test_expect_success 'left alignment formatting at the nth column' '
 '
 
 test_expect_success 'left alignment formatting at the nth column' '
-	COLUMNS=50 git log --pretty="tformat:%h %<|(-10)%s" >actual &&
+	test_with_columns 50 git log --pretty="tformat:%h %<|(-10)%s" >actual &&
 	qz_to_tab_space <<-EOF >expected &&
 	$head1 message two                    Z
 	$head2 message one                    Z
@@ -350,7 +350,7 @@ test_expect_success 'right alignment formatting at the nth column' '
 '
 
 test_expect_success 'right alignment formatting at the nth column' '
-	COLUMNS=50 git log --pretty="tformat:%h %>|(-10)%s" >actual &&
+	test_with_columns 50 git log --pretty="tformat:%h %>|(-10)%s" >actual &&
 	qz_to_tab_space <<-EOF >expected &&
 	$head1                      message two
 	$head2                      message one
@@ -450,7 +450,7 @@ test_expect_success 'center alignment formatting at the nth column' '
 '
 
 test_expect_success 'center alignment formatting at the nth column' '
-	COLUMNS=70 git log --pretty="tformat:%h %><|(-30)%s" >actual &&
+	test_with_columns 70 git log --pretty="tformat:%h %><|(-30)%s" >actual &&
 	qz_to_tab_space <<-EOF >expected &&
 	$head1           message two          Z
 	$head2           message one          Z
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 2f72c5c6883..e893c43d705 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -372,7 +372,7 @@ test_expect_success 'tag -l <pattern> -l <pattern> works, as our buggy documenta
 '
 
 test_expect_success 'listing tags in column' '
-	COLUMNS=41 git tag -l --column=row >actual &&
+	test_with_columns 41 git tag -l --column=row >actual &&
 	cat >expected <<\EOF &&
 a1      aa1     cba     t210    t211
 v0.2.1  v1.0    v1.0.1  v1.1.3
@@ -383,7 +383,7 @@ EOF
 test_expect_success 'listing tags in column with column.*' '
 	test_config column.tag row &&
 	test_config column.ui dense &&
-	COLUMNS=40 git tag -l >actual &&
+	test_with_columns 40 git tag -l >actual &&
 	cat >expected <<\EOF &&
 a1      aa1   cba     t210    t211
 v0.2.1  v1.0  v1.0.1  v1.1.3
@@ -397,7 +397,7 @@ test_expect_success 'listing tag with -n --column should fail' '
 
 test_expect_success 'listing tags -n in column with column.ui ignored' '
 	test_config column.ui "row dense" &&
-	COLUMNS=40 git tag -l -n >actual &&
+	test_with_columns 40 git tag -l -n >actual &&
 	cat >expected <<\EOF &&
 a1              Foo
 aa1             Foo
diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index 0e7cf75435e..5189699debf 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -651,7 +651,7 @@ test_expect_success TTY 'git tag with auto-columns ' '
 	cat >expect <<-\EOF &&
 	initial  one      two      three    four     five
 	EOF
-	test_terminal env PAGER="cat >actual" COLUMNS=80 \
+	test_with_columns 80 test_terminal env PAGER="cat >actual" \
 		git -c column.ui=auto tag --sort=authordate &&
 	test_cmp expect actual
 '
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index 2b72451ba3e..808b8f50576 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -108,13 +108,13 @@ test_expect_success 'status --column' '
 #	dir2/modified  untracked
 #
 EOF
-	COLUMNS=50 git -c status.displayCommentPrefix=true status --column="column dense" >output &&
+	test_with_columns 50 git -c status.displayCommentPrefix=true status --column="column dense" >output &&
 	test_cmp expect output
 '
 
 test_expect_success 'status --column status.displayCommentPrefix=false' '
 	strip_comments expect &&
-	COLUMNS=49 git -c status.displayCommentPrefix=false status --column="column dense" >output &&
+	test_with_columns 49 git -c status.displayCommentPrefix=false status --column="column dense" >output &&
 	test_cmp expect output
 '
 
diff --git a/t/t9002-column.sh b/t/t9002-column.sh
index 89983527b62..9151d69bcf1 100755
--- a/t/t9002-column.sh
+++ b/t/t9002-column.sh
@@ -46,7 +46,7 @@ test_expect_success '80 columns' '
 	cat >expected <<\EOF &&
 one    two    three  four   five   six    seven  eight  nine   ten    eleven
 EOF
-	COLUMNS=80 git column --mode=column <lista >actual &&
+	test_with_columns 80 git column --mode=column <lista >actual &&
 	test_cmp expected actual
 '
 
@@ -65,7 +65,7 @@ eleven
 EOF
 
 test_expect_success COLUMNS_CAN_BE_1 'COLUMNS = 1' '
-	COLUMNS=1 git column --mode=column <lista >actual &&
+	test_with_columns 1 git column --mode=column <lista >actual &&
 	test_cmp expected actual
 '
 
@@ -74,9 +74,6 @@ test_expect_success 'width = 1' '
 	test_cmp expected actual
 '
 
-COLUMNS=20
-export COLUMNS
-
 test_expect_success '20 columns' '
 	cat >expected <<\EOF &&
 one    seven
@@ -86,7 +83,7 @@ four   ten
 five   eleven
 six
 EOF
-	git column --mode=column <lista >actual &&
+	test_with_columns 20 git column --mode=column <lista >actual &&
 	test_cmp expected actual
 '
 
@@ -99,7 +96,7 @@ four   ten
 five   eleven
 six
 EOF
-	git column --mode=column,nodense < lista > actual &&
+	test_with_columns 20 git column --mode=column,nodense < lista > actual &&
 	test_cmp expected actual
 '
 
@@ -110,7 +107,7 @@ two   six   ten
 three seven eleven
 four  eight
 EOF
-	git column --mode=column,dense < lista > actual &&
+	test_with_columns 20 git column --mode=column,dense < lista > actual &&
 	test_cmp expected actual
 '
 
@@ -123,7 +120,7 @@ four    ten
 five    eleven
 six
 EOF
-	git column --mode=column --padding 2 <lista >actual &&
+	test_with_columns 20 git column --mode=column --padding 2 <lista >actual &&
 	test_cmp expected actual
 '
 
@@ -136,7 +133,7 @@ test_expect_success '20 columns, indented' '
   five   eleven
   six
 EOF
-	git column --mode=column --indent="  " <lista >actual &&
+	test_with_columns 20 git column --mode=column --indent="  " <lista >actual &&
 	test_cmp expected actual
 '
 
@@ -149,7 +146,7 @@ seven  eight
 nine   ten
 eleven
 EOF
-	git column --mode=row <lista >actual &&
+	test_with_columns 20 git column --mode=row <lista >actual &&
 	test_cmp expected actual
 '
 
@@ -162,7 +159,7 @@ seven  eight
 nine   ten
 eleven
 EOF
-	git column --mode=row,nodense <lista >actual &&
+	test_with_columns 20 git column --mode=row,nodense <lista >actual &&
 	test_cmp expected actual
 '
 
@@ -173,7 +170,7 @@ four  five   six
 seven eight  nine
 ten   eleven
 EOF
-	git column --mode=row,dense <lista >actual &&
+	test_with_columns 20 git column --mode=row,dense <lista >actual &&
 	test_cmp expected actual
 '
 
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 37da7d9a99a..adc853109e4 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -1718,3 +1718,17 @@ test_region () {
 test_readlink () {
 	perl -le 'print readlink($_) for @ARGV' "$@"
 }
+
+# Test a with a given number of COLUMNS in the environment.
+test_with_columns () {
+	local columns=$1
+	shift
+
+	if ! is_git_command_name "$@"
+	then
+		echo >&7 "test_with_columns: only 'git' is allowed: $*"
+		return 1
+	fi
+
+	COLUMNS=$columns "$@" 2>&7
+} 7>&2 2>&4

-- 
2.33.0.rc0.597.gc569a812f0a