[sheepdog] [PATCH 1/4] tests/fucntional: add a function for invoking sheep with zone

[Hitoshi Mitake]

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.

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?

Thanks,
Hitoshi