Re: [Fuego] [PATCH 4/4] rt: search for the binary if the build phase is skipped

[<Venkata.Pyla@toshiba-tsip.com>]

Hi Tim,

I have few questions on the below implementation for 'get_program_path'

It looks like the function 'get_program_path' always first checks the program is installed in 'PATH', then it checks in path $2 and then $3.
In this situation user cannot skip the installed programs in the PATH and instead use specified path in $2 or $3?

It is not our use case anyway to skip the programs installed in PATH, rather we want to use the installed programs in the PATH variable, 
But I worried if I use 'get_program_path' in the fuego_test.sh, it will always checks the program present in PATH first and then specified in $2 or $3,
this will break for the users who want to use program that is compiled and deployed stages of fuego and also when the program is installed in the PATH.
e.g:
 function test_run {
-    report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./dhrystone $BENCHMARK_DHRYSTONE_LOOPS"
+    get_program_path 'dhrystone'
+    report "$PROGRAM_DHRYSTONE $BENCHMARK_DHRYSTONE_LOOPS"
 }

Instead I am thinking something like this, the function 'get_program_path' should first check the path specified in $2 (where the fuego build and deploy stage are performed) and, if not found then check in path $3 (where fuego build and deploy stages are skipped), this can be made default behaviour inside 'get_program_path' function and user can change if he want by passing different paths.
function test_run {
-    report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./dhrystone $BENCHMARK_DHRYSTONE_LOOPS"
+    get_program_path 'dhrystone'  "$BOARD_TESTDIR/fuego.$TESTDIR" "/bin"
+    report "$PROGRAM_DHRYSTONE $BENCHMARK_DHRYSTONE_LOOPS"
 }

Also the 'get_program_path' function can be called inside the report function with default behaviour, and then path for $3 variable can be modified by taking from test spec variable.
With this one can change the test behaviour to use the test program from specified path in the test spec itself.

Please let me know if this make sense, or clarify me if I misunderstood it entirely.

Thanks,
Venkata.


>-----Original Message-----
>From: Fuego <fuego-bounces@lists.linuxfoundation.org> On Behalf Of
>Tim.Bird@sony.com
>Sent: 02 September 2021 00:40
>To: nguyen dat tho(ＴＳＤＶ Ｅｎｇ　１) <tho1.nguyendat@toshiba.co.jp>
>Cc: fuego@lists.linuxfoundation.org
>Subject: Re: [Fuego] [PATCH 4/4] rt: search for the binary if the build phase is
>skipped
>
>OK - I have completed work on a new function to support the feature you desire.
>
>It is called 'get_program_path', and it is documented here:
>http://fuegotest.org/wiki/function_get_program_path
>
>Please check out this function, and see if it does what you would like.
>I have also created a test (Functional.fuego_function_gpp_check) that tests
>get_program_path, to verify that it works correctly.
>Please try this test in your environment and let me know if you encounter any
>problems.
>
>Also, please note that I have added additional functionality to the report and
>report_append functions, to cd to the test directory on the board
>($BOARD_TESTDIR/fuego.$TESTDIR) automatically, as part of every call to those
>functions.
>
>This means that we can remove that 'cd' operation from a lot of
>report() and report_append() calls.
>
>I assume that because the PROGRAM_xxx variable will have a full path, the cd is
>no longer required in tests that use this as well.  I thought it safer, since you
>might be modifying the report() lines for multiple tests, to set the default
>working directory.  If needed, you can now remove the "cd
>$BOARD_TESTDIR/fuego.$TESTDIR" command from any tests you modify.
>
>This solves the problem that your patch originally addressed, but I have a
>nagging feeling that the higher level concept of installing pre-built packages as
>part of board provisioning might be addressed in another way.
>
>Are you familiar with the 'binary packages' feature of Fuego?  There is a tool to
>pre-build binary packages for a board, called fuego-core/scripts/make_cache.sh
>This is used to create a set of binary packages (also called 'target packages')
>that can be installed separate from Fuego test execution.
>
>This is a feature that was under development for some time, and then got put on
>the backburner.  Please see the page:
>http://fuegotest.org/wiki/Target_Packages.
>
>These packages are intended to replace the 'build' and 'deploy' phases of a test,
>when they are used.  The package contents are (usually) installed relative to
>$BOARD_TESTDIR/fuego.$TESTDIR, but it is possible to install them somewhere
>else (e.g. globally under say /opt/test/fuego)
>
>One idea for this, was to have pre-built binary packages for tests on a central
>server, so that someone could run Fuego and its tests without having to install a
>toolchain for a board at all.
>
>It sounds like you are doing something similar for integration with LAVA and/or
>kernelci.
>(using pre-installed test programs, and skipping the 'build' and 'deploy' steps.)
>
>I'd like to get more details about what you are doing, to possibly consolidate
>these features and avoid fragmenting the code.
> -- Tim
>
>> -----Original Message-----
>> From: tho1.nguyendat@toshiba.co.jp <tho1.nguyendat@toshiba.co.jp>
>>
>> Dear Tim,
>>
>> > Let me know what you think.  I had hoped to prototype something
>> > today, but ran out of time working on other issues.  I'll see what I
>> > can
>> whip up, Please let me know if you see any problems with this
>> approach.  I think it covers your use case.  You will need to have a separate
>mechanism for skipping the build and deploy steps for your use case (but Fuego
>supports this with the test phase control options of ftc).
>> It sounds good.
>> When you finished, please let me know. I will update this patch follow your
>idea.
>>
>> Thanks,
>> Tho
>>
>> -----Original Message-----
>> From: Tim.Bird@sony.com <Tim.Bird@sony.com>
>> Sent: Friday, August 27, 2021 12:28 PM
>> To: sangorrin daniel(サンゴリン ダニエル □ＳＷＣ◯ＡＣＴ)
>> <daniel.sangorrin@toshiba.co.jp>
>> Cc: fuego@lists.linuxfoundation.org; nguyen dat tho(ＴＳＤＶ Ｅｎｇ　１)
>> <tho1.nguyendat@toshiba.co.jp>
>> Subject: RE: [PATCH 4/4] rt: search for the binary if the build phase
>> is skipped
>>
>>
>>
>> > -----Original Message-----
>> > From: daniel.sangorrin@toshiba.co.jp
>> > <daniel.sangorrin@toshiba.co.jp>
>> >
>> > Hi Tim,
>> >
>> > Thanks again for your review.
>> > We will fix the patch and re-send.
>> >
>> > See my comments inline:
>> >
>> > > -----Original Message-----
>> > > From: Tim.Bird@sony.com <Tim.Bird@sony.com>
>> > > Sent: Saturday, August 21, 2021 5:16 AM
>> > > To: sangorrin daniel(サンゴリン ダニエル □ＳＷＣ◯ＡＣＴ)
>> > > <daniel.sangorrin@toshiba.co.jp>
>> > > Cc: fuego@lists.linuxfoundation.org; nguyen dat tho(ＴＳＤＶ Ｅｎｇ　１
>)
>> > > <tho1.nguyendat@toshiba.co.jp>
>> > > Subject: RE: [PATCH 4/4] rt: search for the binary if the build
>> > > phase is skipped
>> > >
>> > > > -----Original Message-----
>> > > > From: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
>> > > >
>> > > > From: Nguyen Dat Tho <tho1.nguyendat@toshiba.co.jp>
>> > > >
>> > > > ftc has the ability to skip the build phase to use a previously
>> > > > built binary or a binary installed on the board's file system
>> > > > (e.g. apt-get install rt-tests).
>> > > >
>> > > > Signed-off-by: Nguyen Dat Tho <tho1.nguyendat@toshiba.co.jp>
>> > > > Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
>> > > > ---
>> > > >  tests/Benchmark.cyclictest/fuego_test.sh  | 7 ++++++-
>> > > >  tests/Benchmark.hackbench/fuego_test.sh   | 7 ++++++-
>> > > >  tests/Benchmark.migratetest/fuego_test.sh | 7 ++++++-
>> > > >  tests/Benchmark.pmqtest/fuego_test.sh     | 8 +++++++-
>> > > >  tests/Benchmark.ptsematest/fuego_test.sh  | 8 +++++++-
>> > > > tests/Benchmark.signaltest/fuego_test.sh  | 7 ++++++-
>> > > > tests/Benchmark.sigwaittest/fuego_test.sh | 7 ++++++-
>> > > > tests/Benchmark.svsematest/fuego_test.sh  | 7 ++++++-
>> > > >  tests/Functional.pi_tests/fuego_test.sh   | 7 ++++++-
>> > > >  9 files changed, 56 insertions(+), 9 deletions(-)
>> > > >
>> > > > diff --git a/tests/Benchmark.cyclictest/fuego_test.sh
>> > > > b/tests/Benchmark.cyclictest/fuego_test.sh
>> > > > index 74d9d24..e7070dd 100755
>> > > > --- a/tests/Benchmark.cyclictest/fuego_test.sh
>> > > > +++ b/tests/Benchmark.cyclictest/fuego_test.sh
>> > > > @@ -24,5 +24,10 @@ function test_deploy {  }
>> > > >
>> > > >  function test_run {
>> > > > -    report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./cyclictest
>$BENCHMARK_CYCLICTEST_PARAMS"
>> > > > +    if [ -f $BOARD_TESTDIR/fuego.$TESTDIR/cyclictest ]; then
>> > > This test ([ -f) won't work on a remote board.  This is running on
>> > > the host machine, which has a different filesystem than the device under
>test, unless you are running using the 'local' TRANSPORT.
>> > >
>> > > There should be a cmd() in here somewhere to perform this test on the
>board's filesystem.
>> >
>> > Yes, you are totally right. Sorry for not catching that problem during my
>review.
>> >
>> > > I'm not sure I follow this.  What does the test_deploy() function look like?
>> > > Under what circumstances would cyclictest not be present in the boards
>$BOARD_TESTDIR/fuego.$TESTDIR directory?
>> >
>> > We want to run fuego natively because it is easier to run it on certain
>scenarios such as LAVA board farms and custom cloud images.
>> > We also want to use packaged versions of the tests and do not store
>> > any test tarball in our repositories. So the OS image will already have the
>tests there and therefore we will skip the build/deploy phases.
>> >
>> > > It seems like this code should be coupled with code that detects
>> > > if the program is already present, and if so 1) avoids deploying
>> > > it and
>> >
>> > We are skipping the build and deploy phase on purpose from ftc.
>> >
>> > > 2) then uses it for the test.
>> > >
>> > > Something like this:
>> > > test_deploy {
>> > >   is_on_target_path cyclictest PROGRAM_CYCLICTEST
>> > >   if [ -z "$PROGRAM_CYCLICTEST" ] ; then
>> > >      put cyclictest $BOARD_TESTDIR/fuego.$TESTDIR
>> > >      PROGRAM_CYCLICTEST=$BOARD_TESTDIR/fuego.$TESTDIR/cyclictest
>> > >  fi
>> > >   export PROGRAM_CYCLICTEST
>> > > }
>> > >
>> > > and then in test_run:
>> > >   if [ -n "$PROGRAM_CYCLICTEST" ] ; then
>> > >     report "$PROGRAM_CYCLICTEST $BENCHMARK_CYCLICTEST_PARAMS"
>> > >   else
>> > >     abort_job "cyclictest is not found on the target"
>> > >   fi
>> > >
>> > > Maybe I'm not understanding the use case here.  Is this for
>> > > running a pre-existing program on the board, or a program that was
>> > > already placed in the BOARD_TESTDIR directory (e.g. from a
>> > > previous run of
>> the test)?
>> >
>> > A pre-existing program on the board (e.g. previously installed).
>> >
>> > > If we have this pattern a lot, then maybe it would make sense to make the
>optional deploy code  a core function, like:
>> > >    put_if_program_not_present cyclictest
>> >
>> > We are skipping the build/deploy phase using ftc. What do you think?
>>
>> OK, based on that, I think I'd like to structure this as follows:
>>
>> I'd like to add a new core function called 'get_program_path'
>>
>> The intent is that code in a test_run function can use that to find
>> the correct path to execute the test program (possibly based on per-lab
>policy).
>>
>> The sequence in test_run function would look like this:
>>
>>   get_program_path cyclictest
>>   report "$PROGRAM_CYCLICTEST $ARGS"
>>
>> There would be no conditionals in the code for individual tests - just this
>additional call to get_program_path.
>>
>> The 'get_program_path' function would have the following functionality:
>> (in pseudo-code, with 'cyclictest' used as a placeholder name for an arbitrary
>program name):
>>
>> if is_on_target_path cyclictest ; then
>>    PROGRAM_CYCLICTEST=(the target path found) elsif cmd "test -f
>$BOARD_TESTDIR/fuego.$TESTDIR/cyclictest" ; then
>>    PROGRAM_CYCLICTEST=$BOARD_TESTDIR/fuego.$TESTDIR/cyclictest
>> else
>>    abortjob "cyclictest was not found"
>>
>> If the function succeeds, then it sets the value of PROGRAM_CYCLICTEST to the
>path on the board to be used to execute cyclictest.
>>
>> This should handle the following cases:
>>  - cyclictest is on the board (on the PATH) from external
>> pre-installation (outside of Fuego)
>>  - cyclictest is on the board in the test directory (from Fuego's deploy, or from
>>   a previous test invocation that did not remove the test directory)
>>
>> The function get_program_path could be extended in the future to
>> implement other program search policies.  The policy above is "use
>> test program already on the board, if found, then the one installed by
>> Fuego."  Other policies might be "use the test program installed by
>> Fuego, then one already on the board, if present", or "let the report() function
>figure out if the program is present or not in the test directory" (this is Fuego's
>current policy).  But extending this to support other policies (and adding the test
>program search policy to the fuego.conf file) will be left for a future exercise.
>>
>> Let me know what you think.  I had hoped to prototype something today,
>> but ran out of time working on other issues.  I'll see what I can whip
>> up, Please let me know if you see any problems with this approach.  I think it
>covers your use case.  You will need to have a separate mechanism for skipping
>the build and deploy steps for your use case (but Fuego supports this with the
>test phase control options of ftc).
>>  -- Tim
>
>_______________________________________________
>Fuego mailing list
>Fuego@lists.linuxfoundation.org
>https://lists.linuxfoundation.org/mailman/listinfo/fuego