Re: [PATCH] ls: add a test to verify that '+' is added

[Pádraig Brady]

On 10/01/2024 21:10, Sylvestre Ledru wrote:
> Le 10/01/2024 à 21:41, Pádraig Brady a écrit :
> > On 10/01/2024 18:21, Sylvestre Ledru wrote:
> > > Hello
> > >
> > > AFAIK, there isn't a test in ls to verify that using setfactl on
> > > directory shows a "+".
> > >
> > > This patch should cover this.
> >
> > That's worthwhile adding.
> > It may need more guards though.
> > I'd be inclined to check getfacl output first before testing with ls.
> > I.e. use the guards from tests/cp/acl.sh
> >
> > This new test will also need to be referenced in tests/local.mk
> >
> > Also the summary should start with "tests:"
> Thanks!
>
> Here it is!

Cool.
I rolled in the following extra guards and pushed.

thanks,
Pádraig.

diff --git a/tests/ls/acl.sh b/tests/ls/acl.sh
index fb7fcd3c1..7911f858f 100644
--- a/tests/ls/acl.sh
+++ b/tests/ls/acl.sh
@@ -1,7 +1,7 @@
 #!/bin/sh
 # verify that ls -al with acl display the "+"

-# Copyright (C) 2011-2024 Free Software Foundation, Inc.
+# Copyright (C) 2024 Free Software Foundation, Inc.

 # This program is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -19,14 +19,16 @@
 . "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
 print_ver_ ls

-require_setfacl_
+require_acl_
+
+# Skip this test if ls was built without ACL support:
+grep '^#define USE_ACL 1' $CONFIG_HEADER > /dev/null ||
+  skip_ "insufficient ACL support"

 mkdir k || framework_failure_
-skip=no
-getfacl k  2> /dev/null || skip=yes
-setfacl -d -m user::rwx k 2> /dev/null || skip=yes
-test $skip = yes &&
-  skip_ "'.' is not on a suitable file system for this test"
+
+setfacl -d -m user::rwx k 2> /dev/null || framework_failure_
+getfacl k | grep 'default:' || skip_ 'default ACL not set'

 ls_l=$(ls -ld k) || fail=1


Actually I also did this to avoid any issues with umask etc.

diff --git a/tests/ls/acl.sh b/tests/ls/acl.sh
index e2e890c5b..04213c780 100755
--- a/tests/ls/acl.sh
+++ b/tests/ls/acl.sh
@@ -33,7 +33,7 @@ getfacl k | grep 'default:' || skip_ 'default ACL not set'
 ls_l=$(ls -ld k) || fail=1

 case $ls_l in
-  drwx*+*) ;;
+  d[rwxsStT-]*+\ *) ;;
   *) fail=1; ;;
 esac


Also `make syntax-check` caught that the script itself wasn't executable.