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

<[email protected]> Wed, 1 Sep 2021 19:09:48 +0000
Newsgroups dev.linux.lists.fuego
Message-ID <BYAPR13MB250327274385B075E5526D2FFDCD9@BYAPR13MB2503.namprd13.prod.outlook.com>
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: [email protected] <[email protected]>
> 
> 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: [email protected] <[email protected]>
> Sent: Friday, August 27, 2021 12:28 PM
> To: sangorrin daniel(サンゴリン ダニエル □SWC◯ACT) <[email protected]>
> Cc: [email protected]; nguyen dat tho(TSDV Eng 1) <[email protected]>
> Subject: RE: [PATCH 4/4] rt: search for the binary if the build phase is skipped
> 
> 
> 
> > -----Original Message-----
> > From: [email protected] <[email protected]>
> >
> > Hi Tim,
> >
> > Thanks again for your review.
> > We will fix the patch and re-send.
> >
> > See my comments inline:
> >
> > > -----Original Message-----
> > > From: [email protected] <[email protected]>
> > > Sent: Saturday, August 21, 2021 5:16 AM
> > > To: sangorrin daniel(サンゴリン ダニエル □SWC◯ACT)
> > > <[email protected]>
> > > Cc: [email protected]; nguyen dat tho(TSDV Eng 1)
> > > <[email protected]>
> > > Subject: RE: [PATCH 4/4] rt: search for the binary if the build
> > > phase is skipped
> > >
> > > > -----Original Message-----
> > > > From: Daniel Sangorrin <[email protected]>
> > > >
> > > > From: Nguyen Dat Tho <[email protected]>
> > > >
> > > > 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 <[email protected]>
> > > > Signed-off-by: Daniel Sangorrin <[email protected]>
> > > > ---
> > > >  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