[LTP] [PATCH v2] ftp01.sh: Add support for test lftp

[Wei Gao]

On Tue, Oct 15, 2024 at 09:39:58PM +0200, Petr Vorel wrote:
> Hi Wei,
> 
> I suppose the v1 is
> https://patchwork.ozlabs.org/project/ltp/patch/20240918100344.21316-1-wegao@suse.com/
Correct
> 
> To be honest, I would rather to remove this FTP test because FTP protocol is
> legacy. I know it is supposed to be a smoke test, but maybe using modern tools
> would be better than keeping test working among various old FTP implementations.
> (Also nontrivial setup is required just for few FTP tests:
> https://github.com/linux-test-project/ltp/tree/master/testcases/network )
> But probably Cyril would be against. @Cyril @Martin WDYT?
> 
> @Wei I suppose the reason for adding lftp (you did not explain it in the commit
> message) is that is the only supported client in SLE Micro? Or something else?
Adding lftp is used for fix SLE test such as SLE-15SP6, on SLE Micro will result TCONF
since no any ftp command support currently.

BTW: there is another PR on openqa side to setup local ssh network env.
https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/20278 

All detail discussion can be also found in:
https://progress.opensuse.org/issues/165848

> 
> > Signed-off-by: Wei Gao <wegao@suse.com>
> > ---
> >  testcases/network/tcp_cmds/ftp/ftp01.sh | 15 +++++++++++++--
> >  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> > diff --git a/testcases/network/tcp_cmds/ftp/ftp01.sh b/testcases/network/tcp_cmds/ftp/ftp01.sh
> > index 53d1eec53..12d32a9a9 100755
> > --- a/testcases/network/tcp_cmds/ftp/ftp01.sh
> > +++ b/testcases/network/tcp_cmds/ftp/ftp01.sh
> > @@ -4,6 +4,7 @@
> >  # Copyright (c) 2003 Manoj Iyer <manjo@mail.utexas.edu>
> >  # Copyright (c) 2001 Robbie Williamson <robbiew@us.ibm.com>
> 
> > +TST_SETUP=setup
> >  TST_TESTFUNC=do_test
> >  TST_CNT=4
> >  TST_NEEDS_CMDS='awk ftp'
> > @@ -11,6 +12,16 @@ TST_NEEDS_TMPDIR=1
> 
> >  RUSER="${RUSER:-root}"
> >  RHOST="${RHOST:-localhost}"
> > +FTP_CMD="ftp -nv"
> > +
> > +setup()
> > +{
> > +    if tst_cmd_available lftp; then
> > +        FTP_CMD="lftp --norc"
> > +    else
> > +        tst_brkm TCONF "No FTP client found"
> Test was converted to the new API, it must be tst_brk.
> Also, this code basically means that testing is done only for lftp,
> otherwise TCONF.
Thanks for point out this. it should replace to following code as Martin suggested:

if tst_cmd_available ftp; then
	FTP_CMD="ftp -nv"
elif tst_cmd_available lftp; then
	FTP_CMD="lftp --norc"
else
	tst_brk TCONF "No FTP client found"
fi

> + You keep requiring ftp in TST_NEEDS_CMDS, but at least on Tumbleweed and
> current Debian testing lftp does not provide symlink to ftp, only tnftp does
> this. Therefore you require both lftp and tnftp for testing.
> 
I suppose ONLY requiring ftp in TST_NEEDS_CMDS is enough, lftp is specific case for sle check.
For Tumbleweed and other OS will directly use ftp(TW only contain ftp by default).

> Kind regards,
> Petr