[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH] tests: don't let a fail=1 env. setting induce unwarranted test f
From: |
Jim Meyering |
Subject: |
[PATCH] tests: don't let a fail=1 env. setting induce unwarranted test failure |
Date: |
Thu, 29 Oct 2009 16:14:19 +0100 |
In tests/ we often do things like this:
fail=0
...
command || fail=1
...
Exit $fail
But some of those scripts neglected the fail=0 initialization.
That bug would lead to numerous spurious failures with "fail=1"
in the environment (or skipped tests if fail=77).
This corrects all of them and adds a syntax-check rule
to ensure that no new instance is introduced.
BTW, I searched for instances in which "fail=1" preceded "fail=0",
but this found none:
git grep -l -E '\<fail=1$') \
| xargs -n1 perl -00 -lne '/fail=1.*fail=0/sm and print $ARGV'
so I didn't bother adding a check for that.
>From a1c1f576a937b2898417dce5de09be563ee5c2c3 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Thu, 29 Oct 2009 14:40:40 +0100
Subject: [PATCH] tests: don't let a fail=1 env. setting induce unwarranted test
failure
* cfg.mk (sc_fail_is_initialized): New rule.
Fix the offenders:
* tests/cp/acl: Set file=0
* tests/cp/backup-is-src: Likewise.
* tests/cp/file-perm-race: Likewise.
* tests/cp/reflink-auto: Likewise.
* tests/cp/same-file: Likewise.
* tests/ln/backup-1: Likewise.
* tests/misc/su-fail: Likewise.
* tests/misc/truncate-owned-by-other: Likewise.
* tests/mkdir/p-3: Likewise.
* tests/mkdir/selinux: Likewise.
* tests/mkdir/special-1: Likewise.
* tests/mv/acl: Likewise.
* tests/mv/backup-is-src: Likewise.
* tests/mv/diag: Likewise.
* tests/mv/force: Likewise.
* tests/mv/hard-link-1: Likewise.
* tests/mv/into-self-3: Likewise.
* tests/mv/sticky-to-xpart: Likewise.
* tests/touch/now-owned-by-other: Likewise.
---
cfg.mk | 11 +++++++++++
tests/cp/acl | 1 +
tests/cp/backup-is-src | 1 +
tests/cp/file-perm-race | 1 +
tests/cp/reflink-auto | 3 ++-
tests/cp/same-file | 2 +-
tests/ln/backup-1 | 1 +
tests/misc/su-fail | 2 ++
tests/misc/truncate-owned-by-other | 1 +
tests/mkdir/p-3 | 1 +
tests/mkdir/selinux | 1 +
tests/mkdir/special-1 | 1 +
tests/mv/acl | 1 +
tests/mv/backup-is-src | 1 +
tests/mv/diag | 1 +
tests/mv/force | 1 +
tests/mv/hard-link-1 | 1 +
tests/mv/into-self-3 | 1 +
tests/mv/sticky-to-xpart | 2 ++
tests/touch/now-owned-by-other | 1 +
20 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/cfg.mk b/cfg.mk
index 1e5108b..c3a7afe 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -230,4 +230,15 @@ sc_prohibit_emacs__indent_tabs_mode__setting:
msg='use of emacs indent-tabs-mode: setting' \
$(_prohibit_regexp)
+# Ensure that each file that contains fail=1 also contains fail=0.
+# Otherwise, setting file=1 in the environment would make tests fail
unexpectedly.
+sc_fail_is_initialized:
+ @files=$$(grep -l -E '\<fail=1$$' $$($(VC_LIST_EXCEPT))); \
+ if test "$$?" = 0; then \
+ grep -LE '\<fail=0$$' $$files | grep . && \
+ { echo '$(ME): the above files do not set fail=0' \
+ 1>&2; exit 1; } || :; \
+ else :; \
+ fi
+
include $(srcdir)/dist-check.mk
diff --git a/tests/cp/acl b/tests/cp/acl
index 2f87428..059a1e1 100755
--- a/tests/cp/acl
+++ b/tests/cp/acl
@@ -45,6 +45,7 @@ acl1=`cd a && getfacl file | grep -v ':bin:' | grep -v
'mask::'` \
test $skip = yes &&
skip_test_ "'.' is not on a suitable file system for this test"
+fail=0
# copy a file without preserving permissions
cp a/file b/ || fail=1
diff --git a/tests/cp/backup-is-src b/tests/cp/backup-is-src
index bf03e59..f226382 100755
--- a/tests/cp/backup-is-src
+++ b/tests/cp/backup-is-src
@@ -26,6 +26,7 @@ fi
echo a > a || framework_failure
echo a-tilde > a~ || framework_failure
+fail=0
# This cp command should exit nonzero.
cp --b=simple a~ a > out 2>&1 && fail=1
diff --git a/tests/cp/file-perm-race b/tests/cp/file-perm-race
index 1ff2746..8ae7cb0 100755
--- a/tests/cp/file-perm-race
+++ b/tests/cp/file-perm-race
@@ -23,6 +23,7 @@ fi
. $srcdir/test-lib.sh
+fail=0
umask 022
mkfifo fifo ||
skip_test_ "fifos not supported"
diff --git a/tests/cp/reflink-auto b/tests/cp/reflink-auto
index d1f6b2b..5e39b72 100755
--- a/tests/cp/reflink-auto
+++ b/tests/cp/reflink-auto
@@ -28,8 +28,9 @@ cleanup_() { rm -rf "$other_partition_tmpdir"; }
a_other="$other_partition_tmpdir/a"
rm -f "$a_other" || framework_failure
-echo non_zero_size > "$a_other"
+echo non_zero_size > "$a_other" || framework_failure
+fail=0
# we shouldn't be able to reflink() files on separate partitions
cp --reflink "$a_other" b && fail=1
diff --git a/tests/cp/same-file b/tests/cp/same-file
index 6d57ebd..d5deb68 100755
--- a/tests/cp/same-file
+++ b/tests/cp/same-file
@@ -220,7 +220,7 @@ cat <<\EOF | sed "$remove_these_sed" > $expected
EOF
-fail=0;
+fail=0
compare $expected $actual 1>&2 || fail=1
diff --git a/tests/ln/backup-1 b/tests/ln/backup-1
index cc3e10e..68c9ca2 100755
--- a/tests/ln/backup-1
+++ b/tests/ln/backup-1
@@ -28,6 +28,7 @@ fi
touch a b || framework_failure
+fail=0
ln b b~ || fail=1
ln -f --b=simple a b || fail=1
diff --git a/tests/misc/su-fail b/tests/misc/su-fail
index bba7d1f..f00edc9 100755
--- a/tests/misc/su-fail
+++ b/tests/misc/su-fail
@@ -24,6 +24,8 @@ if test "$VERBOSE" = yes; then
su --version
fi
+fail=0
+
# Very little that we can test without a root password
su --- / true # unknown option
test $? = 125 || fail=1
diff --git a/tests/misc/truncate-owned-by-other
b/tests/misc/truncate-owned-by-other
index c7bdf61..61cbe8b 100755
--- a/tests/misc/truncate-owned-by-other
+++ b/tests/misc/truncate-owned-by-other
@@ -35,6 +35,7 @@ chmod g+w root-owned
# Ensure that the current directory is searchable by $NON_ROOT_USERNAME.
chmod g+x .
+fail=0
setuidgid $NON_ROOT_USERNAME env PATH="$PATH" truncate -s0 root-owned || fail=1
Exit $fail
diff --git a/tests/mkdir/p-3 b/tests/mkdir/p-3
index df1c56d..06207b2 100755
--- a/tests/mkdir/p-3
+++ b/tests/mkdir/p-3
@@ -30,6 +30,7 @@ mkdir no-access || framework_failure
mkdir no-acce2s || framework_failure
mkdir -p no-acce3s/d || framework_failure
+fail=0
p=`pwd`
(cd no-access && chmod 0 . && mkdir -p "$p/a/b" u/v) 2> /dev/null && fail=1
test -d "$p/a/b" || fail=1
diff --git a/tests/mkdir/selinux b/tests/mkdir/selinux
index ddd237c..d872cb6 100755
--- a/tests/mkdir/selinux
+++ b/tests/mkdir/selinux
@@ -33,6 +33,7 @@ require_selinux_enforcing_
c=invalid-selinux-context
msg="failed to set default file creation context to \`$c':"
+fail=0
# Test each of mkdir, mknod, mkfifo with "-Z invalid-context".
for cmd_w_arg in 'mkdir dir' 'mknod b p' 'mkfifo f'; do
diff --git a/tests/mkdir/special-1 b/tests/mkdir/special-1
index 4956c20..d1af121 100755
--- a/tests/mkdir/special-1
+++ b/tests/mkdir/special-1
@@ -26,6 +26,7 @@ fi
set_mode_string=u=rwx,g=rx,o=w,-s,+t
output_mode_string=drwxr-x-wT
+fail=0
tmp=t
mkdir -m$set_mode_string $tmp || fail=1
diff --git a/tests/mv/acl b/tests/mv/acl
index 5ad8de0..e9fb626 100755
--- a/tests/mv/acl
+++ b/tests/mv/acl
@@ -48,6 +48,7 @@ acl1=`getfacl file` || skip_partition=.
test $skip_partition != none &&
skip_test_ "'$skip' is not on a suitable file system for this test"
+fail=0
# move the access acl of a file
mv file "$other_partition_tmpdir" || fail=1
acl2=`cd "$other_partition_tmpdir" && getfacl file` || framework_failure
diff --git a/tests/mv/backup-is-src b/tests/mv/backup-is-src
index df6561b..7d80078 100755
--- a/tests/mv/backup-is-src
+++ b/tests/mv/backup-is-src
@@ -32,6 +32,7 @@ rm -f "$a" "$a2" || framework_failure
echo a > "$a" || framework_failure
echo a2 > "$a2" || framework_failure
+fail=0
# This mv command should exit nonzero.
mv --b=simple "$a2" "$a" > out 2>&1 && fail=1
diff --git a/tests/mv/diag b/tests/mv/diag
index 6e28fa4..75904d6 100755
--- a/tests/mv/diag
+++ b/tests/mv/diag
@@ -28,6 +28,7 @@ touch f1 || framework_failure
touch f2 || framework_failure
touch d || framework_failure
+fail=0
# These mv commands should all exit nonzero.
# Too few args. This first one did fail, but with an incorrect diagnostic
diff --git a/tests/mv/force b/tests/mv/force
index 88851ac..df43970 100755
--- a/tests/mv/force
+++ b/tests/mv/force
@@ -29,6 +29,7 @@ ff2=mvforce2
echo force-contents > $ff || framework_failure
ln $ff $ff2 || framework_failure
+fail=0
# This mv command should exit nonzero.
mv $ff $ff > out 2>&1 && fail=1
diff --git a/tests/mv/hard-link-1 b/tests/mv/hard-link-1
index 2df2cf3..d626f3c 100755
--- a/tests/mv/hard-link-1
+++ b/tests/mv/hard-link-1
@@ -32,6 +32,7 @@ mkdir $dir || framework_failure
> $dir/a || framework_failure
ln $dir/a $dir/b || framework_failure
+fail=0
mv $dir "$other_partition_tmpdir" || fail=1
# Display inode numbers, one per line.
diff --git a/tests/mv/into-self-3 b/tests/mv/into-self-3
index b9fa41a..e5a4f8d 100755
--- a/tests/mv/into-self-3
+++ b/tests/mv/into-self-3
@@ -28,6 +28,7 @@ dir2=is3-dir2
mkdir $dir1 $dir2 || framework_failure
+fail=0
# This mv command should exit nonzero.
mv $dir1 $dir2 $dir2 > out 2>&1 && fail=1
diff --git a/tests/mv/sticky-to-xpart b/tests/mv/sticky-to-xpart
index f0f9f95..f8855a2 100755
--- a/tests/mv/sticky-to-xpart
+++ b/tests/mv/sticky-to-xpart
@@ -43,6 +43,8 @@ chown "$NON_ROOT_USERNAME" "$other_partition_tmpdir" ||
framework_failure
# We have to allow $NON_ROOT_USERNAME access to ".".
chmod go+x . || framework_failure
+fail=0
+
# Ensure that $NON_ROOT_USERNAME can access the required version of mv.
version=`setuidgid $NON_ROOT_USERNAME env PATH="$PATH" mv --version|sed -n
'1s/.* //p'`
case $version in
diff --git a/tests/touch/now-owned-by-other b/tests/touch/now-owned-by-other
index e124a2e..4ce4ec6 100755
--- a/tests/touch/now-owned-by-other
+++ b/tests/touch/now-owned-by-other
@@ -34,6 +34,7 @@ chmod g+w root-owned
# Ensure that the current directory is searchable by $NON_ROOT_USERNAME.
chmod g+x .
+fail=0
setuidgid $NON_ROOT_USERNAME env PATH="$PATH" touch -d now root-owned || fail=1
Exit $fail
--
1.6.5.2.375.g164f1
- [PATCH] tests: don't let a fail=1 env. setting induce unwarranted test failure,
Jim Meyering <=