Re: [LTP] [PATCH v3] Add containers/share to PATH when call init_ltp_netspace

[Petr Vorel <pvorel@suse.cz>]

Hi Wei,

There are 2 unrelated changes, they should be separated (in 2 commits).
We are sometimes benevolent on this, if the other change is tiny,
but these are 2 functional changes, they should really be separated.

Also, the subject (first line of the patch) is too long and describe only one
change.

> When run single test case use command such as:
> LTP_SHELL_API_TESTS=shell/net/tst_rhost_run.sh make test-shell
Instead of the description how to run tst_rhost_run.sh
(which now with the PATH can be run simply as
./lib/newlib_tests/shell/net/tst_rhost_run.sh)
more important is to say:
1) LTP shell API tests depend on properly compiled LTP.
Therefore this is just a workaround to make visible that some tool is missing.

2) I wonder if there is way to properly fix this dependency in make.
I guess test-shell target should depend on (at least): ns_create ns_exec
ns_ifmove.

> Error msg such as "ns_create: command not found" will popup, so
> need update PATH before call ns_create etc..

> Signed-off-by: WEI GAO <wegao@suse.com>
> ---
> V2 -> V3: move path to test case itself 
> V1 -> V2: add tst_require_cmds for init_ltp_netspace()

>  lib/newlib_tests/shell/net/tst_rhost_run.sh | 2 +-
>  testcases/lib/tst_net.sh                    | 3 ++-
>  2 files changed, 3 insertions(+), 2 deletions(-)

> diff --git a/lib/newlib_tests/shell/net/tst_rhost_run.sh b/lib/newlib_tests/shell/net/tst_rhost_run.sh
> index 773b8dd33..70087aa61 100755
> --- a/lib/newlib_tests/shell/net/tst_rhost_run.sh
> +++ b/lib/newlib_tests/shell/net/tst_rhost_run.sh
> @@ -3,7 +3,7 @@
>  # Copyright (c) 2020 Petr Vorel <pvorel@suse.cz>

>  TST_TESTFUNC=do_test
> -PATH="$(dirname $0)/../../../../testcases/lib/:$PATH"
> +PATH="$(dirname $0)/../../../../testcases/lib/:$(dirname $0)/../../../../testcases/kernel/containers/share/:$PATH"
PATH line will get quite long, I'd use variable for path to root.
It will be also more readable (similar to Makefiles, which have top_srcdir
variable).

root="$(dirname $0)/../../../../"
PATH="$root/testcases/lib/:$root/testcases/kernel/containers/share/:$PATH"

>  export TST_NET_RHOST_RUN_DEBUG=1

> diff --git a/testcases/lib/tst_net.sh b/testcases/lib/tst_net.sh
> index ceb45c98d..2849f6bc6 100644
> --- a/testcases/lib/tst_net.sh
> +++ b/testcases/lib/tst_net.sh
> @@ -128,10 +128,11 @@ tst_net_require_ipv6()

>  init_ltp_netspace()
>  {
> +
Please avoid added extra blank lines like this one.
>  	local pid

Kind regards,
Petr

>  	if [ ! -f /var/run/netns/ltp_ns -a -z "$LTP_NETNS" ]; then
> -		tst_require_cmds ip
> +		tst_require_cmds ip ns_create ns_exec ns_ifmove
>  		tst_require_root

>  		tst_require_drivers veth

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp