[Fuego] [PATCH 1/6] bsdiff: add test_pre_check for command "bsdiff bspatch"

[Liu, Wenlong]

> -----Original Message-----
> From: Bird, Timothy [mailto:Tim.Bird at sony.com]
> Sent: Friday, November 10, 2017 3:31 AM
> To: Liu, Wenlong/刘 文龙 <liuwl.fnst at cn.fujitsu.com>;
> fuego at lists.linuxfoundation.org
> Subject: RE: [Fuego] [PATCH 1/6] bsdiff: add test_pre_check for command
> "bsdiff bspatch"
> 
> 
> 
> > -----Original Message-----
> > From: Liu, Wenlong on Wednesday, November 08, 2017 11:22 PM
> > > -----Original Message-----
> > > From: Bird, Timothy [mailto:Tim.Bird at sony.com]
> > > > -----Original Message-----
> > > > From: Liu Wenlong on Monday, November 06, 2017 7:43 AM This test
> > > > depends on "bsdiff bspatch" being already installed on the target.
> > > >
> > > > Signed-off-by: Liu Wenlong <liuwl.fnst at cn.fujitsu.com>
> > > > ---
> > > >  engine/tests/Functional.bsdiff/fuego_test.sh | 7 +++++++
> > > >  1 file changed, 7 insertions(+)
> > > >
> > > > diff --git a/engine/tests/Functional.bsdiff/fuego_test.sh
> > > > b/engine/tests/Functional.bsdiff/fuego_test.sh
> > > > index 6ebe3c7..dfc3f80 100755
> > > > --- a/engine/tests/Functional.bsdiff/fuego_test.sh
> > > > +++ b/engine/tests/Functional.bsdiff/fuego_test.sh
> > > > @@ -1,5 +1,12 @@
> > > >  tarball=bsdiff.tar.gz
> > > >
> > > > +function test_pre_check {
> > > > +    is_on_target_path bsdiff PROGRAM_BSDIFF
> > > > +    assert_define PROGRAM_BSDIFF
> > > > +    is_on_target_path bspatch PROGRAM_BSPATCH
> > > > +    assert_define PROGRAM_BSPATCH }
> > > > +
> > >
> > > Looks good.  Thanks.
> > >
> > > I have a question about this interface.  It seems like this will be
> > > a common
> > > pattern:
> > > is_on_target_path, followed by assert_define.
> > >
> > > Would it be useful to have an API that does both operations?
> > > I'm thinking of adding something to the dependency checker, so that
> > > the following declaration in fuego_test.sh would perform the same
> > > functionality:
> > >
> > > NEED_PROGRAMS="bsdiff bspatch"
> > >
> > > What do you think?
> > Yes, to add another way to perform the same functionality is useful.
> > But hope it will not let users misunderstand the relationship between
> > "NEED_PROGRAMS" and "is_on_target".
> >
> > I'll add this function for myself first, and then test on it.
> > Thanks for your review.
> 
> I had some more thoughts on this, that I wanted to share, before you get
> too far into an implementation.
> 
> I originally structured this with is_on_target, specifying a shell
> environment variable name, in anticipation of being able to use these types
> of "detected board features" as dynamic variables
> for the board.   I haven't document the dynamic variables feature
> of Fuego yet, but basically you can set an arbitrary variable on a board,
> programmatically, using ftc.
> 
> Like so: ftc set_var myboard PROGRAM_FOO=/usr/weird/location/foo
> 
> Then, you can get the value of a variable using ftc:
> $ ftc query_board myboard -n PROGRAM_FOO /usr/weird/location/foo
> 
> These are stored in the /fuego-rw/boards directory.  The intent of dynamic
> variables is to allow the system or another test to store information that
> can be used by other tests.  This essentially creates an information cache
> about the board, that is programmatically generated and managed.
> 
> One use case for this to have a "board setup" test, that scans for lots
> of different items, and populates the dynamic variables with values that
> are used by other tests.  Then, a function like is_on_target could check
> the dynamic variables first, to determine if the variable is already known.
> This avoids having to find the value at test runtime.  Some items take time
> to discover (using e.g. 'find' on the target board), and this can help reduce
> the time required to check these conditions.
Yes, agree with you.

> I still need to add code to have the board dynamic variables appear
> automatically in the environment for a test when it is run.  So essentially,
> this feature is not quite complete yet.  But that's the direction I'm
> heading with it.
> 
> Anyway, getting back to the is_on_target/assert_define API.  I think it's
> useful to have an environment variable name assigned to each item that is
> scanned for on the target.  My simple example that I provided doesn't do
> that:
> NEED_PROGRAMS="bsdiff bspatch"
> 
> So I'm not sure this is how I'd like that API to look.  Maybe something
> more like:
> NEED_PROGRAM_BSDIFF="bsdiff"
> NEED_PROGRAM_BSPATCH="bspatch"
> 
> would be a better API.  These would, internally, set and use
> PROGRAM_BSDIFF and PROGRAM_BSPATCH variables, that could be cached from
> run to run, or manipulated and/or read by other tests.
Thanks for your explication.

But I have a question：
The variable name format should be unified?

It might be easy that we can use "PROGRAMS_" as the prefix for each program name.
But actually I don't know how to name for SDK or TARGET library in current Fuego.

Best regards
Liu

> Let me know what you think.
>  -- Tim
> 
>