[RFC,1/9] testcases: su: Fix userdel use

Message ID 20180423094639.14612-2-mylene.josserand@bootlin.com
State Rejected
Delegated to: Cyril Hrubis
Headers show
Series
  • testcases: small improvements with TCONF
Related show

Commit Message

Mylène Josserand April 23, 2018, 9:46 a.m.
In case "getent" command is not available in the system,
the userdel command will never be executed and for the
second use, the test will fail with:
     useradd: user 'su_usr1' already exists

Fix that by moving "userdel" command in a new line.

Signed-off-by: Mylène Josserand <mylene.josserand@bootlin.com>
---
 testcases/commands/su/su01 | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Cyril Hrubis April 23, 2018, 10:13 a.m. | #1
Hi!
> In case "getent" command is not available in the system,
> the userdel command will never be executed and for the
> second use, the test will fail with:
>      useradd: user 'su_usr1' already exists
> 
> Fix that by moving "userdel" command in a new line.
> 
> Signed-off-by: Myl??ne Josserand <mylene.josserand@bootlin.com>
> ---
>  testcases/commands/su/su01 | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/testcases/commands/su/su01 b/testcases/commands/su/su01
> index 34386d937..1eedcb636 100755
> --- a/testcases/commands/su/su01
> +++ b/testcases/commands/su/su01
> @@ -86,7 +86,8 @@ rm -f $TEST_ENV_FILE > /dev/null 2>&1
>  #Create 1st test user
>      #erase user if he may exist , so we can have a clean en
>          rm -rf /home/$TEST_USER1
> -        getent passwd $TEST_USER1 > /dev/null 2>&1 && userdel $TEST_USER1
> +        getent passwd $TEST_USER1 > /dev/null 2>&1
> +	userdel $TEST_USER1

I'm pretty sure this was intended to be:

"delete user only if it exists already"

So the whole getent line is useless after this change.

Also I suppose we shouldn't delete users in the test setup anyways, but
the whole tests is a mess and should be cleaned up...
Thomas Petazzoni April 23, 2018, 12:51 p.m. | #2
Hello,

On Mon, 23 Apr 2018 12:13:09 +0200, Cyril Hrubis wrote:

> > diff --git a/testcases/commands/su/su01 b/testcases/commands/su/su01
> > index 34386d937..1eedcb636 100755
> > --- a/testcases/commands/su/su01
> > +++ b/testcases/commands/su/su01
> > @@ -86,7 +86,8 @@ rm -f $TEST_ENV_FILE > /dev/null 2>&1
> >  #Create 1st test user
> >      #erase user if he may exist , so we can have a clean en
> >          rm -rf /home/$TEST_USER1
> > -        getent passwd $TEST_USER1 > /dev/null 2>&1 && userdel $TEST_USER1
> > +        getent passwd $TEST_USER1 > /dev/null 2>&1
> > +	userdel $TEST_USER1  
> 
> I'm pretty sure this was intended to be:
> 
> "delete user only if it exists already"
> 
> So the whole getent line is useless after this change.

Perhaps the getent line can be replaced with something like:

grep -q "^$TEST_USER1:" /etc/passwd && userdel $TEST_USER1

Best regards,

Thomas Petazzoni

Patch

diff --git a/testcases/commands/su/su01 b/testcases/commands/su/su01
index 34386d937..1eedcb636 100755
--- a/testcases/commands/su/su01
+++ b/testcases/commands/su/su01
@@ -86,7 +86,8 @@  rm -f $TEST_ENV_FILE > /dev/null 2>&1
 #Create 1st test user
     #erase user if he may exist , so we can have a clean en
         rm -rf /home/$TEST_USER1
-        getent passwd $TEST_USER1 > /dev/null 2>&1 && userdel $TEST_USER1
+        getent passwd $TEST_USER1 > /dev/null 2>&1
+	userdel $TEST_USER1
 	sleep 1
 
         useradd -m -g users $TEST_USER1
@@ -121,7 +122,8 @@  rm -f $TEST_ENV_FILE > /dev/null 2>&1
 #Create 2nd test user
 	#erase user if he may exist , so we can have a clean en
         rm -rf /home/$TEST_USER2
-        getent passwd $TEST_USER2 > /dev/null 2>&1 && userdel $TEST_USER2
+        getent passwd $TEST_USER2 > /dev/null 2>&1
+	userdel $TEST_USER2
 	sleep 1
 
         useradd -m -g users $TEST_USER2