On 01/03/2020 03:44, Dmitry V. Levin wrote:
On Sat, Feb 29, 2020 at 09:07:10PM +0000, Pádraig Brady wrote:
[...]
diff --git a/tests/ls/stat-free-symlinks.sh b/tests/ls/stat-free-symlinks.sh
index c538a1c51..a786145db 100755
--- a/tests/ls/stat-free-symlinks.sh
+++ b/tests/ls/stat-free-symlinks.sh
@@ -32,7 +32,7 @@ ln -s x link-to-x || framework_failure_
  # symlink and an executable file properly.
LS_COLORS='or=0:mi=0:ex=01;32:ln=01;35' \
-  strace -qe stat ls -F --color=always x link-to-x > out.tmp 2> err || fail
+  strace -qe stat ls -F --color=always x link-to-x > out.tmp 2> err || fail=1
  # Elide info messages strace can send to stdout of the form:
  #   [ Process PID=1234 runs in 32 bit mode. ]
  sed '/Process PID=/d' out.tmp > out

This caught my eye because "strace -e stat" is not portable:
while traditional architectures have stat syscall, more recent
architectures don't have it and the test will fail there.

Also this test is going to be missing the point on traditional
architectures as soon as their libc switch to newfstatat/fstatat64/statx.

That's a good point.
tests/ls/stat-free-color.sh did check for other stat calls,
but missed out on statx() recently introduced to ls.
The attached should make both of these tests effective again.

thanks!
Pádraig
>From 14d9924124a519e963698315a6f93e61c47e7dab Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <p...@draigbrady.com>
Date: Sun, 1 Mar 2020 12:36:35 +0000
Subject: [PATCH] tests: improve test coverage for ls stat checks

* tests/ls/stat-free-color.sh: Check for the availability
of various stat calls individually, and add statx() and fstatat64()
to the list to check.
* tests/ls/stat-free-symlinks.sh: Check syscalls other than stat().
---
 tests/ls/stat-free-color.sh    | 24 +++++++++++----------
 tests/ls/stat-free-symlinks.sh | 39 +++++++++++++++++++++++++++-------
 2 files changed, 44 insertions(+), 19 deletions(-)

diff --git a/tests/ls/stat-free-color.sh b/tests/ls/stat-free-color.sh
index a1766aa5e..00942f774 100755
--- a/tests/ls/stat-free-color.sh
+++ b/tests/ls/stat-free-color.sh
@@ -18,15 +18,17 @@
 
 . "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
 print_ver_ ls
-
-# Note this list of _file name_ stat functions must be
-# as cross platform as possible and so doesn't include
-# fstatat64 as that's not available on aarch64 for example.
-stats='stat,lstat,stat64,lstat64,newfstatat'
-
-require_strace_ $stats
+require_strace_ stat
 require_dirent_d_type_
 
+stats='stat'
+# List of other _file name_ stat functions to increase coverage.
+other_stats='statx lstat stat64 lstat64 newfstatat fstatat64'
+for stat in $other_stats; do
+  strace -qe "$stat" true > /dev/null 2>&1 &&
+    stats="$stats,$stat"
+done
+
 # Disable enough features via LS_COLORS so that ls --color
 # can do its job without calling stat (other than the obligatory
 # one-call-per-command-line argument).
@@ -59,8 +61,8 @@ eval $(dircolors -b color-without-stat)
 # invocation under test.
 mkdir d || framework_failure_
 
-strace -o log1 -e $stats ls --color=always d || fail=1
-n_stat1=$(wc -l < log1) || framework_failure_
+strace -q -o log1 -e $stats ls --color=always d || fail=1
+n_stat1=$(grep -vF '+++' log1 | wc -l) || framework_failure_
 
 test $n_stat1 = 0 \
   && skip_ 'No stat calls recognized on this platform'
@@ -74,8 +76,8 @@ mkdir d/subdir \
   || framework_failure_
 
 # Invocation under test.
-strace -o log2 -e $stats ls --color=always d || fail=1
-n_stat2=$(wc -l < log2) || framework_failure_
+strace -q -o log2 -e $stats ls --color=always d || fail=1
+n_stat2=$(grep -vF '+++' log2 | wc -l) || framework_failure_
 
 # Expect the same number of stat calls.
 test $n_stat1 = $n_stat2 \
diff --git a/tests/ls/stat-free-symlinks.sh b/tests/ls/stat-free-symlinks.sh
index a786145db..80e5a357a 100755
--- a/tests/ls/stat-free-symlinks.sh
+++ b/tests/ls/stat-free-symlinks.sh
@@ -20,6 +20,28 @@
 print_ver_ ls
 require_strace_ stat
 
+stats='stat'
+# List of other _file name_ stat functions to increase coverage.
+other_stats='statx lstat stat64 lstat64 newfstatat fstatat64'
+for stat in $other_stats; do
+  strace -qe "$stat" true > /dev/null 2>&1 &&
+    stats="$stats,$stat"
+done
+
+# The system may perform additional stat-like calls before main.
+# Furthermore, underlying library functions may also implicitly
+# add an extra stat call, e.g. opendir since glibc-2.21-360-g46f894d.
+# To avoid counting those, first get a baseline count for running
+# ls with one empty directory argument.  Then, compare that with the
+# invocation under test.
+mkdir d || framework_failure_
+strace -q -o log1 -e $stats ls -F --color=always d || fail=1
+n_stat1=$(grep -vF '+++' log1 | wc -l) || framework_failure_
+
+test $n_stat1 = 0 \
+  && skip_ 'No stat calls recognized on this platform'
+
+
 touch x || framework_failure_
 chmod a+x x || framework_failure_
 ln -s x link-to-x || framework_failure_
@@ -32,21 +54,22 @@ ln -s x link-to-x || framework_failure_
 # symlink and an executable file properly.
 
 LS_COLORS='or=0:mi=0:ex=01;32:ln=01;35' \
-  strace -qe stat ls -F --color=always x link-to-x > out.tmp 2> err || fail=1
-# Elide info messages strace can send to stdout of the form:
-#   [ Process PID=1234 runs in 32 bit mode. ]
-sed '/Process PID=/d' out.tmp > out
+  strace -qe $stats -o log2 ls -F --color=always x link-to-x > out.tmp || fail=1
+n_stat2=$(grep -vF '+++' log2 | wc -l) || framework_failure_
 
-# With coreutils 6.9 and earlier, this file would contain a
-# line showing ls had called stat on "x".
-grep '^stat("x"' err && fail=1
+# Expect one more stat call,
+# which failed with coreutils 6.9 and earlier, which had 2.
+test $n_stat1 = $(($n_stat2 - 1)) \
+  || { fail=1; head -n30 log*; }
 
 # Check that output is colorized, as requested, too.
 {
   printf '\033[0m\033[01;35mlink-to-x\033[0m@\n'
   printf '\033[01;32mx\033[0m*\n'
 } > exp || fail=1
-
+# Elide info messages strace can send to stdout of the form:
+#   [ Process PID=1234 runs in 32 bit mode. ]
+sed '/Process PID=/d' out.tmp > out
 compare exp out || fail=1
 
 Exit $fail
-- 
2.24.1

Reply via email to