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

[Petr Vorel <pvorel@suse.cz>]

Hi Wei, all,

first, please wait little bit before sending next version.
We might decide to remove these tests.
Below are notes in case we keep them.

> 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

Yes, this would work.

> > + 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).

IMHO no. TST_NEEDS_CMDS should not contain ftp. On Tumbleweed with lftp
installed there is no ftp symlink:

$ lftp --version
LFTP | Version 4.9.2 | Copyright (c) 1996-2020 Alexander V. Lukyanov
...

$ ftp

The program 'ftp' can be found in following packages:
  * ftp [ path: /usr/bin/ftp, repository: OSS ]
  * tnftp [ path: /usr/bin/ftp, repository: OSS ]

Try installing with:
    sudo zypper install <selected_package>

Kind regards,
Petr

> > Kind regards,
> > Petr

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