[Fuego] [PATCH] Improve test cases for command ethtool.

Tim.Bird at sony.com Tim.Bird at sony.com
Sat Feb 16 15:08:43 UTC 2019


This needs to be restructured.  See below.

> -----Original Message-----
> From: Zheng Ruoqin
> 
> 1. Check ETHERNET_DEVICE_NAME value before testing ethtool really
> 
> 2. Put ETHERNET_DEVICE_NAME detection to specific function in library
> 
> Signed-off-by: Wang Mingyu <wangmy at cn.fujitsu.com>
> Signed-off-by: Zheng Ruoqin <zhengrq.fnst at cn.fujitsu.com>
> ---
>  scripts/fuego_board_function_lib.sh              | 15 +++++++++++++++
>  tests/Functional.ethtool/fuego_test.sh           |  1 +
>  tests/Functional.ethtool/tests/ethtool_driver.sh | 17 +++++++++--------
>  tests/Functional.ethtool/tests/ethtool_show.sh   | 16 ++++++++--------
>  4 files changed, 33 insertions(+), 16 deletions(-)
> 
> diff --git a/scripts/fuego_board_function_lib.sh
> b/scripts/fuego_board_function_lib.sh
> index 4c29bf3..8ad6618 100644
> --- a/scripts/fuego_board_function_lib.sh
> +++ b/scripts/fuego_board_function_lib.sh
> @@ -48,3 +48,18 @@ exec_service_on_target() {
>          /etc/init.d/$1 $2
>      fi
>  }
> +
> +# detect_active_eth_device
> +#   Detect the name of actived ethernet device
> +# $1: All ethernet device list
> +# returns: name of actived ethernet device
> +detect_active_eth_device() {
> +    for line in $(cat $1)
> +    do
> +        if ethtool $line | grep "baseT" > /dev/null
> +        then
> +            echo "$line"
> +            break
> +        fi
> +    done
> +}
If no Ethernet device is found, then this routine returns the
empty string.

> diff --git a/tests/Functional.ethtool/fuego_test.sh
> b/tests/Functional.ethtool/fuego_test.sh
> index 233d367..6a3e1a8 100644
> --- a/tests/Functional.ethtool/fuego_test.sh
> +++ b/tests/Functional.ethtool/fuego_test.sh
> @@ -5,6 +5,7 @@ function test_pre_check {
> 
>  function test_deploy {
>      put $TEST_HOME/ethtool_test.sh $BOARD_TESTDIR/fuego.$TESTDIR/
> +    put $FUEGO_CORE/engine/scripts/fuego_board_function_lib.sh
> $BOARD_TESTDIR/fuego.$TESTDIR
The 'engine' directory is now deprecated (in the 'next' branch).
I'm fixing these types of paths up until the 1.5 release, but from then
on paths reference fuego_board_function_lib.sh should not include
an 'engine' component.

This would work without a fixup, because I'm leaving a symlink,
but in general I want to move away from using that directory
in Fuego paths.

>      put -r $TEST_HOME/tests $BOARD_TESTDIR/fuego.$TESTDIR/
>  }
> 
> diff --git a/tests/Functional.ethtool/tests/ethtool_driver.sh
> b/tests/Functional.ethtool/tests/ethtool_driver.sh
> index 6f6d576..d7857d7 100644
> --- a/tests/Functional.ethtool/tests/ethtool_driver.sh
> +++ b/tests/Functional.ethtool/tests/ethtool_driver.sh
> @@ -5,17 +5,18 @@
> 
>  test="driver"
> 
> +. ./fuego_board_function_lib.sh
> +
>  ETHERNET_DEVICE_NAME="have no Ethernet device"
>  ifconfig | cut -d' ' -f1 | sed '/^$/d' > driver_list
These lines should be moved into the library routine.
> 
> -for line in $(cat driver_list)
> -do
> -    if ethtool $line | grep "baseT"
> -    then
> -        ETHERNET_DEVICE_NAME=$line
> -        break
> -    fi
> -done
> +ETHERNET_DEVICE_NAME=$(detect_active_eth_device driver_list)
ETHERNET_DEVICE_NAME can be empty here.
> +
> +if echo $ETHERNET_DEVICE_NAME | grep "have no Ethernet device"
Since we know the exact string for 'not found', we can do a string
compare as a shell internal operation (using the '[' command) rather
than call the external program 'grep'.

This should be:
if [ "$ETHERNET_DEVICE_NAME" = "have no Ethernet device" ]

If you are worried about ETHERNET_DEVICE_NAME being empty
(It shouldn't if you adjust the routine as advised above, but in any event),
you can prevent the shell from having problems by adding a suffix
to both sides of the compare:
if [ "${ETHERNET_DEVICE_NAME}x" = "have no Ethernet devicex"

> +then
> +    echo " -> $test: TEST-FAIL"
> +    exit 1
> +fi
> 
>  if ethtool -i $ETHERNET_DEVICE_NAME | grep driver
>  then
> diff --git a/tests/Functional.ethtool/tests/ethtool_show.sh
> b/tests/Functional.ethtool/tests/ethtool_show.sh
> index 98abe40..9fa8361 100644
> --- a/tests/Functional.ethtool/tests/ethtool_show.sh
> +++ b/tests/Functional.ethtool/tests/ethtool_show.sh
> @@ -5,18 +5,18 @@
> 
>  test="show"
> 
> +. ./fuego_board_function_lib.sh
> +
>  ETHERNET_DEVICE_NAME="have no Ethernet device"
>  ifconfig | cut -d' ' -f1 | sed '/^$/d' > driver_list
> 
> -for line in $(cat driver_list)
> -do
> -    if ethtool $line | grep "baseT"
> -    then
> -        ETHERNET_DEVICE_NAME=$line
> -        break
> -    fi
> -done
> +ETHERNET_DEVICE_NAME=$(detect_active_eth_device driver_list)
> 
> +if echo $ETHERNET_DEVICE_NAME | grep "have no Ethernet device"
Same issues for empty ETHERNET_DEVICE_NAME here.

> +then
> +    echo " -> $test: TEST-FAIL"
> +    exit 1
> +fi
> 
>  if ethtool $ETHERNET_DEVICE_NAME | grep "Settings for"
>  then
> --
> 1.8.3.1

In general I like the approach of putting the routine the board function library,
but you need to be careful not to return an empty string.

Please fix and resubmit.

Thanks!
 -- Tim



More information about the Fuego mailing list