Re: [Fuego] [PATCH 4/4] rt: search for the binary if the build phase is skipped
<[email protected]> Thu, 9 Sep 2021 00:25:24 +0000
| Newsgroups | dev.linux.lists.fuego |
|---|---|
| Message-ID | <TYYPR01MB67294A59B292E62D66F59F49D0D59@TYYPR01MB6729.jpnprd01.prod.outlook.com> |
Hi Tim, > -----Original Message----- > From: [email protected] <[email protected]> [..] > 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 Exactly what I was thinking. > 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. No, no. This must be a misunderstanding. We install the tests into the typical path (/usr/loca/bin etc). [...] > 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. I think the main use case for the variable is to select what binary to run when you have 2 (one in the system path and another one in $BOARD_TESTDIR/fuego.$TESTDIR. We do not have this use case (we only install one test). I think that Venkata's main concern was on the order of the search (first system path and second $BOARD_TESTDIR/fuego.$TESTDIR). Perhaps modifying the order would be a good idea. However, this will only affect users who have 2 test binaries on the same host and that is not our use case. The main use case for having 2 binaries on the same board would be comparing results between 2 versions of the same test. Or at least that is my guess. Concluding: - we don't need the global variable - perhaps it's better to change the search order (first fuego's and second the host) Thanks, Daniel > > > -----Original Message----- > > > From: Fuego <[email protected]> On Behalf Of > > > [email protected] > > > Sent: Wednesday, September 8, 2021 10:14 AM > > > To: pyla venkata(TSIP) <[email protected]> > > > Cc: [email protected]; nguyen dat tho(TSDV Eng 1) > > > <[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(TSDV Eng 1) <[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(サンゴリン ダニエル □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 > > > > > > > > > >_______________________________________________ > > > > >Fuego mailing list > > > > >[email protected] > > > > >https://lists.linuxfoundation.org/mailman/listinfo/fuego > > > _______________________________________________ > > > Fuego mailing list > > > [email protected] > > > https://lists.linuxfoundation.org/mailman/listinfo/fuego