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

> -----Original Message-----
> From: [email protected] <[email protected]>
> 
> Hi
> 
> I was thinking that it would be nice if we could use a variable to communicate to the test whether the user wants it to run the host binary or
> the one built by Fuego.
> This could be set in the Fuego configuration (fuego-ro/conf/fuego.conf) and then overridden on the specs or the --dynamic-vars option.

I was thinking of having a global configuration item like this as well.  It wouldn't
be hard to add. 
> 
> Does the variable  "use_binary_packages=false" have something to do with this?
No.  As Venkata said, this is related to pre-built Fuego packages, not host-based
packages.

I was thinking of something like:
prefer_host_test_programs=true|false
in fuego.conf.

This would be PREFER_HOST_TEST_PROGRAMS as an environment variable
or dynamic variable.

The effect of the flag would be to just change the search order in
get_program_path from:
 board test dir, alternate dirs., PATH
to
 PATH, alternate dirs., board test dir

I still am not sure where the programs that you plan to install (outside of deploy) will
reside.  It sounds like you plan to put them into $BOARD_TESTDIR/fuego.$TESTDIR,
but that would that mean you always have to set --preclean to false when doing
ftc run-test.

If you are putting them in the provisioned image somewhere else, then it seems
like it would be better to not check $BOARD_TESTDIR/fuego.$TESTDIR first
(although it might not matter because you aren't doing deploy, so the test programs
should not be present in $BOARD_TESTDIR/fuego.$TESTDIR.)

It would be nice to avoid having any path detection logic inside the tests
themselves (after the switch to get_program_path).

Should I whip up an implementation.  I have the successor to get_program_path
done, but would still need to do the config variable handling.

Having said all that, if you don't think you'll set the config variable, I'd rather not
add the implementation for it.  Fuego already has a large number of somewhat
idiosyncratic behaviors, and I'd like to avoid adding more unless there's a real
use case for them.
 -- Tim

> > -----Original Message-----
> > From: Fuego <[email protected]> On Behalf Of [email protected]
> > Sent: Wednesday, September 8, 2021 10:14 AM
> > To: pyla venkata(ＴＳＩＰ) <[email protected]>
> > Cc: [email protected]; nguyen dat tho(ＴＳＤＶ Ｅｎｇ　１) <[email protected]>
> > Subject: Re: [Fuego] [PATCH 4/4] rt: search for the binary if the build phase is skipped
> >
> > > -----Original Message-----
> > > From: [email protected] <[email protected]>
> > >
> > > 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.
> >
> > Path 3 is never checked, and it is not a list of directories to check.  It is the return value, if the program is not found in the other 2 paths,
> > and if it is not specified, then it defaults to:
> > $BOARD_TESTDIR/fuego.$TESTDIR/{program_name}
> >
> > I did this to most closely match the current behavior of Fuego.
> > In the discussion that follows, I'm using the following shorthand:
> >  a)  board test dir = $BOARD_TESTDIR/fuego.$TESTDIR
> >  b) PATH = one of the directories in the target boards PATH variable
> >  c) alternate dirs. = one of the directories specified as an argument to get_program_path (2nd argument)
> >
> > Fuego has several kinds of invocations of programs on the board in fuego_test.sh report functions currently:
> >  - from the board test directory, using a relative path (e.g using 'cd board test dir ; ./program_name')
> >     - these are always for programs that Fuego has deployed, this is by far the most common case
> >  - from the board test directory, using a full path
> >     - these are also always for programs that Fuego has deployed, this is very rare
> >  - from the PATH, using just the program name
> >     - these are always for programs that are already assumed to be on the target (xrandr, bc, ls, time, dd, cat, java, etc.)
> >        - this is also a fairly common case.  In this case, Fuego never intended to install this program.
> >         - if the program is possibly not present, then often there is an assert_has_program in the test_pre_check to
> >           make sure it is present before proceeding.
> >  - from other places
> >     - LTP can be run from a pre-installed location now, but it requires manually configuring the spec with
> >       a HOMEDIR argument (or the board with a FUNCTIONAL_LTP_HOMEDIR argument)
> >
> > > In this situation user cannot skip the installed programs in the PATH and instead use specified path in $2 or $3?
> > That's correct.
> >
> > > 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.
> >
> > That's a good observation.  Given that the current most-common behavior is to run the test program from the board test directory,
> > that should be the default behavior of get_program_path, in order to change the behavior for existing tests as little as possible.
> >
> > Under normal circumstances, if the test deploys a program that was built by Fuego, we want to run that, since it is most likely that the
> > arguments and output will be what the Fuego test is expecting and can work with.  If a different version of the test program is on the
> > board, then it might cause unexpected problems running it and parsing the output.
> >
> > That is, if Fuego has deployed 'dhrystone' (for example) to the board test directory, we probably want to run that instead of one found
> > on the board 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"
> > >  }
> >
> > As get_program_path is currently written, you are right, this gives preference to dhrystone on the PATH instead of the one in the board
> > test dir.
> >
> > >
> > > 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.
> > I don't follow this.  Sorry.
> >
> > > function test_run {
> > > -    report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./dhrystone $BENCHMARK_DHRYSTONE_LOOPS"
> > > +    get_program_path 'dhrystone'  "$BOARD_TESTDIR/fuego.$TESTDIR" "/bin"
> > I'm not sure those arguments match what you are saying.  I'm also not sure where you think the PATH check should occur, in the
> > sequence of checks (if anywhere).
> >
> > > +    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.
> > I like the idea of allowing the spec to indicate an alternate path.
> >
> > >
> > > Please let me know if this make sense, or clarify me if I misunderstood it entirely.
> > You raise a good issue, but I'm not sure I understood exactly the use case you have.
> >
> > When you install the test program (eg. dhrystone) into the system, separately from the Fuego deploy step, are you planning on putting
> > it in a regular system directory such as /usr/bin, or are you planning on putting it in the board test dir?
> >
> > Also, if the base distribution on the board already has the program, do you want the Fuego test to run the program you installed, or the
> > one from the base distribution?
> >
> > In any event, let me describe some use cases and some search order precedences, and see if they cover the cases we're interested in -
> > then propose a different implementation of get_program_path.
> >
> > Use case #1 - regular Fuego build, deploy and run
> >   - fuego phasese run: build, deploy, run (and others)
> >  - preferred search preference:
> >     - board test dir
> >     - alternate dir(s) (if specified)
> >     - PATH dir(s)
> >
> >  Use case #2 - Fuego run, without build and deploy
> >    - fuego phases run: only 'run'
> >    - preferred search preference:
> >       - board test dir
> >       - alternate dir(s) (if specified)
> >       - PATH dir(s)
> >
> >  Use case #3 - Fuego test that anticipates that the distro may already have the test program installed
> >     - fuego phases run: only 'run'
> >     - preferred search preference
> >        - from alternate dir
> >        - from PATH
> >        - from board test dir
> >
> > Actually, in case 3, since there is no deploy step, there is no harm to keep the same search order as cases #1 and #2 (board test dir first),
> > as the program should never be found in the board test dir.
> >
> > Please let me know if this is correct.
> >
> > So, I'm inclined to re-write get_program_path to switch the search order to:
> >  - board test dir
> >  - alternate dir
> >  - PATH
> >
> > I find the $BOARD_TESTDIR/fuego.$TESTDIR to be annoying, and I had hoped to avoid using it as an argument.  If it's always the first
> > place searched, then it doesn't need to be specified in the arguments.  If the reference to it could be removed from fuego_test.sh
> > scripts, then it would give the Fuego system the flexibility to change it or use other shell variables in the future (which could be useful.)
> >
> > get_program_path would then have the following arguments:
> >   $1 - program to search for
> >   $2 - alternate dirs. to search (optional, and takes precedence over the PATH)
> >
> > This does not support the case where you want to override running the program from the board test dir (using an alternate location).
> > However, that case can be handled by not using get_program_path, and doing the check directly using something like this:
> > if cmd "test -x $FUNCTIONAL_XXX_PROGRAM_FOO_PATH" ; then
> >     report "$FUCTIONAL_XXX_PROGRAM_FOO_PATH ..."
> > else
> >    ... fall back to some other search path, or report failure ...
> > fi
> >
> > What do you think?
> >  -- Tim
> >
> > >
> > > Thanks,
> > > Venkata.
> > >
> > >
> > > >-----Original Message-----
> > > >From: Fuego <[email protected]> On Behalf Of
> > > >[email protected]
> > > >Sent: 02 September 2021 00:40
> > > >To: nguyen dat tho(ＴＳＤＶ Ｅｎｇ　１) <[email protected]>
> > > >Cc: [email protected]
> > > >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: [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(サンゴリン ダニエル □ＳＷＣ◯ＡＣＴ)
> > > >> <[email protected]>
> > > >> Cc: [email protected]; nguyen dat tho(ＴＳＤＶ Ｅｎｇ　１)
> > > >> <[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(サンゴリン ダニエル □ＳＷＣ◯ＡＣＴ)
> > > >> > > <[email protected]>
> > > >> > > Cc: [email protected]; nguyen dat tho(ＴＳＤＶ Ｅｎｇ　１
> > > >)
> > > >> > > <[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
> > > >
> > > >_______________________________________________
> > > >Fuego mailing list
> > > >[email protected]
> > > >https://lists.linuxfoundation.org/mailman/listinfo/fuego
> > _______________________________________________
> > Fuego mailing list
> > [email protected]
> > https://lists.linuxfoundation.org/mailman/listinfo/fuego