[U-Boot] [PATCH] test:dfu: Add test script for testing DFU regression

[Simon Glass]

Hi Lukasz,

On 22 May 2014 00:43, Lukasz Majewski <l.majewski at samsung.com> wrote:
> This commit adds test script for testing if any commit has introduced
> regression to the DFU.
>
> It uses md5 to test if sent and received file is correct.
> The test detailed description is available at DESCRIPTION.TXT file.
>
> Signed-off-by: Lukasz Majewski <l.majewski at samsung.com>
> ---
>  test/dfu/DESCRIPTION.TXT    |   27 ++++++++++++++

This sounds like an important and useful test, but I have a few
comments (coming from someone with little knowledge of dfu)

Would README be a better name?

>  test/dfu/dfu_gadget_test.sh |   86 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 113 insertions(+)
>  create mode 100644 test/dfu/DESCRIPTION.TXT
>  create mode 100755 test/dfu/dfu_gadget_test.sh
>
> diff --git a/test/dfu/DESCRIPTION.TXT b/test/dfu/DESCRIPTION.TXT
> new file mode 100644
> index 0000000..48da06b
> --- /dev/null
> +++ b/test/dfu/DESCRIPTION.TXT
> @@ -0,0 +1,27 @@
> +DFU TEST CASE DESCRIPTION:
> +
> +For running test script one needs to create:
> +./log and ./bkp
> +
> +One also need to generate with dd following files:
> +
> +dat_*
> +
> +(e.g. dat_127B.img  dat_128B.img  dat_129B.img  dat_33M.img  dat_4095B.img
> +dat_4096B.img  dat_4097B.img  dat_63B.img  dat_64B.img  dat_65B.img
> +dat_960.img  dat_97M.img)

How do you do that? Is this intended for use with a particular board?
I think you are missing some info here.

> +
> +The size is only important (so if=/dev/urandom), especially corner cases of
> +DFU EP0's size of packet (64B). This is very helpful for fixing UDC driver
> +regressions.
> +
> +Moreover on a target device the "dfu_alt_info" env variable should be extended
> +to have "dfu_test.bin fat 0 6;" \ entry [1].

What is [1] ?

> +One can use fat, ext4 or any other supported file system.
> +
> +As a prerequisite one must also create proper partition with file system on it.
> +
> +Example usage:
> +./dfu_gadget_test.sh 11
> +
> +where 11 is the mumber of alt setting corresponding to entry [1].
> \ No newline at end of file

I'm really none-the-wiser as to how this works - I think you need to
add a bit more detail.

> diff --git a/test/dfu/dfu_gadget_test.sh b/test/dfu/dfu_gadget_test.sh
> new file mode 100755
> index 0000000..ebde2ff
> --- /dev/null
> +++ b/test/dfu/dfu_gadget_test.sh
> @@ -0,0 +1,86 @@
> +#! /bin/bash
> +set -e # any command return not equal to zero
> +clear
> +
> +DIR=./
> +SUFFIX=img
> +RCV_DIR=rcv/
> +LOG_FILE=./log/log-`date +%d-%m-%Y_%H-%M-%S`
> +
> +cleanup () {
> +    rm -rf $RCV_DIR
> +}
> +
> +die () {
> +       printf "   \33[31m FAILED \33[0m \n"

Perhaps define $RED or whatever at the top and use those here?

> +       cleanup
> +       exit 1
> +}
> +
> +calculate_md5sum () {
> +    MD5SUM=`md5sum $1`
> +    MD5SUM=`echo $MD5SUM | cut -d ' ' -f1`
> +    echo "md5sum:"$MD5SUM
> +}
> +
> +dfu_test_file () {

I suggest putting the argument in a variable here, instead of using $1
throughout the function.

> +    printf "\33[32m========================================================================================= \33[0m\n"
> +    printf "File:\33[32m %s \33[0m\n" $1
> +
> +    # dfu-util -D $1 -a $TARGET_ALT_SETTING > /dev/null 2>&1

Remove this line?

> +    dfu-util -D $1 -a $TARGET_ALT_SETTING >> $LOG_FILE 2>&1 || die $?
> +
> +    echo -n "TX: "
> +    calculate_md5sum $1
> +
> +    MD5_TX=$MD5SUM
> +
> +    N_FILE=$DIR$RCV_DIR${1:2}"_rcv"
> +
> +    # dfu-util -U $N_FILE -a $TARGET_ALT_SETTING > /dev/null 2>&1

And again

> +    dfu-util -U $N_FILE -a $TARGET_ALT_SETTING >> $LOG_FILE 2>&1 || die $?
> +
> +    echo -n "RX: "
> +    calculate_md5sum $N_FILE
> +    MD5_RX=$MD5SUM
> +
> +    if [ "$MD5_TX" == "$MD5_RX" ]
> +       then
> +       printf "   \33[32m -------> OK \33[0m \n"
> +       else

outdent

> +       printf "   \33[31m -------> FAILED \33[0m \n"
> +       cleanup
> +       exit 1
> +    fi
> +
> +#    echo $N_FILE
> +}
> +
> +printf "\33[32m========================================================================================= \33[0m\n"
> +echo "DFU EP0 transmission test program"
> +echo "Trouble shoot -> disable DBG (even the KERN_DEBUG) in the UDC driver"
> +echo "@ -> TRATS # dfu mmc 0"
> +mkdir -p $RCV_DIR
> +touch $LOG_FILE
> +
> +if [ $# -eq 0 ]
> +then
> +       printf "   \33[31m Please pass alt setting number!!  \33[0m \n"
> +       exit 0
> +fi
> +
> +TARGET_ALT_SETTING=$1
> +
> +if [ -n "$2" ]
> +then
> +       dfu_test_file $2
> +else
> +       for file in $DIR*.$SUFFIX
> +       do
> +           dfu_test_file $file
> +       done
> +fi
> +
> +cleanup
> +
> +exit 0
> \ No newline at end of file

Why not?

> --
> 1.7.10.4
>

Regards,
Simon