On Sat, Jan 11, 2014 at 08:49:00PM +0900, Hitoshi Mitake wrote: > At Sat, 11 Jan 2014 19:26:05 +0800, > Liu Yuan wrote: > > > > On Sat, Jan 11, 2014 at 08:09:16PM +0900, Hitoshi Mitake wrote: > > > At Sat, 11 Jan 2014 18:34:54 +0900, > > > Hitoshi Mitake wrote: > > > > > > > > At Fri, 10 Jan 2014 18:00:14 +0800, > > > > Liu Yuan wrote: > > > > > > > > > > On Fri, Jan 10, 2014 at 03:20:42PM +0900, Hitoshi Mitake wrote: > > > > > > Reported-by: Marcin Mirosław <marcin at mejor.pl> > > > > > > Signed-off-by: Hitoshi Mitake <mitake.hitoshi at lab.ntt.co.jp> > > > > > > --- > > > > > > tests/functional/common.rc | 14 ++++++++++++-- > > > > > > 1 file changed, 12 insertions(+), 2 deletions(-) > > > > > > > > > > > > diff --git a/tests/functional/common.rc b/tests/functional/common.rc > > > > > > index 3785bae..f1ee432 100644 > > > > > > --- a/tests/functional/common.rc > > > > > > +++ b/tests/functional/common.rc > > > > > > @@ -282,7 +282,7 @@ _valgrind_dog() > > > > > > return $ret > > > > > > } > > > > > > > > > > > > -_start_sheep() > > > > > > +_do_start_sheep() > > > > > > { > > > > > > # ensure that sheep is not running > > > > > > local running=true > > > > > > @@ -304,13 +304,23 @@ _start_sheep() > > > > > > MD_STORE=",$STORE/$1/d0,$STORE/$1/d1,$STORE/$1/d2" > > > > > > fi > > > > > > > > > > > > - $SHEEP $STORE/$1$MD_STORE -z $1 -p $((7000+$1)) -c $DRIVER $SHEEP_OPTIONS $2 > > > > > > + $SHEEP $STORE/$1$MD_STORE -z $2 -p $((7000+$1)) -c $DRIVER $SHEEP_OPTIONS $3 > > > > > > > > > > > > if [ $? != 0 ]; then > > > > > > _die "cannot start sheep $1" > > > > > > fi > > > > > > } > > > > > > > > > > > > +_start_sheep() > > > > > > +{ > > > > > > + _do_start_sheep $1 $1 $2 > > > > > > +} > > > > > > + > > > > > > +_start_sheep_with_zone() > > > > > > +{ > > > > > > + _do_start_sheep $1 $2 $3 > > > > > > +} > > > > > > + > > > > > > > > > > I think it would be better to add a more generic helper such as > > > > > > > > > > _start_sheep_raw() > > > > > { > > > > > $SHEEP $STORE/$1$MD_STORE -p $((7000+$1)) -c $DRIVER $SHEEP_OPTIONS $2 > > > > > } > > > > > > > > > > which allow more flexity, which we can > > > > > > > > > > _start_sheep_raw 1 "-z 1" which explicitly pass options as parameters and no > > > > > need to add burden to users that what is parameter list for this helper. > > > > > > > > I agree. I'll implement it in v2. > > > > > > On the second thought, _start_sheep_raw() seems to not a good > > > approach. If we employ this approach, preparing default values of > > > every parameter is required. And we need to replace the default values > > > with passed ones. It requires needless complexity and I think the zone > > > specific wrapper can be acceptable. > > > > > > > I don't think so. What you want is to add '-z x' to _start_sheep and not affect > > old script. _start_sheep() already well perceived with first one as sheep > > identity and then apending user-defined parameters as string. Based on this > > convention, start a sheep with a specified zone should be: > > > > _start_sheep 1 "-z 1" > > > > I think we can achieve this objective by modifying _start_sheep as following > > > > _start_sheep() > > { > > ... > > if '-z' not specified in the $2, then -z $1 > > If we need to pass other options explicitly in our test scripts, it > would be required. But zone is the only one option which needs be > passed to sheep explicitly. sometimes we pass '-w -j -i' options, which current _start_sheep() handle well because they don't need default value at all. > > BTW, if we write such a generalized _start_sheep(), passing > overwritten options in one string is not enough. e.g. other > options like -w and -j can have the string "-z" in their > parameters. We need to use dictionaly or something else. Do we really > need such a functional _start_sheep() now? > I think no. Let's keep it as simple as possible. Curently only '-z' option has the possibility to be overwritten by parameters, so let's simply check '-z' only Unless we have many options need to be overriten, we don't need to introduce a complex data structure for _start_sheep. Thanks Yuan |