* [LTP] Add TST_USES_MODULE and tst_test_root
@ 2019-10-09 6:16 Joerg Vehlow
2019-10-09 6:16 ` [LTP] [PATCH 1/2] tst_test.sh: Add TST_USES_MODULE Joerg Vehlow
2019-10-09 6:16 ` [LTP] [PATCH 2/2] tst_test.sh: Add public tst_test_root command Joerg Vehlow
0 siblings, 2 replies; 16+ messages in thread
From: Joerg Vehlow @ 2019-10-09 6:16 UTC (permalink / raw)
To: ltp
In preparation of lsmod01.sh patch v4.
The test requires a module only, if no modules are loaded and
in order to load the module, root is required.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [LTP] [PATCH 1/2] tst_test.sh: Add TST_USES_MODULE
2019-10-09 6:16 [LTP] Add TST_USES_MODULE and tst_test_root Joerg Vehlow
@ 2019-10-09 6:16 ` Joerg Vehlow
2019-10-09 7:36 ` Petr Vorel
2019-10-09 6:16 ` [LTP] [PATCH 2/2] tst_test.sh: Add public tst_test_root command Joerg Vehlow
1 sibling, 1 reply; 16+ messages in thread
From: Joerg Vehlow @ 2019-10-09 6:16 UTC (permalink / raw)
To: ltp
From: Joerg Vehlow <joerg.vehlow@aox-tech.de>
Adds a new library variable TST_USES_MODULE, that can be used, when a
test may need a module, but should not fail, if the module is not available.
---
doc/test-writing-guidelines.txt | 4 ++-
testcases/lib/tst_test.sh | 50 ++++++++++++++++++++++-----------
2 files changed, 36 insertions(+), 18 deletions(-)
diff --git a/doc/test-writing-guidelines.txt b/doc/test-writing-guidelines.txt
index cd0d28b8e..4a0652a8d 100644
--- a/doc/test-writing-guidelines.txt
+++ b/doc/test-writing-guidelines.txt
@@ -2125,6 +2125,8 @@ simply by setting right '$TST_NEEDS_FOO'.
| 'TST_NEEDS_CMDS' | String with command names that has to be present for
the test (see below).
| 'TST_NEEDS_MODULE' | Test module name needed for the test (see below).
+| 'TST_USES_MODULE' | Same as TST_NEEDS_MODULE, except that a missing module
+| | is not an error.
| 'TST_NEEDS_DRIVERS'| Checks kernel drivers support for the test.
|=============================================================================
@@ -2174,7 +2176,7 @@ Locating kernel modules
+++++++++++++++++++++++
The LTP build system can build kernel modules as well, setting
-'$TST_NEEDS_MODULE' to module name will cause to library to look for the
+'$TST_NEEDS_MODULE' to module name will cause the library to look for the
module in a few possible paths.
If module was found the path to it will be stored into '$TST_MODPATH'
diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
index e0b24c6b9..c70a5abbe 100644
--- a/testcases/lib/tst_test.sh
+++ b/testcases/lib/tst_test.sh
@@ -396,6 +396,32 @@ _tst_require_root()
fi
}
+_tst_find_module()
+{
+ local _tst_module=$1
+ local _tst_is_required=${2:-0}
+
+ for tst_module in "$_tst_module" \
+ "$LTPROOT/testcases/bin/$_tst_module" \
+ "$TST_STARTWD/$_tst_module"; do
+
+ if [ -f "$tst_module" ]; then
+ TST_MODPATH="$tst_module"
+ break
+ fi
+ done
+
+ if [ -z "$TST_MODPATH" ]; then
+ if [ $_tst_is_required -eq 1 ]; then
+ tst_brk TCONF "Failed to find module '$_tst_module'"
+ else
+ tst_res TINFO "Module '$_tst_module' not found."
+ fi
+ else
+ tst_res TINFO "Found module at '$TST_MODPATH'"
+ fi
+}
+
tst_run()
{
local _tst_i
@@ -410,7 +436,7 @@ tst_run()
SETUP|CLEANUP|TESTFUNC|ID|CNT|MIN_KVER);;
OPTS|USAGE|PARSE_ARGS|POS_ARGS);;
NEEDS_ROOT|NEEDS_TMPDIR|TMPDIR|NEEDS_DEVICE|DEVICE);;
- NEEDS_CMDS|NEEDS_MODULE|MODPATH|DATAROOT);;
+ NEEDS_CMDS|NEEDS_MODULE|USES_MODULE|MODPATH|DATAROOT);;
NEEDS_DRIVERS|FS_TYPE|MNTPOINT|MNT_PARAMS);;
IPV6|IPVER|TEST_DATA|TEST_DATA_IFS);;
RETRY_FUNC|RETRY_FN_EXP_BACKOFF);;
@@ -487,22 +513,12 @@ tst_run()
TST_DEVICE_FLAG=1
fi
- if [ -n "$TST_NEEDS_MODULE" ]; then
- for tst_module in "$TST_NEEDS_MODULE" \
- "$LTPROOT/testcases/bin/$TST_NEEDS_MODULE" \
- "$TST_STARTWD/$TST_NEEDS_MODULE"; do
-
- if [ -f "$tst_module" ]; then
- TST_MODPATH="$tst_module"
- break
- fi
- done
-
- if [ -z "$TST_MODPATH" ]; then
- tst_brk TCONF "Failed to find module '$TST_NEEDS_MODULE'"
- else
- tst_res TINFO "Found module at '$TST_MODPATH'"
- fi
+ if [ -n "$TST_NEEDS_MODULE" ] && [ -n "$TST_USES_MODULE" ]; then
+ tst_brk TBROK "Setting TST_NEEDS_MODULE and TST_USES_MODULE at the same time is not allowed"
+ elif [ -n "$TST_NEEDS_MODULE" ]; then
+ _tst_find_module "$TST_NEEDS_MODULE" 1
+ elif [ -n "$TST_USES_MODULE" ]; then
+ _tst_find_module "$TST_USES_MODULE" 0
fi
if [ -n "$TST_SETUP" ]; then
--
2.20.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [LTP] [PATCH 2/2] tst_test.sh: Add public tst_test_root command
2019-10-09 6:16 [LTP] Add TST_USES_MODULE and tst_test_root Joerg Vehlow
2019-10-09 6:16 ` [LTP] [PATCH 1/2] tst_test.sh: Add TST_USES_MODULE Joerg Vehlow
@ 2019-10-09 6:16 ` Joerg Vehlow
2019-10-09 6:52 ` Petr Vorel
2019-10-09 11:39 ` Cyril Hrubis
1 sibling, 2 replies; 16+ messages in thread
From: Joerg Vehlow @ 2019-10-09 6:16 UTC (permalink / raw)
To: ltp
From: Joerg Vehlow <joerg.vehlow@aox-tech.de>
If a test requires root only under certan circumstances, TST_NEEDS_ROOT
is not sufficient, because it always requires root.
---
testcases/lib/tst_test.sh | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
index c70a5abbe..07712670d 100644
--- a/testcases/lib/tst_test.sh
+++ b/testcases/lib/tst_test.sh
@@ -326,6 +326,13 @@ tst_check_cmds()
return 0
}
+tst_test_root()
+{
+ if [ "$(id -ru)" != 0 ]; then
+ tst_brk TCONF "Must be super/root for this test!"
+ fi
+}
+
tst_test_drivers()
{
[ $# -eq 0 ] && return 0
@@ -389,13 +396,6 @@ _tst_setup_timer()
_tst_setup_timer_pid=$!
}
-_tst_require_root()
-{
- if [ "$(id -ru)" != 0 ]; then
- tst_brk TCONF "Must be super/root for this test!"
- fi
-}
-
_tst_find_module()
{
local _tst_module=$1
@@ -469,7 +469,7 @@ tst_run()
tst_brk TBROK "Number of iterations (-i) must be > 0"
fi
- [ "$TST_NEEDS_ROOT" = 1 ] && _tst_require_root
+ [ "$TST_NEEDS_ROOT" = 1 ] && tst_test_root
[ "$TST_DISABLE_APPARMOR" = 1 ] && tst_disable_apparmor
[ "$TST_DISABLE_SELINUX" = 1 ] && tst_disable_selinux
--
2.20.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [LTP] [PATCH 2/2] tst_test.sh: Add public tst_test_root command
2019-10-09 6:16 ` [LTP] [PATCH 2/2] tst_test.sh: Add public tst_test_root command Joerg Vehlow
@ 2019-10-09 6:52 ` Petr Vorel
2019-10-09 6:57 ` Joerg Vehlow
2019-10-09 11:39 ` Cyril Hrubis
1 sibling, 1 reply; 16+ messages in thread
From: Petr Vorel @ 2019-10-09 6:52 UTC (permalink / raw)
To: ltp
Hi Joerg,
...
> testcases/lib/tst_test.sh | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
...
> -_tst_require_root()
> -{
> - if [ "$(id -ru)" != 0 ]; then
> - tst_brk TCONF "Must be super/root for this test!"
> - fi
> -}
You need to replace s/_tst_require_root/tst_test_root/ in other library files
(tst_net.sh, tst_security.sh).
...
Kind regards,
Petr
^ permalink raw reply [flat|nested] 16+ messages in thread
* [LTP] [PATCH 2/2] tst_test.sh: Add public tst_test_root command
2019-10-09 6:52 ` Petr Vorel
@ 2019-10-09 6:57 ` Joerg Vehlow
2019-10-09 7:53 ` Petr Vorel
0 siblings, 1 reply; 16+ messages in thread
From: Joerg Vehlow @ 2019-10-09 6:57 UTC (permalink / raw)
To: ltp
Hi,
> You need to replace s/_tst_require_root/tst_test_root/ in other library files
> (tst_net.sh, tst_security.sh).
Sorry, did not expect functions with _ to be used in other files.
I'll check it next time
J?rg
^ permalink raw reply [flat|nested] 16+ messages in thread
* [LTP] [PATCH 1/2] tst_test.sh: Add TST_USES_MODULE
2019-10-09 6:16 ` [LTP] [PATCH 1/2] tst_test.sh: Add TST_USES_MODULE Joerg Vehlow
@ 2019-10-09 7:36 ` Petr Vorel
2019-10-09 7:48 ` Joerg Vehlow
0 siblings, 1 reply; 16+ messages in thread
From: Petr Vorel @ 2019-10-09 7:36 UTC (permalink / raw)
To: ltp
Hi Joerg,
> From: Joerg Vehlow <joerg.vehlow@aox-tech.de>
> Adds a new library variable TST_USES_MODULE, that can be used, when a
> test may need a module, but should not fail, if the module is not available.
I wonder if TST_USES_MODULE is descriptive enough. But it looks to me better
than TST_GET_MODPATH (which Cyril suggested in v3).
We should think twice as _USES_ keyword should be used consistently for the same
approach in different functionality (i.e. TST_USES_FOO is the same as
TST_NEEDS_FOO, but not TCONF/TBROK if it fails).
But whole concept of TST_USES_FOO looks to me a bit complicated, if needed only
for modules. Cannot we just call _tst_find_module directly in this case and not
introduce variable?
...
> +++ b/doc/test-writing-guidelines.txt
> @@ -2125,6 +2125,8 @@ simply by setting right '$TST_NEEDS_FOO'.
> | 'TST_NEEDS_CMDS' | String with command names that has to be present for
> the test (see below).
> | 'TST_NEEDS_MODULE' | Test module name needed for the test (see below).
> +| 'TST_USES_MODULE' | Same as TST_NEEDS_MODULE, except that a missing module
> +| | is not an error.
> | 'TST_NEEDS_DRIVERS'| Checks kernel drivers support for the test.
> |=============================================================================
> @@ -2174,7 +2176,7 @@ Locating kernel modules
> +++++++++++++++++++++++
> The LTP build system can build kernel modules as well, setting
> -'$TST_NEEDS_MODULE' to module name will cause to library to look for the
> +'$TST_NEEDS_MODULE' to module name will cause the library to look for the
This is unrelated change, I merged it as a separate commit (c518ee8b9).
...
> +_tst_find_module()
> +{
> + local _tst_module=$1
> + local _tst_is_required=${2:-0}
> +
> + for tst_module in "$_tst_module" \
> + "$LTPROOT/testcases/bin/$_tst_module" \
> + "$TST_STARTWD/$_tst_module"; do
nit: (can be fixed by person who merges it): It's not visible, but uses more
tags than it should be, so it looks like:
+ for tst_module in "$_tst_module" \
+ "$LTPROOT/testcases/bin/$_tst_module" \
+ "$TST_STARTWD/$_tst_module"; do
+
+ if [ -f "$tst_module" ]; then
+ TST_MODPATH="$tst_module"
+ break
+ fi
I actually like the original alignment created by Alexey:
for tst_module in "$TST_NEEDS_MODULE" \
"$LTPROOT/testcases/bin/$TST_NEEDS_MODULE" \
"$TST_STARTWD/$TST_NEEDS_MODULE"; do
> +
> + if [ -f "$tst_module" ]; then
> + TST_MODPATH="$tst_module"
> + break
> + fi
> + done
> +
> + if [ -z "$TST_MODPATH" ]; then
> + if [ $_tst_is_required -eq 1 ]; then
> + tst_brk TCONF "Failed to find module '$_tst_module'"
> + else
> + tst_res TINFO "Module '$_tst_module' not found."
nit: please drop dot at the end (can be fixed by person who merges it).
> + fi
> + else
> + tst_res TINFO "Found module at '$TST_MODPATH'"
> + fi
nit: this is IMHO more readable
if [ -n "$TST_MODPATH" ]; then
tst_res TINFO "Found module at '$TST_MODPATH'"
return
fi
if [ $_tst_is_required -eq 1 ]; then
tst_brk TCONF "Failed to find module '$_tst_module'"
else
tst_res TINFO "Module '$_tst_module' not found"
fi
Kind regards,
Petr
^ permalink raw reply [flat|nested] 16+ messages in thread
* [LTP] [PATCH 1/2] tst_test.sh: Add TST_USES_MODULE
2019-10-09 7:36 ` Petr Vorel
@ 2019-10-09 7:48 ` Joerg Vehlow
0 siblings, 0 replies; 16+ messages in thread
From: Joerg Vehlow @ 2019-10-09 7:48 UTC (permalink / raw)
To: ltp
Hi,
>> Adds a new library variable TST_USES_MODULE, that can be used, when a
>> test may need a module, but should not fail, if the module is not available.
> I wonder if TST_USES_MODULE is descriptive enough. But it looks to me better
> than TST_GET_MODPATH (which Cyril suggested in v3).
>
> We should think twice as _USES_ keyword should be used consistently for the same
> approach in different functionality (i.e. TST_USES_FOO is the same as
> TST_NEEDS_FOO, but not TCONF/TBROK if it fails).
>
> But whole concept of TST_USES_FOO looks to me a bit complicated, if needed only
> for modules. Cannot we just call _tst_find_module directly in this case and not
> introduce variable?
I was thinking about adding a function to search for a module, but
struggled with
returning the name of the found found module.
I had something like
MODPATH=$(tst_find_module "$MODULE_NAME")
but this cannot use tst_res or tst_brk, which I don't like. I also don't
like just calling
_tst_find_module and getting the result in some "magic" variable. That
is the reason
why I went with Cyril's Idea of a variable
> ...
>> +_tst_find_module()
>> +{
>> + local _tst_module=$1
>> + local _tst_is_required=${2:-0}
>> +
>> + for tst_module in "$_tst_module" \
>> + "$LTPROOT/testcases/bin/$_tst_module" \
>> + "$TST_STARTWD/$_tst_module"; do
> nit: (can be fixed by person who merges it): It's not visible, but uses more
> tags than it should be, so it looks like:
> + for tst_module in "$_tst_module" \
> + "$LTPROOT/testcases/bin/$_tst_module" \
> + "$TST_STARTWD/$_tst_module"; do
> +
> + if [ -f "$tst_module" ]; then
> + TST_MODPATH="$tst_module"
> + break
> + fi
> I actually like the original alignment created by Alexey:
> for tst_module in "$TST_NEEDS_MODULE" \
> "$LTPROOT/testcases/bin/$TST_NEEDS_MODULE" \
> "$TST_STARTWD/$TST_NEEDS_MODULE"; do
Just an accident by my editor, I'll fix it for v2
>
>> +
>> + if [ -f "$tst_module" ]; then
>> + TST_MODPATH="$tst_module"
>> + break
>> + fi
>> + done
>> +
>> + if [ -z "$TST_MODPATH" ]; then
>> + if [ $_tst_is_required -eq 1 ]; then
>> + tst_brk TCONF "Failed to find module '$_tst_module'"
>> + else
>> + tst_res TINFO "Module '$_tst_module' not found."
> nit: please drop dot at the end (can be fixed by person who merges it).
Fixed for v2
>> + fi
>> + else
>> + tst_res TINFO "Found module at '$TST_MODPATH'"
>> + fi
>
> nit: this is IMHO more readable
> if [ -n "$TST_MODPATH" ]; then
> tst_res TINFO "Found module at '$TST_MODPATH'"
> return
> fi
>
> if [ $_tst_is_required -eq 1 ]; then
> tst_brk TCONF "Failed to find module '$_tst_module'"
> else
> tst_res TINFO "Module '$_tst_module' not found"
> fi
It would still keep the else and not use a return. Indentation clearly
shows what's going on.
But I agree to invert the logic, first testing the good case, than the bad.
>
> Kind regards,
> Petr
J?rg
^ permalink raw reply [flat|nested] 16+ messages in thread
* [LTP] [PATCH 2/2] tst_test.sh: Add public tst_test_root command
2019-10-09 6:57 ` Joerg Vehlow
@ 2019-10-09 7:53 ` Petr Vorel
0 siblings, 0 replies; 16+ messages in thread
From: Petr Vorel @ 2019-10-09 7:53 UTC (permalink / raw)
To: ltp
Hi J?rg,
> > You need to replace s/_tst_require_root/tst_test_root/ in other library files
> > (tst_net.sh, tst_security.sh).
> Sorry, did not expect functions with _ to be used in other files.
> I'll check it next time
No problem :). Yes, they cannot be used in tests. But new shell API library is spread in 3 files.
I added a patch to document library itself from developer perspective [1].
Kind regards,
Petr
[1] https://patchwork.ozlabs.org/patch/1166786/
^ permalink raw reply [flat|nested] 16+ messages in thread
* [LTP] [PATCH 2/2] tst_test.sh: Add public tst_test_root command
2019-10-09 6:16 ` [LTP] [PATCH 2/2] tst_test.sh: Add public tst_test_root command Joerg Vehlow
2019-10-09 6:52 ` Petr Vorel
@ 2019-10-09 11:39 ` Cyril Hrubis
2019-10-09 11:43 ` Joerg Vehlow
2019-10-11 8:20 ` Petr Vorel
1 sibling, 2 replies; 16+ messages in thread
From: Cyril Hrubis @ 2019-10-09 11:39 UTC (permalink / raw)
To: ltp
Hi!
> If a test requires root only under certan circumstances, TST_NEEDS_ROOT
> is not sufficient, because it always requires root.
> ---
> testcases/lib/tst_test.sh | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
> index c70a5abbe..07712670d 100644
> --- a/testcases/lib/tst_test.sh
> +++ b/testcases/lib/tst_test.sh
> @@ -326,6 +326,13 @@ tst_check_cmds()
> return 0
> }
>
> +tst_test_root()
> +{
> + if [ "$(id -ru)" != 0 ]; then
> + tst_brk TCONF "Must be super/root for this test!"
> + fi
> +}
Can we keep the name to be tst_require_root() please?
Historically this function has been always named like this in LTP.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 16+ messages in thread
* [LTP] [PATCH 2/2] tst_test.sh: Add public tst_test_root command
2019-10-09 11:39 ` Cyril Hrubis
@ 2019-10-09 11:43 ` Joerg Vehlow
2019-10-09 11:48 ` Cyril Hrubis
2019-10-11 8:20 ` Petr Vorel
1 sibling, 1 reply; 16+ messages in thread
From: Joerg Vehlow @ 2019-10-09 11:43 UTC (permalink / raw)
To: ltp
Hi,
> Can we keep the name to be tst_require_root() please?
>
> Historically this function has been always named like this in LTP.
>
Not a very good argument for two reasons:
1. The function was internal to the library, so no one (except for
library developers)
?? should know about it anyway. The old public interface
(TST_NEEDS_ROOT) is unchanged
2. I wanted to make it consistent with other functions, that do similar
stuff like
?? tst_test_drivers and tst_test_cmds. Both also call tst_brk in case
of unsuccessful tests.
J?rg
^ permalink raw reply [flat|nested] 16+ messages in thread
* [LTP] [PATCH 2/2] tst_test.sh: Add public tst_test_root command
2019-10-09 11:43 ` Joerg Vehlow
@ 2019-10-09 11:48 ` Cyril Hrubis
2019-10-09 11:53 ` Joerg Vehlow
0 siblings, 1 reply; 16+ messages in thread
From: Cyril Hrubis @ 2019-10-09 11:48 UTC (permalink / raw)
To: ltp
Hi!
> > Can we keep the name to be tst_require_root() please?
> >
> > Historically this function has been always named like this in LTP.
> >
> Not a very good argument for two reasons:
> 1. The function was internal to the library, so no one (except for
> library developers)
It has been in a public API for the old library for more than ten years
before the new library was written.
> 2. I wanted to make it consistent with other functions, that do similar
> stuff like
> ???? tst_test_drivers and tst_test_cmds. Both also call tst_brk in case
> of unsuccessful tests.
Well I do not like these names either, it's less descriptive that it
would have been with tst_require_cmds and tst_require_drivers, so if
anything I would be for renaming the tst_test_* ones.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 16+ messages in thread
* [LTP] [PATCH 2/2] tst_test.sh: Add public tst_test_root command
2019-10-09 11:48 ` Cyril Hrubis
@ 2019-10-09 11:53 ` Joerg Vehlow
2019-10-09 12:28 ` Cyril Hrubis
0 siblings, 1 reply; 16+ messages in thread
From: Joerg Vehlow @ 2019-10-09 11:53 UTC (permalink / raw)
To: ltp
>>> Can we keep the name to be tst_require_root() please?
>>>
>>> Historically this function has been always named like this in LTP.
>>>
>> Not a very good argument for two reasons:
>> 1. The function was internal to the library, so no one (except for
>> library developers)
> It has been in a public API for the old library for more than ten years
> before the new library was written.
Has been, but is not anymore. But I accept it as an argument.
>
>> 2. I wanted to make it consistent with other functions, that do similar
>> stuff like
>> ???? tst_test_drivers and tst_test_cmds. Both also call tst_brk in case
>> of unsuccessful tests.
> Well I do not like these names either, it's less descriptive that it
> would have been with tst_require_cmds and tst_require_drivers, so if
> anything I would be for renaming the tst_test_* ones.
Fair enough, that makes sense. But then we require a change to
tst_test_* first,
otherwise the api names are inconsistent. And I'm not sure if changing the
other functions is really worth it.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [LTP] [PATCH 2/2] tst_test.sh: Add public tst_test_root command
2019-10-09 11:53 ` Joerg Vehlow
@ 2019-10-09 12:28 ` Cyril Hrubis
2019-10-11 8:36 ` Petr Vorel
0 siblings, 1 reply; 16+ messages in thread
From: Cyril Hrubis @ 2019-10-09 12:28 UTC (permalink / raw)
To: ltp
Hi!
> >> 2. I wanted to make it consistent with other functions, that do similar
> >> stuff like
> >> ???? tst_test_drivers and tst_test_cmds. Both also call tst_brk in case
> >> of unsuccessful tests.
> > Well I do not like these names either, it's less descriptive that it
> > would have been with tst_require_cmds and tst_require_drivers, so if
> > anything I would be for renaming the tst_test_* ones.
> Fair enough, that makes sense. But then we require a change to
> tst_test_* first,
> otherwise the api names are inconsistent. And I'm not sure if changing the
> other functions is really worth it.
Should be easy for tst_test_drivers, that one is only used to implement
the NEEDS_DRIVERS variable. And for the second one, we would have to
change a few tests and one line of documentation, but that would be just
a simple script. I can do that if we agree on the change.
@Jan @Peter what do you think?
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 16+ messages in thread
* [LTP] [PATCH 2/2] tst_test.sh: Add public tst_test_root command
2019-10-09 11:39 ` Cyril Hrubis
2019-10-09 11:43 ` Joerg Vehlow
@ 2019-10-11 8:20 ` Petr Vorel
1 sibling, 0 replies; 16+ messages in thread
From: Petr Vorel @ 2019-10-11 8:20 UTC (permalink / raw)
To: ltp
Hi,
> > +tst_test_root()
> > +{
> > + if [ "$(id -ru)" != 0 ]; then
> > + tst_brk TCONF "Must be super/root for this test!"
> > + fi
> > +}
> Can we keep the name to be tst_require_root() please?
> Historically this function has been always named like this in LTP.
+1
Kind regards,
Petr
^ permalink raw reply [flat|nested] 16+ messages in thread
* [LTP] [PATCH 2/2] tst_test.sh: Add public tst_test_root command
2019-10-09 12:28 ` Cyril Hrubis
@ 2019-10-11 8:36 ` Petr Vorel
2019-10-11 8:39 ` Joerg Vehlow
0 siblings, 1 reply; 16+ messages in thread
From: Petr Vorel @ 2019-10-11 8:36 UTC (permalink / raw)
To: ltp
Hi Cyril,
> > >> 2. I wanted to make it consistent with other functions, that do similar
> > >> stuff like
> > >> ???? tst_test_drivers and tst_test_cmds. Both also call tst_brk in case
> > >> of unsuccessful tests.
> > > Well I do not like these names either, it's less descriptive that it
> > > would have been with tst_require_cmds and tst_require_drivers, so if
> > > anything I would be for renaming the tst_test_* ones.
> > Fair enough, that makes sense. But then we require a change to
> > tst_test_* first,
> > otherwise the api names are inconsistent. And I'm not sure if changing the
> > other functions is really worth it.
> Should be easy for tst_test_drivers, that one is only used to implement
> the NEEDS_DRIVERS variable. And for the second one, we would have to
> change a few tests and one line of documentation, but that would be just
> a simple script. I can do that if we agree on the change.
> @Jan @Peter what do you think?
We already did rename once (0567a8958 shell: Rename
s/tst_check_cmds/tst_test_cmds/), but even if we didn't;
I'm for renaming tst_test_* to tst_require_* - clear names are important
(consistency with names as well).
Kind regards,
Petr
^ permalink raw reply [flat|nested] 16+ messages in thread
* [LTP] [PATCH 2/2] tst_test.sh: Add public tst_test_root command
2019-10-11 8:36 ` Petr Vorel
@ 2019-10-11 8:39 ` Joerg Vehlow
0 siblings, 0 replies; 16+ messages in thread
From: Joerg Vehlow @ 2019-10-11 8:39 UTC (permalink / raw)
To: ltp
>> @Jan @Peter what do you think?
> We already did rename once (0567a8958 shell: Rename
> s/tst_check_cmds/tst_test_cmds/), but even if we didn't;
> I'm for renaming tst_test_* to tst_require_* - clear names are important
> (consistency with names as well).
That's two (and me) in favor of renaming tst_test_.
I will submit a patch for it.
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2019-10-11 8:39 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-09 6:16 [LTP] Add TST_USES_MODULE and tst_test_root Joerg Vehlow
2019-10-09 6:16 ` [LTP] [PATCH 1/2] tst_test.sh: Add TST_USES_MODULE Joerg Vehlow
2019-10-09 7:36 ` Petr Vorel
2019-10-09 7:48 ` Joerg Vehlow
2019-10-09 6:16 ` [LTP] [PATCH 2/2] tst_test.sh: Add public tst_test_root command Joerg Vehlow
2019-10-09 6:52 ` Petr Vorel
2019-10-09 6:57 ` Joerg Vehlow
2019-10-09 7:53 ` Petr Vorel
2019-10-09 11:39 ` Cyril Hrubis
2019-10-09 11:43 ` Joerg Vehlow
2019-10-09 11:48 ` Cyril Hrubis
2019-10-09 11:53 ` Joerg Vehlow
2019-10-09 12:28 ` Cyril Hrubis
2019-10-11 8:36 ` Petr Vorel
2019-10-11 8:39 ` Joerg Vehlow
2019-10-11 8:20 ` Petr Vorel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.