Re: Various tests give illusory results

[Jim Meyering]

Jim Meyering wrote:
> Ondrej Oprala wrote:
>> here's the new patch...I hope it's all alright now.
>> And sorry about not mentioning Bernhard in the first place,
>> it just didn't occur to me.
> ...
>> Subject: [PATCH] tests: Add error checking for root-only tests running
>>  setuidgid
>>
>> * NEWS: Mention the test improvement.
>> * tests/init.cfg: Modify the require_root_ function to check
>> for setuidgid calls and proper permissions.
>>
>> Improved-by: Bernhard Voelker
> ...
>> +** Improved tests
>> +
>> +  root-only tests now properly check for permissions of dummy
>> +  user $NON_ROOT_USERNAME before trying to run binaries from the
>> +  src dir.
> ...
>> +setuidgid_has_perm_()
>> +{
>> +  local rm_version=$(
>> +    setuidgid $NON_ROOT_USERNAME env PATH="$PATH" rm --version |
>> +    sed -n 'ls/.* //p'
>> +  )
>> +  case "_${rm_version}_" in
>> +      _${PACKAGE_VERSION}_) ;;
>> +      *) return 1;;
>> +  esac
>> +}
>
> Thanks.
> I've squashed in two minor changes, and will push the result in a few hours.
> The latter switches from use of "_" to ":" to avoid the need for braces.

Somehow I never pushed that.  Sorry.

I've rebased, adjusted NEWS, adjusted for the new location of init.cfg,
and wrote comments for the new code in that file.
I'll push this once you ACK it:

>From a6750ce3c571aa6738ca47b98e50068bc663ea2e Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Thu, 6 Sep 2012 12:00:16 +0200
Subject: [PATCH] tests: improve checks for setuidgid-using root-only tests

* NEWS: Mention the test improvement.
* init.cfg (setuidgid_has_perm_): New function.
(require_root_): Use it.
Improved-by: Bernhard Voelker
---
 NEWS     |  6 ++++++
 init.cfg | 20 ++++++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/NEWS b/NEWS
index 63fa042..6941721 100644
--- a/NEWS
+++ b/NEWS
@@ -25,6 +25,12 @@ GNU coreutils NEWS                                    -*- 
outline -*-

 ** Improvements

+  root-only tests now check for permissions of our dummy user,
+  $NON_ROOT_USERNAME, before trying to run binaries from the build directory.
+  Before, we would get hard-to-diagnose reports of failing root-only tests.
+  Now, those tests are skipped with a useful diagnostic when the root tests
+  are run without following the instructions in README.
+
   stat and tail work better with ZFS and VZFS.  stat -f --format=%T now
   reports the file system type, and tail -f now uses inotify for files
   on those file systems, rather than the default (for unknown file system
diff --git a/init.cfg b/init.cfg
index 13cac74..304b918 100644
--- a/init.cfg
+++ b/init.cfg
@@ -346,11 +346,31 @@ or use the shortcut target of the toplevel Makefile,
   fi
 }

+# Test whether we can run our just-built rm setuidgid-to-root,
+# i.e., that $NON_ROOT_USERNAME has access to the build directory.
+setuidgid_has_perm_()
+{
+  local rm_version=$(
+    setuidgid $NON_ROOT_USERNAME env PATH="$PATH" rm --version |
+    sed -n 'ls/.* //p'
+  )
+  case ":$rm_version:" in
+      :$PACKAGE_VERSION:) ;;
+      *) return 1;;
+  esac
+}
+
 require_root_()
 {
   uid_is_privileged_ || skip_ "must be run as root"
   NON_ROOT_USERNAME=${NON_ROOT_USERNAME=nobody}
   NON_ROOT_GROUP=${NON_ROOT_GROUP=$(id -g $NON_ROOT_USERNAME)}
+
+  # When the current test invokes setuidgid, call setuidgid_has_perm_
+  # to check for a common problem.
+  grep '^[ ]*setuidgid' "../$0" \
+    && { setuidgid_has_perm_ \
+           || skip_ "user $NON_ROOT_USERNAME lacks execute permissions"; }
 }

 skip_if_root_() { uid_is_privileged_ && skip_ "must be run as non-root"; }
--
1.7.12.176.g3fc0e4c