Y01 Test Failure on Arch

Y01 Test Failure on Arch

[Vin Shelton]

[-- Attachment #1: Type: text/plain, Size: 2055 bytes --]

Building from the latest sources on endeavouros (Arch):

TESTNUM=Y01 make check
cd Test ; make check
make[1]: Entering directory '/SSD-2/opt/build/zsh-2021-03-31/Test'
if test -n "gcc"; then \
  cd .. && DESTDIR= \
  make MODDIR=`pwd`/Test/Modules install.modules > /dev/null; \
fi
if test -z "$ZTST_handler"; then \
  ZTST_handler=runtests.zsh; \
fi; \
if ZTST_testlist="`for f in ../../../src/zsh-2021-03-31/Test/Y01*.ztst; \
           do echo $f; done`" \
 ZTST_srcdir="../../../src/zsh-2021-03-31/Test" \
 ZTST_exe=../Src/zsh \
 ../Src/zsh +Z -f ../../../src/zsh-2021-03-31/Test/$ZTST_handler; then \
 stat=0; \
else \
 stat=1; \
fi; \
sleep 1; \
rm -rf Modules .zcompdump; \
exit $stat
../../../src/zsh-2021-03-31/Test/Y01completion.ztst: starting.
--- /tmp/zsh.ztst.628309/ztst.out 2021-03-31 09:31:50.414616505 -0400
+++ /tmp/zsh.ztst.628309/ztst.tout 2021-03-31 09:31:50.647949508 -0400
@@ -1,16 +1,12 @@
-line: {: dir1/}{}
-DESCRIPTION:{expansions}
+line: {: dir1 dir2 file1 file2 }{}
+line: {: dir1 dir2 file1 file2 }{}
+DESCRIPTION:{file}
 DI:{dir1}
 DI:{dir2}
 FI:{file1}
 FI:{file2}
-DESCRIPTION:{all expansions}
-NO:{dir1 dir2 file1 file2}
-DESCRIPTION:{original}
-NO:{*}
-line: {: dir1/}{}
-line: {: dir2/}{}
-line: {: file1 }{}
-line: {: file2 }{}
-line: {: dir1 dir2 file1 file2 }{}
-line: {: *}{}
+line: {: dir1 dir2 file1 file2 dir1/}{}
+line: {: dir1 dir2 file1 file2 dir2/}{}
+line: {: dir1 dir2 file1 file2 file1}{}
+line: {: dir1 dir2 file1 file2 file2}{}
+line: {: dir1 dir2 file1 file2 dir1/}{}
Test ../../../src/zsh-2021-03-31/Test/Y01completion.ztst failed: output
differs from expected as shown above for:
  comptest $': *\t\t\t\t\t\t\t'
Was testing: _expand shows file types
../../../src/zsh-2021-03-31/Test/Y01completion.ztst: test failed.
**************************************
0 successful test scripts, 1 failure, 0 skipped
**************************************
make[1]: *** [Makefile:190: check] Error 1
make[1]: Leaving directory '/SSD-2/opt/build/zsh-2021-03-31/Test'
make: *** [Makefile:263: check] Error 2

  - Vin

[-- Attachment #2: Type: text/html, Size: 5396 bytes --]

Re: Y01 Test Failure on Arch

[Jun T]

> 2021/03/31 22:40, Vin Shelton <acs@alumni.princeton.edu> wrote:
> 
> Building from the latest sources on endeavouros (Arch):
(snip)
> Test ../../../src/zsh-2021-03-31/Test/Y01completion.ztst failed: output differs from expected as shown above for:
>   comptest $': *\t\t\t\t\t\t\t'
> Was testing: _expand shows file types

Same in Ubuntu, Fedora and macOS.

It seems we need to call complete-word instead of expand-or-complete,
and include _expand in the 'completer' style.

The patch below works (all the test pass with the patch), but
I'm not sure this is the best solution.

In the patch I simply replaced expand-or-complete by complete-word
in the function expand-or-complete-with-report() defined in comptest.
(Do we also need to change the function name etc.?)

I also removed extra '\t's from the test since the first \t is enough
for testing whether _expand correctly outputs file types.


diff --git a/Test/Y01completion.ztst b/Test/Y01completion.ztst
index eff6910c2..ff7a28120 100644
--- a/Test/Y01completion.ztst
+++ b/Test/Y01completion.ztst
@@ -44,7 +44,7 @@
 >line: {: dir1/}{}
 >line: {: dir2/}{}
 
-  comptest $': *\t\t\t\t\t\t\t'
+  comptest $': *\t'
 0:_expand shows file types
 >line: {: dir1/}{}
 >DESCRIPTION:{expansions}
@@ -56,12 +56,6 @@
 >NO:{dir1 dir2 file1 file2}
 >DESCRIPTION:{original}
 >NO:{*}
->line: {: dir1/}{}
->line: {: dir2/}{}
->line: {: file1 }{}
->line: {: file2 }{}
->line: {: dir1 dir2 file1 file2 }{}
->line: {: *}{}
 
   comptesteval '_users () { compadd user1 user2 }'
   comptest $': ~\t\t\t\t\t'
diff --git a/Test/comptest b/Test/comptest
index a36e301e0..8f4081b7d 100644
--- a/Test/comptest
+++ b/Test/comptest
@@ -48,11 +48,12 @@ zstyle ":completion:*:descriptions" format "<DESCRIPTION>%d</DESCRIPTION>
 "
 zstyle ":completion:*:options" verbose yes
 zstyle ":completion:*:values" verbose yes
+zstyle ":completion:*" completer _expand _complete
 setopt noalwayslastprompt listrowsfirst completeinword
 zmodload zsh/complist
 expand-or-complete-with-report () {
   print -lr "<WIDGET><expand-or-complete>"
-  zle expand-or-complete
+  zle complete-word
   print -lr - "<LBUFFER>$LBUFFER</LBUFFER>" "<RBUFFER>$RBUFFER</RBUFFER>"
   zle clear-screen
   zle -R

Re: Y01 Test Failure on Arch

[Vin Shelton]

[-- Attachment #1: Type: text/plain, Size: 931 bytes --]

Ping.  The failure still occurs.

HTH,
  Vin

On Thu, Apr 1, 2021 at 12:05 AM Jun T <takimoto-j@kba.biglobe.ne.jp> wrote:

>
> > 2021/03/31 22:40, Vin Shelton <acs@alumni.princeton.edu> wrote:
> >
> > Building from the latest sources on endeavouros (Arch):
> (snip)
> > Test ../../../src/zsh-2021-03-31/Test/Y01completion.ztst failed: output
> differs from expected as shown above for:
> >   comptest $': *\t\t\t\t\t\t\t'
> > Was testing: _expand shows file types
>
> Same in Ubuntu, Fedora and macOS.
>
> It seems we need to call complete-word instead of expand-or-complete,
> and include _expand in the 'completer' style.
>
> The patch below works (all the test pass with the patch), but
> I'm not sure this is the best solution.
>
> In the patch I simply replaced expand-or-complete by complete-word
> in the function expand-or-complete-with-report() defined in comptest.
> (Do we also need to change the function name etc.?)
>

[-- Attachment #2: Type: text/html, Size: 1781 bytes --]

Re: Y01 Test Failure on Arch

[Jun. T]

> 2021/04/03 20:32, Vin Shelton <acs@alumni.princeton.edu> wrote:
> 
> Ping.  The failure still occurs.

Do you mean it still fails with my patch?

-----
Jun

Re: Y01 Test Failure on Arch

[Vin Shelton]

[-- Attachment #1: Type: text/plain, Size: 590 bytes --]

On Sun, Apr 4, 2021 at 12:05 AM Jun. T <takimoto-j@kba.biglobe.ne.jp> wrote:

>
> > 2021/04/03 20:32, Vin Shelton <acs@alumni.princeton.edu> wrote:
> >
> > Ping.  The failure still occurs.
>
> Do you mean it still fails with my patch?
>

No, I mean your patch hasn't been applied.  ISTR you also expressed
concerns that your patch wasn't certainly the right way to go.  Also, *why*
was the test change necessary in the first place?  Philosophically, I think
the tests should reflect the current behavior of the shell - one doesn't
change the test unless one understands why.

Thanks,
  Vin

[-- Attachment #2: Type: text/html, Size: 1583 bytes --]

Re: Y01 Test Failure on Arch

[Daniel Shahaf]

Vin Shelton wrote on Sun, 04 Apr 2021 14:00 +00:00:
> 
> 
> On Sun, Apr 4, 2021 at 12:05 AM Jun. T <takimoto-j@kba.biglobe.ne.jp> wrote:
> > 
> > > 2021/04/03 20:32, Vin Shelton <acs@alumni.princeton.edu> wrote:
> > > 
> > > Ping.  The failure still occurs.
> > 
> > Do you mean it still fails with my patch?
>  
> No, I mean your patch hasn't been applied.  ISTR you also expressed 
> concerns that your patch wasn't certainly the right way to go.  Also, 
> *why* was the test change necessary in the first place?  
> Philosophically, I think the tests should reflect the current behavior 
> of the shell - one doesn't change the test unless one understands why.

Vin, thanks for the nudges and for the review.

Marlon, the failure is due to workers/48214, which is yours.  Thoughts?

Cheers,

Daniel

Re: Y01 Test Failure on Arch

[Marlon Richert]

On Sun, Apr 4, 2021 at 8:02 PM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> Marlon, the failure is due to workers/48214, which is yours.  Thoughts?

My original patch included the necessary changes to `comptest` to make
the tests work correctly. See
https://www.zsh.org/mla/workers/2021/msg00427.html (relevant part
included below). However, when I now do `git show
07a30613f0b0a5b782d30dbeb46df8382f8a4e2a`, it appears that Oliver left
those out of the actual commit.

Oliver, what was the reason you did not commit the changes below in 07a30613f?

diff --git a/Test/comptest b/Test/comptest
index a36e301e0..cdb3ecc61 100644
--- a/Test/comptest
+++ b/Test/comptest
@@ -40,6 +40,7 @@ KEYTIMEOUT=1
 setopt zle
 autoload -U compinit
 compinit -u
+zstyle ":completion:*" completer _expand _complete _ignored
 zstyle ":completion:*:default" list-colors "no=<NO>" "fi=<FI>"
"di=<DI>" "ln=<LN>" "pi=<PI>" "so=<SO>" "bd=<BD>" "cd=<CD>" "ex=<EX>"
"mi=<MI>" "tc=<TC>" "sp=<SP>" "lc=<LC>" "ec=<EC>\n" "rc=<RC>"
 zstyle ":completion:*" group-name ""
 zstyle ":completion:*:messages" format "<MESSAGE>%d</MESSAGE>
@@ -50,9 +51,9 @@ zstyle ":completion:*:options" verbose yes
 zstyle ":completion:*:values" verbose yes
 setopt noalwayslastprompt listrowsfirst completeinword
 zmodload zsh/complist
-expand-or-complete-with-report () {
-  print -lr "<WIDGET><expand-or-complete>"
-  zle expand-or-complete
+complete-word-with-report () {
+  print -lr "<WIDGET><complete-word>"
+  zle complete-word
   print -lr - "<LBUFFER>$LBUFFER</LBUFFER>" "<RBUFFER>$RBUFFER</RBUFFER>"
   zle clear-screen
   zle -R
@@ -80,11 +81,11 @@ zle-finish () {
   (( $+mark )) && print -lr "MARK: $mark"
   zle accept-line
 }
-zle -N expand-or-complete-with-report
+zle -N complete-word-with-report
 zle -N list-choices-with-report
 zle -N comp-finish
 zle -N zle-finish
-bindkey "^I" expand-or-complete-with-report
+bindkey "^I" complete-word-with-report
 bindkey "^D" list-choices-with-report
 bindkey "^Z" comp-finish
 bindkey "^X" zle-finish

Re: Y01 Test Failure on Arch

[Jun T]

> 2021/04/05 23:58, Marlon Richert <marlon.richert@gmail.com> wrote:
> 
> On Sun, Apr 4, 2021 at 8:02 PM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
>> Marlon, the failure is due to workers/48214, which is yours.  Thoughts?
> 
> My original patch included the necessary changes to `comptest` to make
> the tests work correctly. See
> https://www.zsh.org/mla/workers/2021/msg00427.html (relevant part
> included below). However, when I now do `git show
> 07a30613f0b0a5b782d30dbeb46df8382f8a4e2a`, it appears that Oliver left
> those out of the actual commit.

Aha!
It was a mystery for me how you could run Y01 successfully.
Sorry for not going back to your original post.
I believe Oliver was just too busy when committing the patch.

But the test still fails as follows:

--- /tmp/zsh.ztst.34237/ztst.out	2021-04-06 13:06:20.000000000 +0900
+++ /tmp/zsh.ztst.34237/ztst.tout	2021-04-06 13:06:20.000000000 +0900
@@ -8,9 +8,9 @@
 NO:{dir1 dir2 file1 file2}
 DESCRIPTION:{original}
 NO:{*}
-line: {: dir1/}{}
 line: {: dir2/}{}
 line: {: file1 }{}
 line: {: file2 }{}
 line: {: dir1 dir2 file1 file2 }{}
 line: {: *}{}
+line: {: dir1/}{}
Test ./Y01completion.ztst failed: output differs from expected as shown above for:
  comptest $': *\t\t\t\t\t\t\t'
Was testing: _expand shows file types
./Y01completion.ztst: test failed.

Can you reproduce this?
Is it OK to replace "comptest $': *\t\t\t\t\t\t\t'" by "comptest $': *\t'"
as in my patch in workers/48363⁩?

Re: Y01 Test Failure on Arch

[Marlon]

[-- Attachment #1: Type: text/plain, Size: 1314 bytes --]


> On 6. Apr 2021, at 7.23, Jun T <takimoto-j@kba.biglobe.ne.jp> wrote:
> 
> Aha!
> It was a mystery for me how you could run Y01 successfully.
> Sorry for not going back to your original post.
> I believe Oliver was just too busy when committing the patch.
> 
> But the test still fails as follows:
> 
> --- /tmp/zsh.ztst.34237/ztst.out	2021-04-06 13:06:20.000000000 +0900
> +++ /tmp/zsh.ztst.34237/ztst.tout	2021-04-06 13:06:20.000000000 +0900
> @@ -8,9 +8,9 @@
> NO:{dir1 dir2 file1 file2}
> DESCRIPTION:{original}
> NO:{*}
> -line: {: dir1/}{}
> line: {: dir2/}{}
> line: {: file1 }{}
> line: {: file2 }{}
> line: {: dir1 dir2 file1 file2 }{}
> line: {: *}{}
> +line: {: dir1/}{}
> Test ./Y01completion.ztst failed: output differs from expected as shown above for:
>  comptest $': *\t\t\t\t\t\t\t'
> Was testing: _expand shows file types
> ./Y01completion.ztst: test failed.
> 
> Can you reproduce this?
> Is it OK to replace "comptest $': *\t\t\t\t\t\t\t'" by "comptest $': *\t'"
> as in my patch in workers/48363⁩?

Yes, I can reproduce it. However, I don’t think we should discard all of the additional `\t`s, since it also tests the suffixes that _expand adds, which are different from _complete. I would like to suggest a slightly different fix. See attached patch.


[-- Attachment #2: 0001-Fix-_expand-completion-test.txt --]
[-- Type: text/plain, Size: 2398 bytes --]

From 032b6bd0b7c90e3bed9fbd7771ea1b6969b4c24f Mon Sep 17 00:00:00 2001
From: Marlon Richert <marlon.richert@gmail.com>
Date: Tue, 6 Apr 2021 14:29:26 +0300
Subject: [PATCH] Fix _expand completion test

---
 Test/Y01completion.ztst |  3 +--
 Test/comptest           | 11 ++++++-----
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/Test/Y01completion.ztst b/Test/Y01completion.ztst
index eff6910c2..858fa7220 100644
--- a/Test/Y01completion.ztst
+++ b/Test/Y01completion.ztst
@@ -44,7 +44,7 @@
 >line: {: dir1/}{}
 >line: {: dir2/}{}
 
-  comptest $': *\t\t\t\t\t\t\t'
+  comptest $': *\t\t\t\t\t\t'
 0:_expand shows file types
 >line: {: dir1/}{}
 >DESCRIPTION:{expansions}
@@ -56,7 +56,6 @@
 >NO:{dir1 dir2 file1 file2}
 >DESCRIPTION:{original}
 >NO:{*}
->line: {: dir1/}{}
 >line: {: dir2/}{}
 >line: {: file1 }{}
 >line: {: file2 }{}
diff --git a/Test/comptest b/Test/comptest
index a36e301e0..79c69979a 100644
--- a/Test/comptest
+++ b/Test/comptest
@@ -40,6 +40,7 @@ KEYTIMEOUT=1
 setopt zle
 autoload -U compinit
 compinit -u
+zstyle ":completion:*" completer _expand _complete _ignored
 zstyle ":completion:*:default" list-colors "no=<NO>" "fi=<FI>" "di=<DI>" "ln=<LN>" "pi=<PI>" "so=<SO>" "bd=<BD>" "cd=<CD>" "ex=<EX>" "mi=<MI>" "tc=<TC>" "sp=<SP>" "lc=<LC>" "ec=<EC>\n" "rc=<RC>"
 zstyle ":completion:*" group-name ""
 zstyle ":completion:*:messages" format "<MESSAGE>%d</MESSAGE>
@@ -50,9 +51,9 @@ zstyle ":completion:*:options" verbose yes
 zstyle ":completion:*:values" verbose yes
 setopt noalwayslastprompt listrowsfirst completeinword
 zmodload zsh/complist
-expand-or-complete-with-report () {
-  print -lr "<WIDGET><expand-or-complete>"
-  zle expand-or-complete
+complete-word-with-report () {
+  print -lr "<WIDGET><complete-word>"
+  zle complete-word
   print -lr - "<LBUFFER>$LBUFFER</LBUFFER>" "<RBUFFER>$RBUFFER</RBUFFER>"
   zle clear-screen
   zle -R
@@ -80,11 +81,11 @@ zle-finish () {
   (( $+mark )) && print -lr "MARK: $mark"
   zle accept-line
 }
-zle -N expand-or-complete-with-report
+zle -N complete-word-with-report
 zle -N list-choices-with-report
 zle -N comp-finish
 zle -N zle-finish
-bindkey "^I" expand-or-complete-with-report
+bindkey "^I" complete-word-with-report
 bindkey "^D" list-choices-with-report
 bindkey "^Z" comp-finish
 bindkey "^X" zle-finish
-- 
2.31.0


[-- Attachment #3: Type: text/plain, Size: 2 bytes --]

Re: Y01 Test Failure on Arch

[Oliver Kiddle]

Marlon Richert wrote:
> My original patch included the necessary changes to `comptest` to make
> the tests work correctly. See

> Oliver, what was the reason you did not commit the changes below in 07a30613f?

I think that was a consequence of gmail word-wrapping the patch. I must
have have missed the malformed-patch error and because it only affected
the last file, it wasn't obvious that anything was missing.

But as Jun T points out, even with the patched comptest file, the test
still fails.

Oliver

Re: Y01 Test Failure on Arch

[Bart Schaefer]

(I'm on "vacation" this week so won't have as much feedback as I have lately.)

On Sun, Apr 4, 2021 at 7:01 AM Vin Shelton <acs@alumni.princeton.edu> wrote:
>
> Also, why was the test change necessary in the first place?

The test was not checking the correct thing.  expand-or-complete first
invokes the built-in expand-word and then falls back on completion,
but the patch updated the _expand completer (which was never reached
after expand-word produced a result).

Re: Y01 Test Failure on Arch

[Vin Shelton]

[-- Attachment #1: Type: text/plain, Size: 629 bytes --]

So it sounds like Jun's fix is the right one?

  - Vin

On Sun, Apr 4, 2021 at 2:27 PM Bart Schaefer <schaefer@brasslantern.com>
wrote:

> (I'm on "vacation" this week so won't have as much feedback as I have
> lately.)
>
> On Sun, Apr 4, 2021 at 7:01 AM Vin Shelton <acs@alumni.princeton.edu>
> wrote:
> >
> > Also, why was the test change necessary in the first place?
>
> The test was not checking the correct thing.  expand-or-complete first
> invokes the built-in expand-word and then falls back on completion,
> but the patch updated the _expand completer (which was never reached
> after expand-word produced a result).
>

[-- Attachment #2: Type: text/html, Size: 1317 bytes --]