Message ID | 20200604095727.21720-1-yosun@suse.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [v2] fstests: modify user name beginning with non-digit | expand |
Hi Sero,
Reviewed-by: Petr Vorel <pvorel@suse.cz>
Kind regards,
Petr
On 6/4/20 4:57 AM, Yong Sun wrote: > openSUSE and SLE don't support username begin with digit, so it will > skip test generic/597 and generic/598 by lack of 123456-fsgqa user. > generic/597 and 598 are not test username begin with digit on purpose > (different with generic/381). It's will be helpful to use an username > begin with non-digit in this case. > > Signed-off-by: Sun Yong <yosun@suse.com> > --- > README | 1 + > tests/generic/597 | 7 ++----- > tests/generic/598 | 7 ++----- > 3 files changed, 5 insertions(+), 10 deletions(-) > > diff --git a/README b/README > index 094a7742..cffa0bc6 100644 > --- a/README > +++ b/README > @@ -22,6 +22,7 @@ _______________________ > - create fsgqa test user ("sudo useradd -m fsgqa") > - create fsgqa group ("sudo groupadd fsgqa") > - create 123456-fsgqa test user ("sudo useradd 123456-fsgqa") > +- create fsgqa-123456 test user ("sudo useradd fsgqa-123456") > > ______________________ > USING THE FSQA SUITE > diff --git a/tests/generic/597 b/tests/generic/597 > index 1d87a23a..11846b95 100755 > --- a/tests/generic/597 > +++ b/tests/generic/597 > @@ -41,13 +41,10 @@ _supported_os Linux > _require_test > _require_sysctl_variable fs.protected_symlinks > _require_sysctl_variable fs.protected_hardlinks > -# su in _require_user prints warnings about user name starts with a digit, > -# discard the warning > -_require_user 123456-fsgqa >/dev/null 2>&1 > -# Do this SECOND so that qa_user is fsgqa, and _user_do uses that account This comment is still relevant. It's a quirk that _user_do uses the second _require_user stated[1], and I wanted to highlight that in the comment. Otherwise, TBH I guess I'd rather see the user simply be "fsgqa2" or something, so it's not easily confused with the 123456-fsgqa user. Not a technical problem with this patch though, just a preference. Alternately, these two tests /might/ be able to run just as well with "root" as one user or the other, but I didn't want to risk accidentally getting the permissions tests wrong by using a user with elevated privs.... so I'm not sure about that option. -Eric [1] # check for a user on the machine, fsgqa as default # _require_user() { qa_user=fsgqa if [ -n "$1" ];then qa_user=$1 fi ... _user_do() { echo $1 | su -s /bin/bash $qa_user 2>&1 | _filter_user_do } tho it might be better to enhance _user_do to take a username as an optional (or required) argument, to avoid this complexity/confusion.... that's a different patch tho. > +_require_user fsgqa-123456 > _require_user fsgqa > > -OWNER=123456-fsgqa > +OWNER=fsgqa-123456 > OTHER=fsgqa
Eric Sandeen <sandeen@sandeen.net> 于2020年6月4日周四 下午9:56写道: > > On 6/4/20 4:57 AM, Yong Sun wrote: > > openSUSE and SLE don't support username begin with digit, so it will > > skip test generic/597 and generic/598 by lack of 123456-fsgqa user. > > generic/597 and 598 are not test username begin with digit on purpose > > (different with generic/381). It's will be helpful to use an username > > begin with non-digit in this case. > > > > Signed-off-by: Sun Yong <yosun@suse.com> > > --- > > README | 1 + > > tests/generic/597 | 7 ++----- > > tests/generic/598 | 7 ++----- > > 3 files changed, 5 insertions(+), 10 deletions(-) > > > > diff --git a/README b/README > > index 094a7742..cffa0bc6 100644 > > --- a/README > > +++ b/README > > @@ -22,6 +22,7 @@ _______________________ > > - create fsgqa test user ("sudo useradd -m fsgqa") > > - create fsgqa group ("sudo groupadd fsgqa") > > - create 123456-fsgqa test user ("sudo useradd 123456-fsgqa") > > +- create fsgqa-123456 test user ("sudo useradd fsgqa-123456") > > > > ______________________ > > USING THE FSQA SUITE > > diff --git a/tests/generic/597 b/tests/generic/597 > > index 1d87a23a..11846b95 100755 > > --- a/tests/generic/597 > > +++ b/tests/generic/597 > > @@ -41,13 +41,10 @@ _supported_os Linux > > _require_test > > _require_sysctl_variable fs.protected_symlinks > > _require_sysctl_variable fs.protected_hardlinks > > -# su in _require_user prints warnings about user name starts with a digit, > > -# discard the warning > > -_require_user 123456-fsgqa >/dev/null 2>&1 > > -# Do this SECOND so that qa_user is fsgqa, and _user_do uses that account > > This comment is still relevant. It's a quirk that _user_do uses the second > _require_user stated[1], and I wanted to highlight that in the comment. > I see, I'll left this comments in v3. > Otherwise, TBH I guess I'd rather see the user simply be "fsgqa2" or something, > so it's not easily confused with the 123456-fsgqa user. Not a technical problem > with this patch though, just a preference. > > Alternately, these two tests /might/ be able to run just as well with "root" > as one user or the other, but I didn't want to risk accidentally getting the > permissions tests wrong by using a user with elevated privs.... so I'm not > sure about that option. > IMO fsgqa2 is better than root. root may misleading test motivation. Also It's will be more convenience to choose when people need a second user in other tests. > -Eric > > [1] > # check for a user on the machine, fsgqa as default > # > _require_user() > { > qa_user=fsgqa > if [ -n "$1" ];then > qa_user=$1 > fi > > ... > > _user_do() > { > echo $1 | su -s /bin/bash $qa_user 2>&1 | _filter_user_do > } > > tho it might be better to enhance _user_do to take a username as an optional > (or required) argument, to avoid this complexity/confusion.... that's a different > patch tho. +1, this may need a different patch. Thanks, Sero > > > +_require_user fsgqa-123456 > > _require_user fsgqa > > > > -OWNER=123456-fsgqa > > +OWNER=fsgqa-123456 > > OTHER=fsgqa
diff --git a/README b/README index 094a7742..cffa0bc6 100644 --- a/README +++ b/README @@ -22,6 +22,7 @@ _______________________ - create fsgqa test user ("sudo useradd -m fsgqa") - create fsgqa group ("sudo groupadd fsgqa") - create 123456-fsgqa test user ("sudo useradd 123456-fsgqa") +- create fsgqa-123456 test user ("sudo useradd fsgqa-123456") ______________________ USING THE FSQA SUITE diff --git a/tests/generic/597 b/tests/generic/597 index 1d87a23a..11846b95 100755 --- a/tests/generic/597 +++ b/tests/generic/597 @@ -41,13 +41,10 @@ _supported_os Linux _require_test _require_sysctl_variable fs.protected_symlinks _require_sysctl_variable fs.protected_hardlinks -# su in _require_user prints warnings about user name starts with a digit, -# discard the warning -_require_user 123456-fsgqa >/dev/null 2>&1 -# Do this SECOND so that qa_user is fsgqa, and _user_do uses that account +_require_user fsgqa-123456 _require_user fsgqa -OWNER=123456-fsgqa +OWNER=fsgqa-123456 OTHER=fsgqa # Save current system state to reset when done diff --git a/tests/generic/598 b/tests/generic/598 index 998b62cf..b6a1cb3a 100755 --- a/tests/generic/598 +++ b/tests/generic/598 @@ -41,13 +41,10 @@ _supported_os Linux _require_test _require_sysctl_variable fs.protected_regular _require_sysctl_variable fs.protected_fifos -# su in _require_user prints warnings about user name starts with a digit, -# discard the warning -_require_user 123456-fsgqa >/dev/null 2>&1 -# Do this SECOND so that qa_user is fsgqa, and _user_do uses that account +_require_user fsgqa-123456 _require_user fsgqa -USER1=123456-fsgqa +USER1=fsgqa-123456 USER2=fsgqa # Save current system state to reset when done
openSUSE and SLE don't support username begin with digit, so it will skip test generic/597 and generic/598 by lack of 123456-fsgqa user. generic/597 and 598 are not test username begin with digit on purpose (different with generic/381). It's will be helpful to use an username begin with non-digit in this case. Signed-off-by: Sun Yong <yosun@suse.com> --- README | 1 + tests/generic/597 | 7 ++----- tests/generic/598 | 7 ++----- 3 files changed, 5 insertions(+), 10 deletions(-)