All of lore.kernel.org
 help / color / mirror / Atom feed
From: Cyril Hrubis <chrubis@suse.cz>
To: ltp@lists.linux.it
Subject: [LTP] [RFC PATCH 1/2] lib/tst_test.sh: TST_TESTFUNC_DATA and TST_TESTFUNC_DATA_IFS
Date: Wed, 16 May 2018 14:50:09 +0200	[thread overview]
Message-ID: <20180516125008.GA544@rei> (raw)
In-Reply-To: <20180516123924.11047-2-pvorel@suse.cz>

Hi!
> This is specific only for shell.
> 
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> If we accept it, it's a question if pass iteration number as well
> (and if yes, maybe it should be in $1 and data in $2).
> 
> Kind regards,
> Petr
> ---
>  doc/test-writing-guidelines.txt | 23 +++++++++++++++++++++++
>  testcases/lib/tst_test.sh       | 20 ++++++++++++++++++--
>  2 files changed, 41 insertions(+), 2 deletions(-)
> 
> diff --git a/doc/test-writing-guidelines.txt b/doc/test-writing-guidelines.txt
> index 8e405a034..2184f6365 100644
> --- a/doc/test-writing-guidelines.txt
> +++ b/doc/test-writing-guidelines.txt
> @@ -1485,6 +1485,29 @@ Otherwise, if '$TST_CNT' is set but there is no '$\{TST_TESTFUNC\}1', etc.,
>  the '$TST_TESTFUNC' is executed '$TST_CNT' times and the test number is passed
>  to it in the '$1'.
>  
> +[source,sh]
> +-------------------------------------------------------------------------------
> +#!/bin/sh
> +#
> +# Example test with tests using TST_TESTFUNC_DATA
> +#
> +
> +TST_TESTFUNC=do_test
> +TST_TESTFUNC_DATA="foo bar"

Why not just TST_TEST_DATA and TST_TEST_DATA_IFS?

> +. tst_test.sh
> +
> +do_test()
> +{
> +	1) tst_res TPASS "Test passed with data '$1'";;
> +}
> +
> +tst_run
> +-------------------------------------------------------------------------------
> +
> +It's possible to pass data for function with '$TST_TESTFUNC_DATA'. Optional
> +'$TST_TESTFUNC_DATA_IFS' variable is supported to set '$IFS' for splitting.
> +'$TST_TESTFUNC_DATA' cannot be mixed with '$TST_CNT'.
> +
>  2.3.2 Library variables
>  ^^^^^^^^^^^^^^^^^^^^^^^
>  
> diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
> index 8d49d34b6..31bf51e10 100644
> --- a/testcases/lib/tst_test.sh
> +++ b/testcases/lib/tst_test.sh
> @@ -246,7 +246,7 @@ tst_rescmp()
>  
>  tst_run()
>  {
> -	local tst_i
> +	local tst_i ifs

The variables here has to be still prefixed with tst_, even though they
are local these are still defined in the $TST_TESTFUNC.

>  	if [ -n "$TST_TEST_PATH" ]; then
>  		for tst_i in $(grep TST_ "$TST_TEST_PATH" | sed 's/.*TST_//; s/[="} \t\/:`].*//'); do
> @@ -255,7 +255,7 @@ tst_run()
>  			OPTS|USAGE|PARSE_ARGS|POS_ARGS);;
>  			NEEDS_ROOT|NEEDS_TMPDIR|NEEDS_DEVICE|DEVICE);;
>  			NEEDS_CMDS|NEEDS_MODULE|MODPATH|DATAROOT);;
> -			IPV6);;
> +			IPV6|TESTFUNC_DATA|TESTFUNC_DATA_IFS);;
>  			*) tst_res TWARN "Reserved variable TST_$tst_i used!";;
>  			esac
>  		done
> @@ -364,6 +364,18 @@ tst_run()
>  					TST_COUNT=$((TST_COUNT+1))
>  				done
>  			fi
> +		elif [ -n "$TST_TESTFUNC_DATA" ]; then
> +			if [ -n "$TST_TESTFUNC_DATA_IFS" ]; then
> +				ifs="$IFS"
> +				IFS="$TST_TESTFUNC_DATA_IFS"
> +			fi
> +			for tst_i in $TST_TESTFUNC_DATA; do
> +				[ -n "$ifs" ] && IFS="$ifs"
> +				local res=$(tst_resstr)
> +				$TST_TESTFUNC $tst_i
> +				tst_rescmp "$res"
> +				TST_COUNT=$((TST_COUNT+1))
> +			done

I do not like redefing IFS, it's really ugly.

What about something as:

DATA="foo:bar:d dd"
DELIM=":"
i=1
while true; do
        param="$(echo "$DATA" | cut -d$DELIM -f$i)"
        [ -z "$param" ] && break;
        echo "$param"
        i=$((i+1))
done

>  		else
>  			local res=$(tst_resstr)
>  			$TST_TESTFUNC
> @@ -400,6 +412,10 @@ if [ -z "$TST_NO_DEFAULT_RUN" ]; then
>  		tst_brk TBROK "TST_TESTFUNC is not defined"
>  	fi
>  
> +	if [ -n "$TST_CNT" -a -n "$TST_TESTFUNC_DATA" ]; then
> +		tst_brk TBROK "TST_CNT cannot be mixed with TST_TESTFUNC_DATA"
> +	fi

I'm not sure about this, why cannot each of the functions take
TST_TESTFUNC_DATA parameters?

I.e. we may have two test functions that take exaclty the same array of
parameters, setting TST_CNT=2 and TST_TESTFUNC_DATA may make sense...

>  	if [ -n "$TST_CNT" ]; then
>  		if ! tst_is_int "$TST_CNT"; then
>  			tst_brk TBROK "TST_CNT must be integer"

-- 
Cyril Hrubis
chrubis@suse.cz

  reply	other threads:[~2018-05-16 12:50 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-16 12:39 [LTP] [RFC PATCH 0/2] Shell API: TST_TESTFUNC_DATA Petr Vorel
2018-05-16 12:39 ` [LTP] [RFC PATCH 1/2] lib/tst_test.sh: TST_TESTFUNC_DATA and TST_TESTFUNC_DATA_IFS Petr Vorel
2018-05-16 12:50   ` Cyril Hrubis [this message]
2018-05-16 13:42     ` Petr Vorel
2018-05-16 14:01       ` Petr Vorel
2018-05-16 12:39 ` [LTP] [RFC PATCH 2/2] net/{stress, virt}: Use " Petr Vorel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180516125008.GA544@rei \
    --to=chrubis@suse.cz \
    --cc=ltp@lists.linux.it \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.