| Message ID | 20201116101840.15433-1-kory.maincent@bootlin.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | net/host: update to new api | expand |
Hi Kory, thanks for your patch. ... > do_test() > { > - tst_resm TINFO "test basic functionality of the \`$TC' command." > + tst_res TINFO "test basic functionality of the host command." > - while [ $TST_COUNT -lt $NUMLOOPS ]; do > + while [ $TST_COUNT -le $NUMLOOPS ]; do IMHO there is no need to have loop like this. If required, we'd just add -iN parameter to it in the runtest file (where N is <1,max int), but IMHO it's enough to test host only once. > if rhost_addr=$(host $RHOST); then > - rhost_addr=$(echo "$rhost_addr" | awk -F, '{print $NF}') >/dev/null 2>&1 > - if ! host $rhost_addr >/dev/null 2>&1; then > - end_testcase "reverse lookup with host failed" > - fi > - > + rhost_addr=$(echo "$rhost_addr" | awk '{print $NF}') >/dev/null 2>&1 > + EXPECT_PASS host $rhost_addr \>/dev/null 2>&1 We need to redirect also second > and &: EXPECT_PASS host $rhost_addr \>/dev/null 2\>\&1 > else > - end_testcase "host $RHOST on local machine failed" > + tst_brk TFAIL "host $RHOST on local machine failed" > fi > - incr_tst_count > + TST_COUNT=$((TST_COUNT + 1)) > sleep $SLEEPTIME Also sleep time is not really needed. ... Can I merge this simplified variant? Kind regards, Petr #!/bin/sh # SPDX-License-Identifier: GPL-2.0-or-later # Copyright (c) Köry Maincent <kory.maincent@bootlin.com> 2020 # Copyright (c) Manoj Iyer <manjo@mail.utexas.edu> 2003 # Copyright (c) Robbie Williamson <robbiew@us.ibm.com> 2001 # Copyright (c) International Business Machines Corp., 2000 TST_TESTFUNC="do_test" TST_NEEDS_CMDS="awk host hostname" . tst_net.sh do_test() { local rhost=${RHOST:-$(hostname)} local rhost_addr tst_res TINFO "test basic functionality of the host command" if rhost_addr=$(host $rhost); then rhost_addr=$(echo "$rhost_addr" | awk '{print $NF}') EXPECT_PASS host $rhost_addr \>/dev/null 2\>\&1 else tst_brk TFAIL "host $rhost on local machine failed" fi } tst_run
Hi Kory, Alexey, > Hi Kory, > thanks for your patch. > ... > > do_test() > > { > > - tst_resm TINFO "test basic functionality of the \`$TC' command." > > + tst_res TINFO "test basic functionality of the host command." > > - while [ $TST_COUNT -lt $NUMLOOPS ]; do > > + while [ $TST_COUNT -le $NUMLOOPS ]; do > IMHO there is no need to have loop like this. > If required, we'd just add -iN parameter to it in the runtest file (where N is > <1,max int), but IMHO it's enough to test host only once. > > if rhost_addr=$(host $RHOST); then > > - rhost_addr=$(echo "$rhost_addr" | awk -F, '{print $NF}') >/dev/null 2>&1 > > - if ! host $rhost_addr >/dev/null 2>&1; then > > - end_testcase "reverse lookup with host failed" > > - fi > > - > > + rhost_addr=$(echo "$rhost_addr" | awk '{print $NF}') >/dev/null 2>&1 > > + EXPECT_PASS host $rhost_addr \>/dev/null 2>&1 > We need to redirect also second > and &: > EXPECT_PASS host $rhost_addr \>/dev/null 2\>\&1 BTW 2>&1 does not make sense in this context (it's a redirection of error message of EXPECT_PASS function, which would be to stdout. I also noticed this part of traceroute01.sh is also wrong: EXPECT_PASS traceroute $ip $bytes -n -m 2 $opts \>out.log 2>&1 It should be: EXPECT_PASS traceroute $ip $bytes -n -m 2 $opts \>out.log 2\>\&1 But IMHO it'd be better in both cases to keep stderr not redirected (don't hide problems). Therefore, unless you're against it, I'll remove redirection from traceroute01.sh. Kind regards, Petr
Hello Petr, On Mon, 16 Nov 2020 19:48:25 +0100 Petr Vorel <pvorel@suse.cz> wrote: > > > + EXPECT_PASS host $rhost_addr \>/dev/null 2>&1 > > We need to redirect also second > and &: > > EXPECT_PASS host $rhost_addr \>/dev/null 2\>\&1 > > BTW 2>&1 does not make sense in this context (it's a redirection of error > message of EXPECT_PASS function, which would be to stdout. > I also noticed this part of traceroute01.sh is also wrong: > EXPECT_PASS traceroute $ip $bytes -n -m 2 $opts \>out.log 2>&1 > > It should be: > EXPECT_PASS traceroute $ip $bytes -n -m 2 $opts \>out.log 2\>\&1 > > But IMHO it'd be better in both cases to keep stderr not redirected > (don't hide problems). Therefore, unless you're against it, I'll remove > redirection from traceroute01.sh. Yes, your right. I am new to LTP so I still make some obvious mistakes, thank you for taking time to correct me. The simplified variant without stderr redirection seems okay to me. Regards, Köry
On 16.11.2020 21:41, Petr Vorel wrote: > Hi Kory, > > thanks for your patch. > ... >> do_test() >> { > >> - tst_resm TINFO "test basic functionality of the \`$TC' command." >> + tst_res TINFO "test basic functionality of the host command." > >> - while [ $TST_COUNT -lt $NUMLOOPS ]; do >> + while [ $TST_COUNT -le $NUMLOOPS ]; do > IMHO there is no need to have loop like this. > If required, we'd just add -iN parameter to it in the runtest file (where N is > <1,max int), but IMHO it's enough to test host only once. > >> if rhost_addr=$(host $RHOST); then >> - rhost_addr=$(echo "$rhost_addr" | awk -F, '{print $NF}') >/dev/null 2>&1 >> - if ! host $rhost_addr >/dev/null 2>&1; then >> - end_testcase "reverse lookup with host failed" >> - fi >> - >> + rhost_addr=$(echo "$rhost_addr" | awk '{print $NF}') >/dev/null 2>&1 >> + EXPECT_PASS host $rhost_addr \>/dev/null 2>&1 > We need to redirect also second > and &: > EXPECT_PASS host $rhost_addr \>/dev/null 2\>\&1 > >> else >> - end_testcase "host $RHOST on local machine failed" >> + tst_brk TFAIL "host $RHOST on local machine failed" >> fi > >> - incr_tst_count >> + TST_COUNT=$((TST_COUNT + 1)) >> sleep $SLEEPTIME > Also sleep time is not really needed. > > ... > > Can I merge this simplified variant? ... > > . tst_net.sh > > do_test() > { > local rhost=${RHOST:-$(hostname)} Hi Petr, Kory For new API we shouldn't use RHOST. Also the naming is misleading, it can be a local host name. > local rhost_addr > > tst_res TINFO "test basic functionality of the host command" > > if rhost_addr=$(host $rhost); then > rhost_addr=$(echo "$rhost_addr" | awk '{print $NF}') > EXPECT_PASS host $rhost_addr \>/dev/null 2\>\&1 > else > tst_brk TFAIL "host $rhost on local machine failed" > fi > } > > tst_run >
Hi Alexey, > > do_test() > > { > > local rhost=${RHOST:-$(hostname)} > Hi Petr, Kory > For new API we shouldn't use RHOST. Also the naming is misleading, > it can be a local host name. Thanks for catching it. Suppose this version is ok. Kind regards, Petr TST_TESTFUNC="do_test" TST_NEEDS_CMDS="awk host hostname" . tst_net.sh do_test() { local lhost="$(hostname)" local addr tst_res TINFO "test basic functionality of the host command" if addr=$(host $lhost); then addr=$(echo "$addr" | awk '{print $NF}') EXPECT_PASS host $addr \>/dev/null else tst_brk TFAIL "host $lhost on local machine failed" fi } tst_run
diff --git a/testcases/network/tcp_cmds/host/host01.sh b/testcases/network/tcp_cmds/host/host01.sh index 1308870f7..4987cb838 100755 --- a/testcases/network/tcp_cmds/host/host01.sh +++ b/testcases/network/tcp_cmds/host/host01.sh @@ -1,87 +1,43 @@ #!/bin/sh # -# Copyright (c) International Business Machines Corp., 2000 -# -# This program is free software; you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation; either version 2 of the License, or -# (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See -# the GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program; if not, write to the Free Software -# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA -# -# -# -# FILE : host -# -# PURPOSE: To test the basic functionality of the `host` command. -# -# SETUP: If "RHOST" is not exported, then the local hostname is used. -# -# HISTORY: -# 06/06/03 Manoj Iyer manjo@mail.utexas.edu -# - Modified to use LTP tests APIs -# 03/01 Robbie Williamson (robbiew@us.ibm.com) -# -Ported -# -# -#----------------------------------------------------------------------- +# SPDX-License-Identifier: GPL-2.0-or-later +# Copyright (c) Köry Maincent <kory.maincent@bootlin.com> 2020 +# Copyright (c) Manoj Iyer <manjo@mail.utexas.edu> 2003 +# Copyright (c) Robbie Williamson <robbiew@us.ibm.com> 2001 +# Copyright (c) International Business Machines Corp., 2000 + +TST_SETUP="do_setup" +TST_TESTFUNC="do_test" +TST_NEEDS_CMDS="awk host hostname" + +. tst_net.sh do_setup() { NUMLOOPS=${NUMLOOPS:-1} SLEEPTIME=${SLEEPTIME:-0} - - tst_setup - - exists awk host hostname - RHOST=${RHOST:-`hostname`} - } -#----------------------------------------------------------------------- -# -# FUNCTION: do_test -# -#----------------------------------------------------------------------- - do_test() { - tst_resm TINFO "test basic functionality of the \`$TC' command." + tst_res TINFO "test basic functionality of the host command." - while [ $TST_COUNT -lt $NUMLOOPS ]; do + while [ $TST_COUNT -le $NUMLOOPS ]; do if rhost_addr=$(host $RHOST); then - rhost_addr=$(echo "$rhost_addr" | awk -F, '{print $NF}') >/dev/null 2>&1 - if ! host $rhost_addr >/dev/null 2>&1; then - end_testcase "reverse lookup with host failed" - fi - + rhost_addr=$(echo "$rhost_addr" | awk '{print $NF}') >/dev/null 2>&1 + EXPECT_PASS host $rhost_addr \>/dev/null 2>&1 else - end_testcase "host $RHOST on local machine failed" + tst_brk TFAIL "host $RHOST on local machine failed" fi - incr_tst_count + TST_COUNT=$((TST_COUNT + 1)) sleep $SLEEPTIME done } -#----------------------------------------------------------------------- -# FUNCTION: MAIN -#----------------------------------------------------------------------- -. net_cmdlib.sh - -read_opts $* -do_setup -do_test -end_testcase +tst_run
Fixed awk field separator parsing. Signed-off-by: Kory Maincent <kory.maincent@bootlin.com> --- testcases/network/tcp_cmds/host/host01.sh | 80 +++++------------------ 1 file changed, 18 insertions(+), 62 deletions(-)