Re: [LTP] [PATCH v3 4/5] tst_test.sh: Cleanup getopts usage

[Petr Vorel <pvorel@suse.cz>]

Hi Cyril,

...
> > diff --git a/testcases/lib/tst_net.sh b/testcases/lib/tst_net.sh
...
> > +[ -n "$TST_USE_LEGACY_API" ] && . test.sh || . tst_test.sh
> >  [ -n "$TST_PRINT_HELP" -o -n "$TST_NET_SKIP_VARIABLE_INIT" ] && return 0
>             ^
> 	    This is no longer set in the tst_test.sh.
Good point, thanks!

> And we do not return from the tst_test.sh when -h was passed so I guess
> that we can just delete this part of the check.
No, this is needed for lib/newlib_tests/shell/net/tst_ipaddr_un.sh, which really
does not want to initialize netns.

...
> > diff --git a/testcases/network/dhcp/dnsmasq_tests.sh b/testcases/network/dhcp/dnsmasq_tests.sh
...
> > +. dhcp_lib.sh
> > +
> > +lease_dir="/var/lib/misc"
> > +tst_selinux_enforced && lease_dir="/var/lib/dnsmasq"
> > +
> > +dhcp_name="dnsmasq"
> > +log="/var/log/dnsmasq.tst.log"
> > +
> > +lease_file="$lease_dir/dnsmasq.tst.leases"
> > +
> > +common_opt="--no-hosts --no-resolv --dhcp-authoritative \
> > +	--log-facility=$log --interface=$iface0 \
> > +	--dhcp-leasefile=$lease_file --port=0 --conf-file= "

> Here as well, it does not seem that these variables are redefined so can
> we move the . dhcp_lib.sh just before the tst_run?

Again, needed, because tst_selinux_enforced can be called after loading
tst_test.sh (here after dhcp_lib.sh), because till then it's not visible.

I noted it after v1:
https://lore.kernel.org/ltp/Ykd9CAXfi9FZ+iHB@pevik/

I note it in the commit message.

> >  tst_run

> ...

> > diff --git a/testcases/network/stress/broken_ip/broken_ip-nexthdr.sh b/testcases/network/stress/broken_ip/broken_ip-nexthdr.sh
> > index ec6643af6..805b1f5ab 100755
> > --- a/testcases/network/stress/broken_ip/broken_ip-nexthdr.sh
> > +++ b/testcases/network/stress/broken_ip/broken_ip-nexthdr.sh
> > @@ -6,17 +6,17 @@
> >  # Author: Mitsuru Chinen <mitch@jp.ibm.com>

> >  TST_TESTFUNC="do_test"
> > -. tst_net.sh

> >  do_test()
> >  {
> > -	# not supported on IPv4
> > -	TST_IPV6=6
> > -	TST_IPVER=6
> > -
> >  	tst_res TINFO "Sending ICMPv6 with wrong next header for $NS_DURATION sec"
> >  	tst_icmp -t $NS_DURATION -s "0 100 500 1000 $NS_ICMPV6_SENDER_DATA_MAXSIZE" -n
> >  	tst_ping
> >  }

> > +. tst_net.sh
> > +# not supported on IPv4
> > +TST_IPV6=6
> > +TST_IPVER=6

> Here as well, are these actually changed if we set them before we source
> the tst_net.sh?
Well, they are changed *after* loading tst_net.sh.

> >  tst_run

> Other than these minor things this is rather nice cleanup that
> simplifies the code a bit.

> I guess that in the long term we can clean the shell tests so that there
> is no need to have explicit call to the tst_run and instead the test
> would be started automatically from the sourced tst_test.sh
+1 (although it was nice to search for tst_run to see what is the actual test,
because there are several shell libraries to search).

> Anyways I think that this patchset is good to go as long as we shuffle
> the stuff that can be shuffled. With that:

> Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

Thanks!

As Martin replied to netns_breakns.sh args, I'm going to merge it,
because I have 3 patchsets which are waiting for this to be merged.

Kind regards,
Petr

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