Re: [LTP] [PATCH 1/6] ftp/ftp01: Use tst_net.sh

[Petr Vorel <pvorel@suse.cz>]

Hi Akihiko,

we do this rewrite in a single commit. It does not make much sense to split it.
I'd squash it, if it were ready, but we're not there yet.
Also, you're supposed to replace GPL verbose text at the top, history etc.
with just:

#!/bin/sh
# SPDX-License-Identifier: GPL-2.0-or-later
# Copyright (c) 2022 Akihiko Odaki <akihiko.odaki@daynix.com>
# Copyright (c) 2003 Manoj Iyer <manjo@mail.utexas.edu>
# Copyright (c) 2001 Robbie Williamson <robbiew@us.ibm.com>

TST_TESTFUNC=do_test
TST_CNT=4
...
You forget TST_CNT=4, thus only single test is being run.

i.e. all the useless and now even outdated comments (even mentioning .rhosts, history, setup)
must go away.

The main problem is, that with improperly installed data files test happily
passes, because compares empty strings:

ftp01 1 TINFO: timeout per run is 0h 5m 0s
Not connected.
Not connected.
Not connected.
Not connected.
ls: cannot access 'ascii.sm': No such file or directory
ls: cannot access '/opt/ltp/testcases/bin/datafiles/ascii.sm': No such file or directory
ftp01 1 TPASS: [ '' = '' ] passed as expected
Not connected.
Not connected.
Not connected.
Not connected.
ls: cannot access 'ascii.med': No such file or directory
ls: cannot access '/opt/ltp/testcases/bin/datafiles/ascii.med': No such file or directory
ftp01 1 TPASS: [ '' = '' ] passed as expected
Not connected.
Not connected.
Not connected.
Not connected.
ls: cannot access 'ascii.lg': No such file or directory
ls: cannot access '/opt/ltp/testcases/bin/datafiles/ascii.lg': No such file or directory
ftp01 1 TPASS: [ '' = '' ] passed as expected
Not connected.
Not connected.
Not connected.
Not connected.
ls: cannot access 'ascii.jmb': No such file or directory
ls: cannot access '/opt/ltp/testcases/bin/datafiles/ascii.jmb': No such file or directory
ftp01 1 TPASS: [ '' = '' ] passed as expected

Summary:
passed   4
failed   0
broken   0
skipped  0
warnings 0

When installing it properly, it does not work:

ftp01 1 TINFO: timeout per run is 0h 5m 0s
Not connected.
Not connected.
Not connected.
Not connected.
ls: cannot access 'ascii.sm': No such file or directory
ftp01 1 TFAIL: [ '' = '220' ] failed unexpectedly
Not connected.
Not connected.
Not connected.
Not connected.
ls: cannot access 'ascii.med': No such file or directory
ftp01 1 TFAIL: [ '' = '4020' ] failed unexpectedly
Not connected.
Not connected.
Not connected.
Not connected.
ls: cannot access 'ascii.lg': No such file or directory
ftp01 1 TFAIL: [ '' = '80020' ] failed unexpectedly
Not connected.
Not connected.
Not connected.
Not connected.
ls: cannot access 'ascii.jmb': No such file or directory
ftp01 1 TFAIL: [ '' = '1600020' ] failed unexpectedly
ftp01 2 TINFO: AppArmor enabled, this may affect test results
ftp01 2 TINFO: it can be disabled with TST_DISABLE_APPARMOR=1 (requires super/root)
ftp01 2 TINFO: loaded AppArmor profiles: none

Summary:
passed   0
failed   4
broken   0
skipped  0
warnings 0

First, there should be some check for $sum1 or $sum2 not being empty,
and probably if the file exists (probably in the setup).

The reason is in your version of setup function:
do_setup()
{

    TC=ftp
    RUSER=${RUSER:-root}

}

Here you have
* empty lines (remove it)
* TC=ftp is useless (remove it)
* RUSER could be defined above the setup function
(more readable, no need to be in the setup function)

But somehow cd into $TST_NET_DATAROOT in ftp code does not work.
I've tried to copy it instead:

do_setup()
{
    local file

    for file in $ASCII_FILES $BINARY_FILES; do
        ROD cp -v $TST_NET_DATAROOT/$file .
    done
}

But that does not work. I need some time to figure out what's wrong
(not an FTP expert).

Other things to fix:

Also file variable in test_get() and test_put() should be also declared with
local.

I don't like list_files() at all, why not just define
at the top of the test and then use them as I suggested before?

ASCII_FILES='bin.sm bin.med bin.lg bin.jmb'
BINARY_FILES='ascii.sm ascii.med ascii.lg ascii.jmb'

do_test()
{
    case $1 in
    1) test_get $BIN_FILES;;
    2) test_get $ASCII_FILES;;
    3) test_put $BIN_FILES;;
    4) test_put $ASCII_FILES;;
    esac
}

Also, thinking about the tool which gets the size. Looking at
https://stackoverflow.com/questions/1815329/portable-way-to-get-file-size-in-bytes-in-the-shell
https://unix.stackexchange.com/questions/16640/how-can-i-get-the-size-of-a-file-in-a-bash-script
'ls -l | awk '{print $5}' might be really the best tool to get file size as both
ls and awk are probably everywhere. And although we wrote some C code to avoid
external dependencies, here I'd keep it, because other alternatives has
drawbacks:

'du -b' is fast, but might not be everywhere.

'stat -c %s' is reported not to be portable:
https://stackoverflow.com/questions/1815329/portable-way-to-get-file-size-in-bytes-in-the-shell#comment21604978_5258707

'wc -c' is reported to be slow and also might not be everywhere:
https://stackoverflow.com/questions/1815329/portable-way-to-get-file-size-in-bytes-in-the-shell#comment21604978_5258707

'find "$file" -printf "%s"' would also work, but might not be everywhere.

I'll try to fix the problem, but not sure when I get time to fix it.
I've added all fixes into my fork, feel free to use it if you have time to post
new version. But please, test it before whether it works.
https://github.com/pevik/ltp/commits/akihiko_odaki/ftp01.v2.fixes

Kind regards,
Petr

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