[LTP] [PATCH 1/1] tst_test.sh: Add test cmd helper tst_test_cmds()

[LTP] [PATCH 1/1] tst_test.sh: Add test cmd helper tst_test_cmds()

[Petr Vorel]

+ tst_cmd_available()

tst_test_cmds() is meant to be a check just for a particular test.
Works like tst_check_cmds(), but instead of tst_brk() calls tst_res().

tst_cmd_available() helper can handle cases when command shell builtin
is not available (e.g. Busybox).

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
You may don't like support for obscure shell with no command support (or
-v param for command which is IMHO not POSIX).
I introduced tst_cmd_available mainly for reducing duplicity, but it
might be useful anyway.

Kind regards,
Petr
---
 doc/test-writing-guidelines.txt | 17 +++++++++++++++++
 testcases/lib/tst_test.sh       | 22 ++++++++++++++++++++--
 2 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/doc/test-writing-guidelines.txt b/doc/test-writing-guidelines.txt
index cbbfe6c0f..bf59a178c 100644
--- a/doc/test-writing-guidelines.txt
+++ b/doc/test-writing-guidelines.txt
@@ -1519,6 +1519,23 @@ existence each of them and exits the test with 'TCONF' on first misssing.
 Alternatively the 'tst_check_cmds()' function can be used to do the same on
 runtime, since sometimes we need to the check at runtime too.
 
+'tst_test_cmds()' can be used for requirements just for a particular test
+as it doesn't exit. Supposed usage is:
+...
+
+TST_TESTFUNC=do_test
+. tst_test.sh
+
+do_test()
+{
+	tst_test_cmds cmd || return
+	cmd --foo
+	...
+}
+
+tst_run
+...
+
 Locating kernel modules
 +++++++++++++++++++++++
 
diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
index 48afb9cc4..5ebe32edf 100644
--- a/testcases/lib/tst_test.sh
+++ b/testcases/lib/tst_test.sh
@@ -201,12 +201,30 @@ tst_mkfs()
 	ROD_SILENT mkfs.$fs_type $fs_opts $device
 }
 
+tst_cmd_available()
+{
+	if type command > /dev/null 2>&1; then
+		command -v $1 > /dev/null 2>&1 || return 1
+	else
+		which $1 > /dev/null 2>&1 || return 1
+	fi
+}
+
 tst_check_cmds()
 {
 	local cmd
 	for cmd in $*; do
-		if ! command -v $cmd > /dev/null 2>&1; then
-			tst_brk TCONF "'$cmd' not found"
+		tst_cmd_available $cmd || tst_brk TCONF "'$cmd' not found"
+	done
+}
+
+tst_test_cmds()
+{
+	local cmd
+	for cmd in $*; do
+		if ! tst_cmd_available $cmd; then
+			tst_res TCONF "'$cmd' not found"
+			return 1
 		fi
 	done
 }
-- 
2.16.2

[LTP] [PATCH 1/1] tst_test.sh: Add test cmd helper tst_test_cmds()

[Petr Vorel]

Hi,

> + tst_cmd_available()

> tst_test_cmds() is meant to be a check just for a particular test.
> Works like tst_check_cmds(), but instead of tst_brk() calls tst_res().

> tst_cmd_available() helper can handle cases when command shell builtin
> is not available (e.g. Busybox).

> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
...
> +tst_cmd_available()
> +{
> +	if type command > /dev/null 2>&1; then
> +		command -v $1 > /dev/null 2>&1 || return 1
> +	else
> +		which $1 > /dev/null 2>&1 || return 1
Pedantic version would check $? for 127 (indicating which itself it's not found).
Probably worth of adding it.

Kind regards,
Petr

[LTP] [PATCH 1/1] tst_test.sh: Add test cmd helper tst_test_cmds()

[Cyril Hrubis]

Hi!
> > +		which $1 > /dev/null 2>&1 || return 1
> Pedantic version would check $? for 127 (indicating which itself it's not found).
> Probably worth of adding it.

Are you sure?

$ which foo
...
$ echo $?
1

Also:

$ busybox which foo
$ echo $?
1


-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 1/1] tst_test.sh: Add test cmd helper tst_test_cmds()

[Petr Vorel]

Hi Cyril,

> > > +		which $1 > /dev/null 2>&1 || return 1
> > Pedantic version would check $? for 127 (indicating which itself it's not found).
> > Probably worth of adding it.

> Are you sure?

> $ which foo
> ...
> $ echo $?
> 1

> Also:

> $ busybox which foo
> $ echo $?
> 1

Yes, exit code 1 is when 'which' command doesn't find command.
Exit code 127 at least some shells use when command not found (in this case when 'which'
command itself is not found.

$ busybox foo; echo $?
foo: applet not found
127

Missing which IMHO only the case of busybox build with CONFIG_WHICH=n (but default is 'y')
=> corner case. I understand if we want to ignore it.

(+ Android - at least old versions had 'toolbox' instead of busybox => we'd ignore this
Android it's special and currently not fully supported anyway.


Kind regards,
Petr

[LTP] [PATCH 1/1] tst_test.sh: Add test cmd helper tst_test_cmds()

[Cyril Hrubis]

Hi!
>  doc/test-writing-guidelines.txt | 17 +++++++++++++++++
>  testcases/lib/tst_test.sh       | 22 ++++++++++++++++++++--
>  2 files changed, 37 insertions(+), 2 deletions(-)
> 
> diff --git a/doc/test-writing-guidelines.txt b/doc/test-writing-guidelines.txt
> index cbbfe6c0f..bf59a178c 100644
> --- a/doc/test-writing-guidelines.txt
> +++ b/doc/test-writing-guidelines.txt
> @@ -1519,6 +1519,23 @@ existence each of them and exits the test with 'TCONF' on first misssing.
>  Alternatively the 'tst_check_cmds()' function can be used to do the same on
>  runtime, since sometimes we need to the check at runtime too.
>  
> +'tst_test_cmds()' can be used for requirements just for a particular test
> +as it doesn't exit. Supposed usage is:
                         ^
			 Expected

Also we really should say that the call will issue TCONF here, because
it's not clear that it would.

> +...
> +
> +TST_TESTFUNC=do_test
> +. tst_test.sh
> +
> +do_test()
> +{
> +	tst_test_cmds cmd || return
> +	cmd --foo
> +	...
> +}
> +
> +tst_run
> +...
> +
>  Locating kernel modules
>  +++++++++++++++++++++++
>  
> diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
> index 48afb9cc4..5ebe32edf 100644
> --- a/testcases/lib/tst_test.sh
> +++ b/testcases/lib/tst_test.sh
> @@ -201,12 +201,30 @@ tst_mkfs()
>  	ROD_SILENT mkfs.$fs_type $fs_opts $device
>  }
>  
> +tst_cmd_available()
> +{
> +	if type command > /dev/null 2>&1; then
> +		command -v $1 > /dev/null 2>&1 || return 1
> +	else
> +		which $1 > /dev/null 2>&1 || return 1
> +	fi
> +}

We are falling back to which if command is not available here?

Are you aware of any shell that is lacking command?

Also we should probably add return 0 at the end of the function, as it
is the code is correct, since the return value would be return value of
the last executed line, which is guaranteed to be 0 because of the
|| return 1 but it's kind of confusing to omit it.

>  tst_check_cmds()
>  {
>  	local cmd
>  	for cmd in $*; do
> -		if ! command -v $cmd > /dev/null 2>&1; then
> -			tst_brk TCONF "'$cmd' not found"
> +		tst_cmd_available $cmd || tst_brk TCONF "'$cmd' not found"
> +	done
> +}
> +
> +tst_test_cmds()
> +{
> +	local cmd
> +	for cmd in $*; do
> +		if ! tst_cmd_available $cmd; then
> +			tst_res TCONF "'$cmd' not found"
> +			return 1
>  		fi
>  	done
>  }

Here as well, explicit return 0 will help code readability a bit.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 1/1] tst_test.sh: Add test cmd helper tst_test_cmds()

[Petr Vorel]

Hi,

...
> > +'tst_test_cmds()' can be used for requirements just for a particular test
> > +as it doesn't exit. Supposed usage is:
>                          ^
> 			 Expected

> Also we really should say that the call will issue TCONF here, because
> it's not clear that it would.

...
> > +tst_cmd_available()
> > +{
> > +	if type command > /dev/null 2>&1; then
> > +		command -v $1 > /dev/null 2>&1 || return 1
> > +	else
> > +		which $1 > /dev/null 2>&1 || return 1
> > +	fi
> > +}

> We are falling back to which if command is not available here?

> Are you aware of any shell that is lacking command?
Busybox configured with CONFIG_WHICH=n (but default is 'y') => corner case.

> Also we should probably add return 0 at the end of the function, as it
> is the code is correct, since the return value would be return value of
> the last executed line, which is guaranteed to be 0 because of the
> || return 1 but it's kind of confusing to omit it.
Sure, I'll do. (I've learned to avoid 'return 0' from test_net.sh, but it can be
dangerous).


Kind regards,
Petr