[Fuego] fixes for the linaro and rt tests

[Fuego] fixes for the linaro and rt tests

[Daniel Sangorrin]

Hi Tim

Please consider merging these patches.

[PATCH 1/4] linaro: localhost does not require ssh

This ones is self explanatory (no SSH if you use the local board)

[PATCH 2/4] linaro: update to python3

This one uses pip3 because the upstream python scripts are for
python3 now.

I thought about adding the dependencies to the Docker install
scripts, but since the requirements are in the upstream repository
I thought it is fine leaving them as they are (you can always
install them in advance and then skip this phase)

[PATCH 3/4] hackbench: fix the chart config file

I think this was some copy paste file, which is fixed now.

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

This are all very similar fixes where we enable support for
running binaries that already exist on the board's file system.
For example, because you install rt-tests via apt-get and then
you skipped the Fuego build phase.

Let me know what you think!

Thanks,
Daniel

[Fuego] [PATCH 1/4] linaro: localhost does not require ssh

[Daniel Sangorrin]

From: Nguyen Dat Tho <tho1.nguyendat@toshiba.co.jp>

When running the test on localhost ssh is not required

Signed-off-by: Nguyen Dat Tho <tho1.nguyendat@toshiba.co.jp>
Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
---
 tests/Functional.linaro/fuego_test.sh | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/tests/Functional.linaro/fuego_test.sh b/tests/Functional.linaro/fuego_test.sh
index 86d3c43..677ab49 100755
--- a/tests/Functional.linaro/fuego_test.sh
+++ b/tests/Functional.linaro/fuego_test.sh
@@ -19,7 +19,10 @@ function test_pre_check {
     # vi ~/.ssh/config
     #  Host 192.167.1.99 <- replace with your boards ip address ($IPADDR)
     #    IdentityFile ~/.ssh/bbb_id_rsa
-    assert_define SSH_KEY "Please setup SSH_KEY on your board file (fuego-ro/boards/$NODE_NAME.board)"
+    # [Note] when running on localhost SSH_KEY is not required
+    if [ $IPADDR != "127.0.0.1" ]; then
+        assert_define SSH_KEY "Please setup SSH_KEY on your board file (fuego-ro/boards/$NODE_NAME.board)"
+    fi
 }
 
 function test_build {
@@ -57,7 +60,12 @@ function test_run {
         SKIPFLAG=""
     fi
 
-    test-runner -o ${LOGDIR} $test_or_plan_flag ${REPO_PATH}/$yaml_file $PARAMS -g $LOGIN@$IPADDR $SKIPFLAG -e
+    # SSH is not required when running on localhost
+    if [ $IPADDR != "127.0.0.1" ]; then
+        test-runner -o ${LOGDIR} $test_or_plan_flag ${REPO_PATH}/$yaml_file $PARAMS -g $LOGIN@$IPADDR $SKIPFLAG -e
+    else
+        test-runner -o ${LOGDIR} $test_or_plan_flag ${REPO_PATH}/$yaml_file $PARAMS $SKIPFLAG -e
+    fi
 }
 
 # FIXTHIS: the log directory is populated with a copy of the whole repository, clean unnecessary files
-- 
2.17.1

[Fuego] [PATCH 2/4] linaro: update to python3

[Daniel Sangorrin]

From: Nguyen Dat Tho <tho1.nguyendat@toshiba.co.jp>

The upstream linaro repository[1] now uses python3 so
use pip3 to install the requirements

[1] https://github.com/Linaro/test-definitions

Signed-off-by: Nguyen Dat Tho <tho1.nguyendat@toshiba.co.jp>
Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
---
 tests/Functional.linaro/fuego_test.sh | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/Functional.linaro/fuego_test.sh b/tests/Functional.linaro/fuego_test.sh
index 677ab49..c775baa 100755
--- a/tests/Functional.linaro/fuego_test.sh
+++ b/tests/Functional.linaro/fuego_test.sh
@@ -26,8 +26,10 @@ function test_pre_check {
 }
 
 function test_build {
+    apt-get install python3-pip
     source ./automated/bin/setenv.sh
-    pip install -r $REPO_PATH/automated/utils/requirements.txt --user
+    pip3 install setuptools --user
+    pip3 install -r $REPO_PATH/automated/utils/requirements.txt --user
 }
 
 function test_run {
-- 
2.17.1

Re: [Fuego] [PATCH 2/4] linaro: update to python3

[Tim.Bird]

Ok - I went to apply this today, and I realized I still have some questions...

See inline below.


> -----Original Message-----
> From: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
> 
> From: Nguyen Dat Tho <tho1.nguyendat@toshiba.co.jp>
> 
> The upstream linaro repository[1] now uses python3 so
> use pip3 to install the requirements
> 
> [1] https://github.com/Linaro/test-definitions
> 
> Signed-off-by: Nguyen Dat Tho <tho1.nguyendat@toshiba.co.jp>
> Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
> ---
>  tests/Functional.linaro/fuego_test.sh | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/Functional.linaro/fuego_test.sh b/tests/Functional.linaro/fuego_test.sh
> index 677ab49..c775baa 100755
> --- a/tests/Functional.linaro/fuego_test.sh
> +++ b/tests/Functional.linaro/fuego_test.sh
> @@ -26,8 +26,10 @@ function test_pre_check {
>  }
> 
>  function test_build {
> +    apt-get install python3-pip
>      source ./automated/bin/setenv.sh
> -    pip install -r $REPO_PATH/automated/utils/requirements.txt --user
> +    pip3 install setuptools --user
> +    pip3 install -r $REPO_PATH/automated/utils/requirements.txt --user
>  }

What is the effective user account when this is run in Fuego non-container
mode?

For non-container execution, do you execute build steps as user 'fuego'
or 'jenkins'?  For a container environment, the build steps are performed
as user 'jenkins'.  Will these steps even work, in that environment?

Does this need to be 'sudo apt-get install', to handle the container case?

Do the pip3 operations work at the system level, or at the level of
an individual account.

If a better python3-pip is needed for building, I think this should go
into the installation scripts, unless this is intended to be doing something
on the device under test, and not the host.

This will have a permanent effect on the build environment, for either the
container, or the host machine where Fuego is running.

In your test environment, is the host machine the same as the device
under test, so that these changes are thrown away when the image is
thrown away?  Or is this persistent?

It seems like there should be a check to avoid re-installing these, if they
are already present.  But maybe "apt-get install " or "pip3 install" will
just issue a warning, and there is not harm to executing these if
the packages are already there.

Anyway, as  you can tell, I am confused by this patch.

Overall, I don't object to it.  I just want to understand what's happening
with pip3 installation and setuptools installation at the user level and
system level, for the case where:
 - the test is executing in a Fuego container, with a remote DUT
 - the test is executing in a Fuego container, with a local DUT (DUT=self)
 - the text is executing natively, with a remote DUT
 - the test is executing natively, with a local DUT (DUT=self)

Thanks,
 -- Tim

Re: [Fuego] [PATCH 2/4] linaro: update to python3

[tho1.nguyendat]

Dear Tim,

> If a better python3-pip is needed for building, I think this should go into the installation scripts, unless this is intended to be doing something on the device under test, and not the host.
I will move linaro's dependencies to https://bitbucket.org/fuegotest/fuego/src/master/install-scripts/install-debian-common.sh.
What do you think?

Thanks,
Tho

-----Original Message-----
From: Tim.Bird@sony.com <Tim.Bird@sony.com> 
Sent: Friday, August 27, 2021 12:54 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 2/4] linaro: update to python3

Ok - I went to apply this today, and I realized I still have some questions...

See inline below.


> -----Original Message-----
> From: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
> 
> From: Nguyen Dat Tho <tho1.nguyendat@toshiba.co.jp>
> 
> The upstream linaro repository[1] now uses python3 so use pip3 to 
> install the requirements
> 
> [1] https://github.com/Linaro/test-definitions
> 
> Signed-off-by: Nguyen Dat Tho <tho1.nguyendat@toshiba.co.jp>
> Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
> ---
>  tests/Functional.linaro/fuego_test.sh | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/Functional.linaro/fuego_test.sh 
> b/tests/Functional.linaro/fuego_test.sh
> index 677ab49..c775baa 100755
> --- a/tests/Functional.linaro/fuego_test.sh
> +++ b/tests/Functional.linaro/fuego_test.sh
> @@ -26,8 +26,10 @@ function test_pre_check {  }
> 
>  function test_build {
> +    apt-get install python3-pip
>      source ./automated/bin/setenv.sh
> -    pip install -r $REPO_PATH/automated/utils/requirements.txt --user
> +    pip3 install setuptools --user
> +    pip3 install -r $REPO_PATH/automated/utils/requirements.txt 
> + --user
>  }

What is the effective user account when this is run in Fuego non-container mode?

For non-container execution, do you execute build steps as user 'fuego'
or 'jenkins'?  For a container environment, the build steps are performed as user 'jenkins'.  Will these steps even work, in that environment?

Does this need to be 'sudo apt-get install', to handle the container case?

Do the pip3 operations work at the system level, or at the level of an individual account.

If a better python3-pip is needed for building, I think this should go into the installation scripts, unless this is intended to be doing something on the device under test, and not the host.

This will have a permanent effect on the build environment, for either the container, or the host machine where Fuego is running.

In your test environment, is the host machine the same as the device under test, so that these changes are thrown away when the image is thrown away?  Or is this persistent?

It seems like there should be a check to avoid re-installing these, if they are already present.  But maybe "apt-get install " or "pip3 install" will just issue a warning, and there is not harm to executing these if the packages are already there.

Anyway, as  you can tell, I am confused by this patch.

Overall, I don't object to it.  I just want to understand what's happening with pip3 installation and setuptools installation at the user level and system level, for the case where:
 - the test is executing in a Fuego container, with a remote DUT
 - the test is executing in a Fuego container, with a local DUT (DUT=self)
 - the text is executing natively, with a remote DUT
 - the test is executing natively, with a local DUT (DUT=self)

Thanks,
 -- Tim

Re: [Fuego] [PATCH 2/4] linaro: update to python3

[Tim.Bird]

Tho,

I just did a long-winded answer to Daniel on this topic.  Please see that e-mail.
(But in summary, I'd like to split this change up - some in install and some in the test.)
 -- Tim


> -----Original Message-----
> From: tho1.nguyendat@toshiba.co.jp <tho1.nguyendat@toshiba.co.jp>
> 
> Dear Tim,
> 
> > If a better python3-pip is needed for building, I think this should go into the installation scripts, unless this is intended to be doing
> something on the device under test, and not the host.
> I will move linaro's dependencies to https://bitbucket.org/fuegotest/fuego/src/master/install-scripts/install-debian-common.sh.
> What do you think?
> 
> Thanks,
> Tho
> 
> -----Original Message-----
> From: Tim.Bird@sony.com <Tim.Bird@sony.com>
> Sent: Friday, August 27, 2021 12:54 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 2/4] linaro: update to python3
> 
> Ok - I went to apply this today, and I realized I still have some questions...
> 
> See inline below.
> 
> 
> > -----Original Message-----
> > From: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
> >
> > From: Nguyen Dat Tho <tho1.nguyendat@toshiba.co.jp>
> >
> > The upstream linaro repository[1] now uses python3 so use pip3 to
> > install the requirements
> >
> > [1] https://github.com/Linaro/test-definitions
> >
> > Signed-off-by: Nguyen Dat Tho <tho1.nguyendat@toshiba.co.jp>
> > Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
> > ---
> >  tests/Functional.linaro/fuego_test.sh | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/tests/Functional.linaro/fuego_test.sh
> > b/tests/Functional.linaro/fuego_test.sh
> > index 677ab49..c775baa 100755
> > --- a/tests/Functional.linaro/fuego_test.sh
> > +++ b/tests/Functional.linaro/fuego_test.sh
> > @@ -26,8 +26,10 @@ function test_pre_check {  }
> >
> >  function test_build {
> > +    apt-get install python3-pip
> >      source ./automated/bin/setenv.sh
> > -    pip install -r $REPO_PATH/automated/utils/requirements.txt --user
> > +    pip3 install setuptools --user
> > +    pip3 install -r $REPO_PATH/automated/utils/requirements.txt
> > + --user
> >  }
> 
> What is the effective user account when this is run in Fuego non-container mode?
> 
> For non-container execution, do you execute build steps as user 'fuego'
> or 'jenkins'?  For a container environment, the build steps are performed as user 'jenkins'.  Will these steps even work, in that environment?
> 
> Does this need to be 'sudo apt-get install', to handle the container case?
> 
> Do the pip3 operations work at the system level, or at the level of an individual account.
> 
> If a better python3-pip is needed for building, I think this should go into the installation scripts, unless this is intended to be doing something
> on the device under test, and not the host.
> 
> This will have a permanent effect on the build environment, for either the container, or the host machine where Fuego is running.
> 
> In your test environment, is the host machine the same as the device under test, so that these changes are thrown away when the image is
> thrown away?  Or is this persistent?
> 
> It seems like there should be a check to avoid re-installing these, if they are already present.  But maybe "apt-get install " or "pip3 install"
> will just issue a warning, and there is not harm to executing these if the packages are already there.
> 
> Anyway, as  you can tell, I am confused by this patch.
> 
> Overall, I don't object to it.  I just want to understand what's happening with pip3 installation and setuptools installation at the user level
> and system level, for the case where:
>  - the test is executing in a Fuego container, with a remote DUT
>  - the test is executing in a Fuego container, with a local DUT (DUT=self)
>  - the text is executing natively, with a remote DUT
>  - the test is executing natively, with a local DUT (DUT=self)
> 
> Thanks,
>  -- Tim

Re: [Fuego] [PATCH 2/4] linaro: update to python3

[daniel.sangorrin]

Hi Tim,

Thanks for your review.
See my comments inline.

> -----Original Message-----
> From: Tim.Bird@sony.com <Tim.Bird@sony.com>
> Sent: Friday, August 27, 2021 2:54 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 2/4] linaro: update to python3
> 
> Ok - I went to apply this today, and I realized I still have some questions...
> 
> See inline below.
> 
> 
> > -----Original Message-----
> > From: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
> >
> > From: Nguyen Dat Tho <tho1.nguyendat@toshiba.co.jp>
> >
> > The upstream linaro repository[1] now uses python3 so use pip3 to
> > install the requirements
> >
> > [1] https://github.com/Linaro/test-definitions
> >
> > Signed-off-by: Nguyen Dat Tho <tho1.nguyendat@toshiba.co.jp>
> > Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
> > ---
> >  tests/Functional.linaro/fuego_test.sh | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/tests/Functional.linaro/fuego_test.sh
> > b/tests/Functional.linaro/fuego_test.sh
> > index 677ab49..c775baa 100755
> > --- a/tests/Functional.linaro/fuego_test.sh
> > +++ b/tests/Functional.linaro/fuego_test.sh
> > @@ -26,8 +26,10 @@ function test_pre_check {  }
> >
> >  function test_build {
> > +    apt-get install python3-pip
> >      source ./automated/bin/setenv.sh
> > -    pip install -r $REPO_PATH/automated/utils/requirements.txt --user
> > +    pip3 install setuptools --user
> > +    pip3 install -r $REPO_PATH/automated/utils/requirements.txt
> > + --user
> >  }
> 
> What is the effective user account when this is run in Fuego non-container mode?

We normally install fuego as root and without jenkins. So it would be running with root permissions.
 
> For non-container execution, do you execute build steps as user 'fuego'
> or 'jenkins'?  

We use root. However, you can also install jenkins even when you don't use containers.

https://bitbucket.org/fuegotest/fuego/src/caf37be917f3756980879f32ad044b8f240fbd25/install-native.sh#lines-97

So in that case, it would jenkins.

> For a container environment, the build steps are performed as user 'jenkins'.  Will these steps even work, in that
> environment?

I believe they should work, but we will double check and let you know.
 
> Does this need to be 'sudo apt-get install', to handle the container case?

Probably we can use the Debian packages, we haven't tested that. Also the requirements.txt from Linaro unfortunately does not specify versions.
 
> Do the pip3 operations work at the system level, or at the level of an individual account.

I think that if you run them from jenkins it will install them using jenkins (although i am not sure what happens if jenkins does not have a $HOME folder). We run it with root so i guess they go to /root/.cache/pip or something like that.

> If a better python3-pip is needed for building, I think this should go into the installation scripts, unless this is intended to be doing
> something on the device under test, and not the host.

This was intended to run on the host for host-target configurations, and on the DUT for native configurations (eg. LAVA).

We can put them on the installation scripts, I also thought about this.
The only bad thing is that if the upstream repository changes the requirements.txt file, we might not notice.

Also, it is easier to understand the dependencies of each test if we put them there. Actually this should be handled better in general.

Should I move this to the install scripts then?

In that case, we will install the requirements without --user (or perhaps using Debian packages) .
Regarding the upstream requirements.txt file, we can ignore until the test doesnt work or we can pin the current commit id.
 
> This will have a permanent effect on the build environment, for either the container, or the host machine where Fuego is running.

Correct.

> In your test environment, is the host machine the same as the device under test, so that these changes are thrown away when the
> image is thrown away?  Or is this persistent?

We want to use the LAVA model of always testing on a pristine image. So after running all tests, the image is discarded.

> It seems like there should be a check to avoid re-installing these, if they are already present.  But maybe "apt-get install " or "pip3
> install" will just issue a warning, and there is not harm to executing these if the packages are already there.

Yes, they would only issue a warning. But if we remove test_build and move the dependencies to the install scripts, then we will not need checks at all.

> 
> Anyway, as  you can tell, I am confused by this patch.
> 
> Overall, I don't object to it.  I just want to understand what's happening with pip3 installation and setuptools installation at the user
> level and system level, for the case where:
>  - the test is executing in a Fuego container, with a remote DUT
>  - the test is executing in a Fuego container, with a local DUT (DUT=self)
>  - the text is executing natively, with a remote DUT
>  - the test is executing natively, with a local DUT (DUT=self)

Sorry for the confusion.

We will move the run dependencies to the install scripts. Then, we will test that those combinations work and what user/location is used to install the pip packages.

What do you think?

Thanks,
Daniel

Re: [Fuego] [PATCH 2/4] linaro: update to python3

[Tim.Bird]

> -----Original Message-----
> From: daniel.sangorrin@toshiba.co.jp <daniel.sangorrin@toshiba.co.jp>
> 
> Hi Tim,
> 
> Thanks for your review.
> See my comments inline.
> 
> > -----Original Message-----
> > From: Tim.Bird@sony.com <Tim.Bird@sony.com>
> > Sent: Friday, August 27, 2021 2:54 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 2/4] linaro: update to python3
> >
> > Ok - I went to apply this today, and I realized I still have some questions...
> >
> > See inline below.
> >
> >
> > > -----Original Message-----
> > > From: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
> > >
> > > From: Nguyen Dat Tho <tho1.nguyendat@toshiba.co.jp>
> > >
> > > The upstream linaro repository[1] now uses python3 so use pip3 to
> > > install the requirements
> > >
> > > [1] https://github.com/Linaro/test-definitions
> > >
> > > Signed-off-by: Nguyen Dat Tho <tho1.nguyendat@toshiba.co.jp>
> > > Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
> > > ---
> > >  tests/Functional.linaro/fuego_test.sh | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tests/Functional.linaro/fuego_test.sh
> > > b/tests/Functional.linaro/fuego_test.sh
> > > index 677ab49..c775baa 100755
> > > --- a/tests/Functional.linaro/fuego_test.sh
> > > +++ b/tests/Functional.linaro/fuego_test.sh
> > > @@ -26,8 +26,10 @@ function test_pre_check {  }
> > >
> > >  function test_build {
> > > +    apt-get install python3-pip
> > >      source ./automated/bin/setenv.sh
> > > -    pip install -r $REPO_PATH/automated/utils/requirements.txt --user
> > > +    pip3 install setuptools --user
> > > +    pip3 install -r $REPO_PATH/automated/utils/requirements.txt
> > > + --user
> > >  }
> >
> > What is the effective user account when this is run in Fuego non-container mode?
> 
> We normally install fuego as root and without jenkins. So it would be running with root permissions.
> 
> > For non-container execution, do you execute build steps as user 'fuego'
> > or 'jenkins'?
> 
> We use root. However, you can also install jenkins even when you don't use containers.
> 
> https://bitbucket.org/fuegotest/fuego/src/caf37be917f3756980879f32ad044b8f240fbd25/install-native.sh#lines-97
> 
> So in that case, it would jenkins.
> 
> > For a container environment, the build steps are performed as user 'jenkins'.  Will these steps even work, in that
> > environment?
> 
> I believe they should work, but we will double check and let you know.
> 
> > Does this need to be 'sudo apt-get install', to handle the container case?
> 
> Probably we can use the Debian packages, we haven't tested that. Also the requirements.txt from Linaro unfortunately does not specify
> versions.

What is this requirements.txt file?  I'm not well-versed on pip and how it works (other than that it
downloads packages and resolves dependencies, like most package manager).
Does this requirements.txt have the version of python modules that are needed for Linaro's tools?
Does it indicate the list of python modules required for every test in the test-definitions git
repository, or just the python modules needed for the Linaro core itself?

The reason I ask is to get a sense of the scope of the items that will be installed when this
is executed.

where does REPO_PATH come from in this test?  (I'm going to guess it comes from setenv.sh)

> 
> > Do the pip3 operations work at the system level, or at the level of an individual account.
> 
> I think that if you run them from jenkins it will install them using jenkins (although i am not sure what happens if jenkins does not have a
> $HOME folder). We run it with root so i guess they go to /root/.cache/pip or something like that.
> 
> > If a better python3-pip is needed for building, I think this should go into the installation scripts, unless this is intended to be doing
> > something on the device under test, and not the host.
> 
> This was intended to run on the host for host-target configurations, and on the DUT for native configurations (eg. LAVA).
> 
> We can put them on the installation scripts, I also thought about this.
> The only bad thing is that if the upstream repository changes the requirements.txt file, we might not notice.
I agree. That's very likely.
> 
> Also, it is easier to understand the dependencies of each test if we put them there. Actually this should be handled better in general.
Agreed.  I have struggled with how much to put into the docker container (or host system, for native installs) to
support individual tests, and when exactly to put them there.

If you put every possible needed tool in the container, for all tests, you end up with a pretty bloated container.
If you defer loading the needed tools and packages until a test is actually invoked, it keeps unnecessary
bloat out of the container, but it risks items not finding out about dependency issues until then.

I see pros and cons of both approaches. 

> 
> Should I move this to the install scripts then?

See below for my thinking on this.

> 
> In that case, we will install the requirements without --user (or perhaps using Debian packages) .
> Regarding the upstream requirements.txt file, we can ignore until the test doesnt work or we can pin the current commit id.
> 
> > This will have a permanent effect on the build environment, for either the container, or the host machine where Fuego is running.
> 
> Correct.
> 
> > In your test environment, is the host machine the same as the device under test, so that these changes are thrown away when the
> > image is thrown away?  Or is this persistent?
> 
> We want to use the LAVA model of always testing on a pristine image. So after running all tests, the image is discarded.

So this works pretty well when you're doing the LAVA thing, with Fuego native on the device under test.
It gets dicier when this is run in the docker container.  This test_build potentially changes the contents of
the python3 site-packages for the Jenkins user - which will affect on an ongoing basis all the other 
tests on the system.

In a perfect world, we would sandbox this, and create a separate custom site-packages directory
just for the Linaro tests (maybe inside the /fuego-rw/buildzone/Functional.linaro directory).  This avoid
changing the python modules for other tests.

However, having said that, I don't think we have a ton of tests that use python3 modules, or especially
any esoteric python3 modules, which would create incompatibilities.  So setting up a private site-packages
directory per test seems like overkill.  I reserve the right to do so sometime in the future, though, if we
run into conflicts.

> 
> > It seems like there should be a check to avoid re-installing these, if they are already present.  But maybe "apt-get install " or "pip3
> > install" will just issue a warning, and there is not harm to executing these if the packages are already there.
> 
> Yes, they would only issue a warning. But if we remove test_build and move the dependencies to the install scripts, then we will not need
> checks at all.
> 
> >
> > Anyway, as  you can tell, I am confused by this patch.
> >
> > Overall, I don't object to it.  I just want to understand what's happening with pip3 installation and setuptools installation at the user
> > level and system level, for the case where:
> >  - the test is executing in a Fuego container, with a remote DUT
> >  - the test is executing in a Fuego container, with a local DUT (DUT=self)
> >  - the text is executing natively, with a remote DUT
> >  - the test is executing natively, with a local DUT (DUT=self)
> 
> Sorry for the confusion.
> 
> We will move the run dependencies to the install scripts. Then, we will test that those combinations work and what user/location is used to
> install the pip packages.
> 
> What do you think?

OK - pip3 and (python3) setuptools should be up-to-date at the system level.  So I'd like to see the
installation of those put into the install scripts.  We'll be using more and more python3 code in
the future, so I think this would be good.

For the packages specifically needed by Linaro tests definitions, I'm OK with deferring filling out the
modules from requirements.txt into a local (--user) account, to when the Linaro test is run.

So I think I'd like to see this line stay in the test:
 +    pip3 install -r $REPO_PATH/automated/utils/requirements.txt --user

Is that OK?

 -- Tim

Re: [Fuego] [PATCH 2/4] linaro: update to python3

[Tim.Bird]

FYI - I did some testing here, manually do the apt-get install of python-pip3 into my
old Debian 9 (jessie) Fuego container.

And here's the error message I get from the command
'pip3 install -r /fuego-rw/buildzone/min1.smoke.Functional.linaro-x86_64/automated/utils/requirements.txt --user'

Collecting squad_client>0.5 (from -r /fuego-rw/buildzone/min1.smoke.Functional.linaro-x86_64/automated/utils/requirements.txt (line 5))
  Could not find a version that satisfies the requirement squad_client>0.5 (from -r /fuego-rw/buildzone/min1.smoke.Functional.linaro-x86_64/automated/utils/requirements.txt (line 5)) (from versions: )
No matching distribution found for squad_client>0.5 (from -r /fuego-rw/buildzone/min1.smoke.Functional.linaro-x86_64/automated/utils/requirements.txt (line 5))
----

There's a 'squad-client 0.18' on pypi.org (note the middle dash, not underscore).

Upon investigation, it turns out that the latest squad-client requires python 3.6, and Debian stretch
has python 3.5.  I checked versions back to squad-client 0.1, and all of them require python 3.6,
so the Functional.linaro is not compatible with the default python in the Debian in the
current Fuego docker container.

Ugh.
 -- Tim



> -----Original Message-----
> From: Bird, Tim
> Sent: Friday, August 27, 2021 3:48 PM
> To: daniel.sangorrin@toshiba.co.jp
> Cc: fuego@lists.linuxfoundation.org; tho1.nguyendat@toshiba.co.jp
> Subject: RE: [PATCH 2/4] linaro: update to python3
> 
> > -----Original Message-----
> > From: daniel.sangorrin@toshiba.co.jp <daniel.sangorrin@toshiba.co.jp>
> >
> > Hi Tim,
> >
> > Thanks for your review.
> > See my comments inline.
> >
> > > -----Original Message-----
> > > From: Tim.Bird@sony.com <Tim.Bird@sony.com>
> > > Sent: Friday, August 27, 2021 2:54 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 2/4] linaro: update to python3
> > >
> > > Ok - I went to apply this today, and I realized I still have some questions...
> > >
> > > See inline below.
> > >
> > >
> > > > -----Original Message-----
> > > > From: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
> > > >
> > > > From: Nguyen Dat Tho <tho1.nguyendat@toshiba.co.jp>
> > > >
> > > > The upstream linaro repository[1] now uses python3 so use pip3 to
> > > > install the requirements
> > > >
> > > > [1] https://github.com/Linaro/test-definitions
> > > >
> > > > Signed-off-by: Nguyen Dat Tho <tho1.nguyendat@toshiba.co.jp>
> > > > Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
> > > > ---
> > > >  tests/Functional.linaro/fuego_test.sh | 4 +++-
> > > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/tests/Functional.linaro/fuego_test.sh
> > > > b/tests/Functional.linaro/fuego_test.sh
> > > > index 677ab49..c775baa 100755
> > > > --- a/tests/Functional.linaro/fuego_test.sh
> > > > +++ b/tests/Functional.linaro/fuego_test.sh
> > > > @@ -26,8 +26,10 @@ function test_pre_check {  }
> > > >
> > > >  function test_build {
> > > > +    apt-get install python3-pip
> > > >      source ./automated/bin/setenv.sh
> > > > -    pip install -r $REPO_PATH/automated/utils/requirements.txt --user
> > > > +    pip3 install setuptools --user
> > > > +    pip3 install -r $REPO_PATH/automated/utils/requirements.txt
> > > > + --user
> > > >  }
> > >
> > > What is the effective user account when this is run in Fuego non-container mode?
> >
> > We normally install fuego as root and without jenkins. So it would be running with root permissions.
> >
> > > For non-container execution, do you execute build steps as user 'fuego'
> > > or 'jenkins'?
> >
> > We use root. However, you can also install jenkins even when you don't use containers.
> >
> > https://bitbucket.org/fuegotest/fuego/src/caf37be917f3756980879f32ad044b8f240fbd25/install-native.sh#lines-97
> >
> > So in that case, it would jenkins.
> >
> > > For a container environment, the build steps are performed as user 'jenkins'.  Will these steps even work, in that
> > > environment?
> >
> > I believe they should work, but we will double check and let you know.
> >
> > > Does this need to be 'sudo apt-get install', to handle the container case?
> >
> > Probably we can use the Debian packages, we haven't tested that. Also the requirements.txt from Linaro unfortunately does not specify
> > versions.
> 
> What is this requirements.txt file?  I'm not well-versed on pip and how it works (other than that it
> downloads packages and resolves dependencies, like most package manager).
> Does this requirements.txt have the version of python modules that are needed for Linaro's tools?
> Does it indicate the list of python modules required for every test in the test-definitions git
> repository, or just the python modules needed for the Linaro core itself?
> 
> The reason I ask is to get a sense of the scope of the items that will be installed when this
> is executed.
> 
> where does REPO_PATH come from in this test?  (I'm going to guess it comes from setenv.sh)
> 
> >
> > > Do the pip3 operations work at the system level, or at the level of an individual account.
> >
> > I think that if you run them from jenkins it will install them using jenkins (although i am not sure what happens if jenkins does not have a
> > $HOME folder). We run it with root so i guess they go to /root/.cache/pip or something like that.
> >
> > > If a better python3-pip is needed for building, I think this should go into the installation scripts, unless this is intended to be doing
> > > something on the device under test, and not the host.
> >
> > This was intended to run on the host for host-target configurations, and on the DUT for native configurations (eg. LAVA).
> >
> > We can put them on the installation scripts, I also thought about this.
> > The only bad thing is that if the upstream repository changes the requirements.txt file, we might not notice.
> I agree. That's very likely.
> >
> > Also, it is easier to understand the dependencies of each test if we put them there. Actually this should be handled better in general.
> Agreed.  I have struggled with how much to put into the docker container (or host system, for native installs) to
> support individual tests, and when exactly to put them there.
> 
> If you put every possible needed tool in the container, for all tests, you end up with a pretty bloated container.
> If you defer loading the needed tools and packages until a test is actually invoked, it keeps unnecessary
> bloat out of the container, but it risks items not finding out about dependency issues until then.
> 
> I see pros and cons of both approaches.
> 
> >
> > Should I move this to the install scripts then?
> 
> See below for my thinking on this.
> 
> >
> > In that case, we will install the requirements without --user (or perhaps using Debian packages) .
> > Regarding the upstream requirements.txt file, we can ignore until the test doesnt work or we can pin the current commit id.
> >
> > > This will have a permanent effect on the build environment, for either the container, or the host machine where Fuego is running.
> >
> > Correct.
> >
> > > In your test environment, is the host machine the same as the device under test, so that these changes are thrown away when the
> > > image is thrown away?  Or is this persistent?
> >
> > We want to use the LAVA model of always testing on a pristine image. So after running all tests, the image is discarded.
> 
> So this works pretty well when you're doing the LAVA thing, with Fuego native on the device under test.
> It gets dicier when this is run in the docker container.  This test_build potentially changes the contents of
> the python3 site-packages for the Jenkins user - which will affect on an ongoing basis all the other
> tests on the system.
> 
> In a perfect world, we would sandbox this, and create a separate custom site-packages directory
> just for the Linaro tests (maybe inside the /fuego-rw/buildzone/Functional.linaro directory).  This avoid
> changing the python modules for other tests.
> 
> However, having said that, I don't think we have a ton of tests that use python3 modules, or especially
> any esoteric python3 modules, which would create incompatibilities.  So setting up a private site-packages
> directory per test seems like overkill.  I reserve the right to do so sometime in the future, though, if we
> run into conflicts.
> 
> >
> > > It seems like there should be a check to avoid re-installing these, if they are already present.  But maybe "apt-get install " or "pip3
> > > install" will just issue a warning, and there is not harm to executing these if the packages are already there.
> >
> > Yes, they would only issue a warning. But if we remove test_build and move the dependencies to the install scripts, then we will not need
> > checks at all.
> >
> > >
> > > Anyway, as  you can tell, I am confused by this patch.
> > >
> > > Overall, I don't object to it.  I just want to understand what's happening with pip3 installation and setuptools installation at the user
> > > level and system level, for the case where:
> > >  - the test is executing in a Fuego container, with a remote DUT
> > >  - the test is executing in a Fuego container, with a local DUT (DUT=self)
> > >  - the text is executing natively, with a remote DUT
> > >  - the test is executing natively, with a local DUT (DUT=self)
> >
> > Sorry for the confusion.
> >
> > We will move the run dependencies to the install scripts. Then, we will test that those combinations work and what user/location is used
> to
> > install the pip packages.
> >
> > What do you think?
> 
> OK - pip3 and (python3) setuptools should be up-to-date at the system level.  So I'd like to see the
> installation of those put into the install scripts.  We'll be using more and more python3 code in
> the future, so I think this would be good.
> 
> For the packages specifically needed by Linaro tests definitions, I'm OK with deferring filling out the
> modules from requirements.txt into a local (--user) account, to when the Linaro test is run.
> 
> So I think I'd like to see this line stay in the test:
>  +    pip3 install -r $REPO_PATH/automated/utils/requirements.txt --user
> 
> Is that OK?
> 
>  -- Tim

Re: [Fuego] [PATCH 2/4] linaro: update to python3

[daniel.sangorrin]

Hi Tim,

Thanks for checking.
Probably Squad is not needed to run the tests, only to upload the results to Squad.

Thanks,
Daniel

> -----Original Message-----
> From: Tim.Bird@sony.com <Tim.Bird@sony.com>
> Sent: Saturday, August 28, 2021 9:05 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 2/4] linaro: update to python3
> 
> FYI - I did some testing here, manually do the apt-get install of python-pip3 into my old Debian 9 (jessie) Fuego container.
> 
> And here's the error message I get from the command
> 'pip3 install -r /fuego-rw/buildzone/min1.smoke.Functional.linaro-x86_64/automated/utils/requirements.txt --user'
> 
> Collecting squad_client>0.5 (from -r /fuego-rw/buildzone/min1.smoke.Functional.linaro-x86_64/automated/utils/requirements.txt (line 5))
>   Could not find a version that satisfies the requirement squad_client>0.5 (from -r /fuego-rw/buildzone/min1.smoke.Functional.linaro-
> x86_64/automated/utils/requirements.txt (line 5)) (from versions: ) No matching distribution found for squad_client>0.5 (from -r /fuego-
> rw/buildzone/min1.smoke.Functional.linaro-x86_64/automated/utils/requirements.txt (line 5))
> ----
> 
> There's a 'squad-client 0.18' on pypi.org (note the middle dash, not underscore).
> 
> Upon investigation, it turns out that the latest squad-client requires python 3.6, and Debian stretch has python 3.5.  I checked versions back
> to squad-client 0.1, and all of them require python 3.6, so the Functional.linaro is not compatible with the default python in the Debian in
> the current Fuego docker container.
> 
> Ugh.
>  -- Tim
> 
> 
> 
> > -----Original Message-----
> > From: Bird, Tim
> > Sent: Friday, August 27, 2021 3:48 PM
> > To: daniel.sangorrin@toshiba.co.jp
> > Cc: fuego@lists.linuxfoundation.org; tho1.nguyendat@toshiba.co.jp
> > Subject: RE: [PATCH 2/4] linaro: update to python3
> >
> > > -----Original Message-----
> > > From: daniel.sangorrin@toshiba.co.jp
> > > <daniel.sangorrin@toshiba.co.jp>
> > >
> > > Hi Tim,
> > >
> > > Thanks for your review.
> > > See my comments inline.
> > >
> > > > -----Original Message-----
> > > > From: Tim.Bird@sony.com <Tim.Bird@sony.com>
> > > > Sent: Friday, August 27, 2021 2:54 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 2/4] linaro: update to python3
> > > >
> > > > Ok - I went to apply this today, and I realized I still have some questions...
> > > >
> > > > See inline below.
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
> > > > >
> > > > > From: Nguyen Dat Tho <tho1.nguyendat@toshiba.co.jp>
> > > > >
> > > > > The upstream linaro repository[1] now uses python3 so use pip3
> > > > > to install the requirements
> > > > >
> > > > > [1] https://github.com/Linaro/test-definitions
> > > > >
> > > > > Signed-off-by: Nguyen Dat Tho <tho1.nguyendat@toshiba.co.jp>
> > > > > Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
> > > > > ---
> > > > >  tests/Functional.linaro/fuego_test.sh | 4 +++-
> > > > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/tests/Functional.linaro/fuego_test.sh
> > > > > b/tests/Functional.linaro/fuego_test.sh
> > > > > index 677ab49..c775baa 100755
> > > > > --- a/tests/Functional.linaro/fuego_test.sh
> > > > > +++ b/tests/Functional.linaro/fuego_test.sh
> > > > > @@ -26,8 +26,10 @@ function test_pre_check {  }
> > > > >
> > > > >  function test_build {
> > > > > +    apt-get install python3-pip
> > > > >      source ./automated/bin/setenv.sh
> > > > > -    pip install -r $REPO_PATH/automated/utils/requirements.txt --user
> > > > > +    pip3 install setuptools --user
> > > > > +    pip3 install -r $REPO_PATH/automated/utils/requirements.txt
> > > > > + --user
> > > > >  }
> > > >
> > > > What is the effective user account when this is run in Fuego non-container mode?
> > >
> > > We normally install fuego as root and without jenkins. So it would be running with root permissions.
> > >
> > > > For non-container execution, do you execute build steps as user 'fuego'
> > > > or 'jenkins'?
> > >
> > > We use root. However, you can also install jenkins even when you don't use containers.
> > >
> > > https://bitbucket.org/fuegotest/fuego/src/caf37be917f3756980879f32ad
> > > 044b8f240fbd25/install-native.sh#lines-97
> > >
> > > So in that case, it would jenkins.
> > >
> > > > For a container environment, the build steps are performed as user
> > > > 'jenkins'.  Will these steps even work, in that environment?
> > >
> > > I believe they should work, but we will double check and let you know.
> > >
> > > > Does this need to be 'sudo apt-get install', to handle the container case?
> > >
> > > Probably we can use the Debian packages, we haven't tested that.
> > > Also the requirements.txt from Linaro unfortunately does not specify versions.
> >
> > What is this requirements.txt file?  I'm not well-versed on pip and
> > how it works (other than that it downloads packages and resolves dependencies, like most package manager).
> > Does this requirements.txt have the version of python modules that are needed for Linaro's tools?
> > Does it indicate the list of python modules required for every test in
> > the test-definitions git repository, or just the python modules needed for the Linaro core itself?
> >
> > The reason I ask is to get a sense of the scope of the items that will
> > be installed when this is executed.
> >
> > where does REPO_PATH come from in this test?  (I'm going to guess it
> > comes from setenv.sh)
> >
> > >
> > > > Do the pip3 operations work at the system level, or at the level of an individual account.
> > >
> > > I think that if you run them from jenkins it will install them using
> > > jenkins (although i am not sure what happens if jenkins does not have a $HOME folder). We run it with root so i guess they go to
> /root/.cache/pip or something like that.
> > >
> > > > If a better python3-pip is needed for building, I think this
> > > > should go into the installation scripts, unless this is intended to be doing something on the device under test, and not the host.
> > >
> > > This was intended to run on the host for host-target configurations, and on the DUT for native configurations (eg. LAVA).
> > >
> > > We can put them on the installation scripts, I also thought about this.
> > > The only bad thing is that if the upstream repository changes the requirements.txt file, we might not notice.
> > I agree. That's very likely.
> > >
> > > Also, it is easier to understand the dependencies of each test if we put them there. Actually this should be handled better in general.
> > Agreed.  I have struggled with how much to put into the docker
> > container (or host system, for native installs) to support individual tests, and when exactly to put them there.
> >
> > If you put every possible needed tool in the container, for all tests, you end up with a pretty bloated container.
> > If you defer loading the needed tools and packages until a test is
> > actually invoked, it keeps unnecessary bloat out of the container, but it risks items not finding out about dependency issues until then.
> >
> > I see pros and cons of both approaches.
> >
> > >
> > > Should I move this to the install scripts then?
> >
> > See below for my thinking on this.
> >
> > >
> > > In that case, we will install the requirements without --user (or perhaps using Debian packages) .
> > > Regarding the upstream requirements.txt file, we can ignore until the test doesnt work or we can pin the current commit id.
> > >
> > > > This will have a permanent effect on the build environment, for either the container, or the host machine where Fuego is running.
> > >
> > > Correct.
> > >
> > > > In your test environment, is the host machine the same as the
> > > > device under test, so that these changes are thrown away when the image is thrown away?  Or is this persistent?
> > >
> > > We want to use the LAVA model of always testing on a pristine image. So after running all tests, the image is discarded.
> >
> > So this works pretty well when you're doing the LAVA thing, with Fuego native on the device under test.
> > It gets dicier when this is run in the docker container.  This
> > test_build potentially changes the contents of the python3
> > site-packages for the Jenkins user - which will affect on an ongoing basis all the other tests on the system.
> >
> > In a perfect world, we would sandbox this, and create a separate
> > custom site-packages directory just for the Linaro tests (maybe inside
> > the /fuego-rw/buildzone/Functional.linaro directory).  This avoid changing the python modules for other tests.
> >
> > However, having said that, I don't think we have a ton of tests that
> > use python3 modules, or especially any esoteric python3 modules, which
> > would create incompatibilities.  So setting up a private site-packages
> > directory per test seems like overkill.  I reserve the right to do so sometime in the future, though, if we run into conflicts.
> >
> > >
> > > > It seems like there should be a check to avoid re-installing
> > > > these, if they are already present.  But maybe "apt-get install " or "pip3 install" will just issue a warning, and there is not harm to
> executing these if the packages are already there.
> > >
> > > Yes, they would only issue a warning. But if we remove test_build
> > > and move the dependencies to the install scripts, then we will not need checks at all.
> > >
> > > >
> > > > Anyway, as  you can tell, I am confused by this patch.
> > > >
> > > > Overall, I don't object to it.  I just want to understand what's
> > > > happening with pip3 installation and setuptools installation at the user level and system level, for the case where:
> > > >  - the test is executing in a Fuego container, with a remote DUT
> > > >  - the test is executing in a Fuego container, with a local DUT
> > > > (DUT=self)
> > > >  - the text is executing natively, with a remote DUT
> > > >  - the test is executing natively, with a local DUT (DUT=self)
> > >
> > > Sorry for the confusion.
> > >
> > > We will move the run dependencies to the install scripts. Then, we
> > > will test that those combinations work and what user/location is
> > > used
> > to
> > > install the pip packages.
> > >
> > > What do you think?
> >
> > OK - pip3 and (python3) setuptools should be up-to-date at the system
> > level.  So I'd like to see the installation of those put into the
> > install scripts.  We'll be using more and more python3 code in the future, so I think this would be good.
> >
> > For the packages specifically needed by Linaro tests definitions, I'm
> > OK with deferring filling out the modules from requirements.txt into a local (--user) account, to when the Linaro test is run.
> >
> > So I think I'd like to see this line stay in the test:
> >  +    pip3 install -r $REPO_PATH/automated/utils/requirements.txt --user
> >
> > Is that OK?
> >
> >  -- Tim

[Fuego] [PATCH 3/4] hackbench: fix the chart config file

[Daniel Sangorrin]

From: Nguyen Dat Tho <tho1.nguyendat@toshiba.co.jp>

Signed-off-by: Nguyen Dat Tho <tho1.nguyendat@toshiba.co.jp>
Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
---
 tests/Benchmark.hackbench/chart_config.json | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/Benchmark.hackbench/chart_config.json b/tests/Benchmark.hackbench/chart_config.json
index 2f58df8..f5ca3e4 100644
--- a/tests/Benchmark.hackbench/chart_config.json
+++ b/tests/Benchmark.hackbench/chart_config.json
@@ -1,3 +1,3 @@
 {
-        "hackbench":["hackbench"]
+        "chart_type": "testset_summary_table"
 }
-- 
2.17.1

Re: [Fuego] [PATCH 3/4] hackbench: fix the chart config file

[Tim.Bird]

OK - I spoke too quickly on this one also.

See comments inline below.

> -----Original Message-----
> From: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
> 
> From: Nguyen Dat Tho <tho1.nguyendat@toshiba.co.jp>
> 
> Signed-off-by: Nguyen Dat Tho <tho1.nguyendat@toshiba.co.jp>
> Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
> ---
>  tests/Benchmark.hackbench/chart_config.json | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/Benchmark.hackbench/chart_config.json b/tests/Benchmark.hackbench/chart_config.json
> index 2f58df8..f5ca3e4 100644
> --- a/tests/Benchmark.hackbench/chart_config.json
> +++ b/tests/Benchmark.hackbench/chart_config.json
> @@ -1,3 +1,3 @@
>  {
> -        "hackbench":["hackbench"]
> +        "chart_type": "testset_summary_table"

This chart_config was indeed broken.  Thanks for catching that.
However, since this is a benchmark, I prefer for the 'off-the-shelf'
chart_type to be "measure_plot".

I applied this patch, but with "measure_plot" instead of "testset_summary_table"
as the chart_type.

If you have a reason you'd prefer testset_summary_table over measure_plot,
please let me know and we can discuss it.

Thanks,
 -- Tim

>  }
> --
> 2.17.1
> 

Re: [Fuego] [PATCH 3/4] hackbench: fix the chart config file

[daniel.sangorrin]

Thanks Tim, chart_type "measure_plot" sounds better indeed.

> -----Original Message-----
> From: Tim.Bird@sony.com <Tim.Bird@sony.com>
> Sent: Friday, August 27, 2021 9:33 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 3/4] hackbench: fix the chart config file
> 
> OK - I spoke too quickly on this one also.
> 
> See comments inline below.
> 
> > -----Original Message-----
> > From: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
> >
> > From: Nguyen Dat Tho <tho1.nguyendat@toshiba.co.jp>
> >
> > Signed-off-by: Nguyen Dat Tho <tho1.nguyendat@toshiba.co.jp>
> > Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
> > ---
> >  tests/Benchmark.hackbench/chart_config.json | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tests/Benchmark.hackbench/chart_config.json b/tests/Benchmark.hackbench/chart_config.json
> > index 2f58df8..f5ca3e4 100644
> > --- a/tests/Benchmark.hackbench/chart_config.json
> > +++ b/tests/Benchmark.hackbench/chart_config.json
> > @@ -1,3 +1,3 @@
> >  {
> > -        "hackbench":["hackbench"]
> > +        "chart_type": "testset_summary_table"
> 
> This chart_config was indeed broken.  Thanks for catching that.
> However, since this is a benchmark, I prefer for the 'off-the-shelf'
> chart_type to be "measure_plot".
> 
> I applied this patch, but with "measure_plot" instead of "testset_summary_table"
> as the chart_type.
> 
> If you have a reason you'd prefer testset_summary_table over measure_plot,
> please let me know and we can discuss it.
> 
> Thanks,
>  -- Tim
> 
> >  }
> > --
> > 2.17.1
> >

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

[Daniel Sangorrin]

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
+        report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./cyclictest $BENCHMARK_CYCLICTEST_PARAMS"
+    else
+        assert_has_program cyclictest
+        report "$PROGRAM_CYCLICTEST $BENCHMARK_CYCLICTEST_PARAMS"
+    fi
 }
diff --git a/tests/Benchmark.hackbench/fuego_test.sh b/tests/Benchmark.hackbench/fuego_test.sh
index cc05c74..da209ff 100755
--- a/tests/Benchmark.hackbench/fuego_test.sh
+++ b/tests/Benchmark.hackbench/fuego_test.sh
@@ -16,5 +16,10 @@ function test_deploy {
 }
 
 function test_run {
-    report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./hackbench $BENCHMARK_HACKBENCH_PARAMS"
+    if [ -f $BOARD_TESTDIR/fuego.$TESTDIR/hackbench ]; then
+        report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./hackbench $BENCHMARK_HACKBENCH_PARAMS"
+    else
+        assert_has_program hackbench
+        report "$PROGRAM_HACKBENCH $BENCHMARK_HACKBENCH_PARAMS"
+    fi
 }
diff --git a/tests/Benchmark.migratetest/fuego_test.sh b/tests/Benchmark.migratetest/fuego_test.sh
index eeaa4f6..0b22818 100755
--- a/tests/Benchmark.migratetest/fuego_test.sh
+++ b/tests/Benchmark.migratetest/fuego_test.sh
@@ -23,5 +23,10 @@ function test_deploy {
 }
 
 function test_run {
-    report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./${TEST_COMMAND} $BENCHMARK_MIGRATETEST_PARAMS"
+    if [ -f $BOARD_TESTDIR/fuego.$TESTDIR/$TEST_COMMAND ]; then
+        report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./${TEST_COMMAND} $BENCHMARK_MIGRATETEST_PARAMS"
+    else
+        assert_has_program $TEST_COMMAND
+        report "$PROGRAM_RT_MIGRATE_TEST $BENCHMARK_MIGRATETEST_PARAMS"
+    fi
 }
diff --git a/tests/Benchmark.pmqtest/fuego_test.sh b/tests/Benchmark.pmqtest/fuego_test.sh
index 6733e78..8d3f75a 100755
--- a/tests/Benchmark.pmqtest/fuego_test.sh
+++ b/tests/Benchmark.pmqtest/fuego_test.sh
@@ -27,5 +27,11 @@ function test_run {
     # The number for getting the lines depends on the cpu number of target machine.
     target_cpu_number=$(cmd "nproc")
     getting_line_number=$(( $target_cpu_number + $target_cpu_number ))
-    report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./pmqtest $BENCHMARK_PMQTEST_PARAMS | tail -$getting_line_number"
+
+    if [ -f $BOARD_TESTDIR/fuego.$TESTDIR/pmqtest ]; then
+        report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./pmqtest $BENCHMARK_PMQTEST_PARAMS | tail -$getting_line_number"
+    else
+        assert_has_program pmqtest
+        report "$PROGRAM_PMQTEST $BENCHMARK_PMQTEST_PARAMS | tail -$getting_line_number"
+    fi
 }
diff --git a/tests/Benchmark.ptsematest/fuego_test.sh b/tests/Benchmark.ptsematest/fuego_test.sh
index a626d44..e534412 100755
--- a/tests/Benchmark.ptsematest/fuego_test.sh
+++ b/tests/Benchmark.ptsematest/fuego_test.sh
@@ -27,5 +27,11 @@ function test_run {
     # The number for getting the lines depends on the cpu number of target machine.
     target_cpu_number=$(cmd "cat /proc/cpuinfo | grep processor | wc -l")
     getting_line_number=$(( $target_cpu_number + $target_cpu_number ))
-    report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./ptsematest $BENCHMARK_PTSEMATEST_PARAMS | tail -$getting_line_number"
+
+    if [ -f $BOARD_TESTDIR/fuego.$TESTDIR/ptsematest ]; then
+        report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./ptsematest $BENCHMARK_PTSEMATEST_PARAMS | tail -$getting_line_number"
+    else
+        assert_has_program ptsematest
+        report "$PROGRAM_PTSEMATEST $BENCHMARK_PTSEMATEST_PARAMS | tail -$getting_line_number"
+    fi
 }
diff --git a/tests/Benchmark.signaltest/fuego_test.sh b/tests/Benchmark.signaltest/fuego_test.sh
index 54e08b1..8811d17 100755
--- a/tests/Benchmark.signaltest/fuego_test.sh
+++ b/tests/Benchmark.signaltest/fuego_test.sh
@@ -22,5 +22,10 @@ function test_deploy {
 }
 
 function test_run {
-    report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./signaltest $BENCHMARK_SIGNALTEST_PARAMS"
+    if [ -f $BOARD_TESTDIR/fuego.$TESTDIR/signaltest ]; then
+        report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./signaltest $BENCHMARK_SIGNALTEST_PARAMS"
+    else
+        assert_has_program signaltest
+        report "$PROGRAM_SIGNALTEST $BENCHMARK_SIGNALTEST_PARAMS"
+    fi
 }
diff --git a/tests/Benchmark.sigwaittest/fuego_test.sh b/tests/Benchmark.sigwaittest/fuego_test.sh
index e419f17..babcab6 100755
--- a/tests/Benchmark.sigwaittest/fuego_test.sh
+++ b/tests/Benchmark.sigwaittest/fuego_test.sh
@@ -27,5 +27,10 @@ function test_run {
     # The number for getting the lines depends on the cpu number of target machine.
     target_cpu_number=$(cmd "nproc")
     getting_line_number=$(( $target_cpu_number + $target_cpu_number ))
-    report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./sigwaittest $BENCHMARK_SIGWAITTEST_PARAMS | tail -$getting_line_number"
+    if [ -f $BOARD_TESTDIR/fuego.$TESTDIR/sigwaittest ]; then
+        report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./sigwaittest $BENCHMARK_SIGWAITTEST_PARAMS | tail -$getting_line_number"
+    else
+        assert_has_program sigwaittest
+        report "$PROGRAM_SIGWAITTEST $BENCHMARK_SIGWAITTEST_PARAMS | tail -$getting_line_number"
+    fi
 }
diff --git a/tests/Benchmark.svsematest/fuego_test.sh b/tests/Benchmark.svsematest/fuego_test.sh
index 2f6e914..cb0184b 100755
--- a/tests/Benchmark.svsematest/fuego_test.sh
+++ b/tests/Benchmark.svsematest/fuego_test.sh
@@ -27,5 +27,10 @@ function test_run {
     # The number for getting the lines depends on the cpu number of target machine.
     target_cpu_number=$(cmd "cat /proc/cpuinfo | grep processor | wc -l")
     getting_line_number=$(( $target_cpu_number + $target_cpu_number ))
-    report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./svsematest $BENCHMARK_SVSEMATEST_PARAMS | tail -$getting_line_number"
+    if [ -f $BOARD_TESTDIR/fuego.$TESTDIR/svsematest ]; then
+        report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./svsematest $BENCHMARK_SVSEMATEST_PARAMS | tail -$getting_line_number"
+    else
+        assert_has_program svsematest
+        report "$PROGRAM_SVSEMATEST $BENCHMARK_SVSEMATEST_PARAMS | tail -$getting_line_number"
+    fi
 }
diff --git a/tests/Functional.pi_tests/fuego_test.sh b/tests/Functional.pi_tests/fuego_test.sh
index bf94a63..590fbf3 100755
--- a/tests/Functional.pi_tests/fuego_test.sh
+++ b/tests/Functional.pi_tests/fuego_test.sh
@@ -22,7 +22,12 @@ function test_deploy {
 }
 
 function test_run {
-    report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./pi_stress $FUNCTIONAL_PI_TESTS_PARAMS"
+    if [ -f $BOARD_TESTDIR/fuego.$TESTDIR/pi_stress ]; then
+        report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./pi_stress $FUNCTIONAL_PI_TESTS_PARAMS"
+    else
+        assert_has_program pi_stress
+        report "$PROGRAM_PI_STRESS $FUNCTIONAL_PI_TESTS_PARAMS"
+    fi
 }
 
 function test_processing {
-- 
2.17.1

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

[Tim.Bird]

> -----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.

> +        report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./cyclictest $BENCHMARK_CYCLICTEST_PARAMS"
> +    else
> +        assert_has_program cyclictest
> +        report "$PROGRAM_CYCLICTEST $BENCHMARK_CYCLICTEST_PARAMS"
The report line is identical in both branches of the if.  This should
be put outside the conditional.

> +    fi

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?

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
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)?

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

 -- Tim

>  }
> diff --git a/tests/Benchmark.hackbench/fuego_test.sh b/tests/Benchmark.hackbench/fuego_test.sh
> index cc05c74..da209ff 100755
> --- a/tests/Benchmark.hackbench/fuego_test.sh
> +++ b/tests/Benchmark.hackbench/fuego_test.sh
> @@ -16,5 +16,10 @@ function test_deploy {
>  }
> 
>  function test_run {
> -    report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./hackbench $BENCHMARK_HACKBENCH_PARAMS"
> +    if [ -f $BOARD_TESTDIR/fuego.$TESTDIR/hackbench ]; then
> +        report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./hackbench $BENCHMARK_HACKBENCH_PARAMS"
> +    else
> +        assert_has_program hackbench
> +        report "$PROGRAM_HACKBENCH $BENCHMARK_HACKBENCH_PARAMS"
> +    fi
>  }
> diff --git a/tests/Benchmark.migratetest/fuego_test.sh b/tests/Benchmark.migratetest/fuego_test.sh
> index eeaa4f6..0b22818 100755
> --- a/tests/Benchmark.migratetest/fuego_test.sh
> +++ b/tests/Benchmark.migratetest/fuego_test.sh
> @@ -23,5 +23,10 @@ function test_deploy {
>  }
> 
>  function test_run {
> -    report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./${TEST_COMMAND} $BENCHMARK_MIGRATETEST_PARAMS"
> +    if [ -f $BOARD_TESTDIR/fuego.$TESTDIR/$TEST_COMMAND ]; then
> +        report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./${TEST_COMMAND} $BENCHMARK_MIGRATETEST_PARAMS"
> +    else
> +        assert_has_program $TEST_COMMAND
> +        report "$PROGRAM_RT_MIGRATE_TEST $BENCHMARK_MIGRATETEST_PARAMS"
> +    fi
>  }
> diff --git a/tests/Benchmark.pmqtest/fuego_test.sh b/tests/Benchmark.pmqtest/fuego_test.sh
> index 6733e78..8d3f75a 100755
> --- a/tests/Benchmark.pmqtest/fuego_test.sh
> +++ b/tests/Benchmark.pmqtest/fuego_test.sh
> @@ -27,5 +27,11 @@ function test_run {
>      # The number for getting the lines depends on the cpu number of target machine.
>      target_cpu_number=$(cmd "nproc")
>      getting_line_number=$(( $target_cpu_number + $target_cpu_number ))
> -    report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./pmqtest $BENCHMARK_PMQTEST_PARAMS | tail -$getting_line_number"
> +
> +    if [ -f $BOARD_TESTDIR/fuego.$TESTDIR/pmqtest ]; then
> +        report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./pmqtest $BENCHMARK_PMQTEST_PARAMS | tail -$getting_line_number"
> +    else
> +        assert_has_program pmqtest
> +        report "$PROGRAM_PMQTEST $BENCHMARK_PMQTEST_PARAMS | tail -$getting_line_number"
> +    fi
>  }
> diff --git a/tests/Benchmark.ptsematest/fuego_test.sh b/tests/Benchmark.ptsematest/fuego_test.sh
> index a626d44..e534412 100755
> --- a/tests/Benchmark.ptsematest/fuego_test.sh
> +++ b/tests/Benchmark.ptsematest/fuego_test.sh
> @@ -27,5 +27,11 @@ function test_run {
>      # The number for getting the lines depends on the cpu number of target machine.
>      target_cpu_number=$(cmd "cat /proc/cpuinfo | grep processor | wc -l")
>      getting_line_number=$(( $target_cpu_number + $target_cpu_number ))
> -    report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./ptsematest $BENCHMARK_PTSEMATEST_PARAMS | tail -$getting_line_number"
> +
> +    if [ -f $BOARD_TESTDIR/fuego.$TESTDIR/ptsematest ]; then
> +        report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./ptsematest $BENCHMARK_PTSEMATEST_PARAMS | tail -$getting_line_number"
> +    else
> +        assert_has_program ptsematest
> +        report "$PROGRAM_PTSEMATEST $BENCHMARK_PTSEMATEST_PARAMS | tail -$getting_line_number"
> +    fi
>  }
> diff --git a/tests/Benchmark.signaltest/fuego_test.sh b/tests/Benchmark.signaltest/fuego_test.sh
> index 54e08b1..8811d17 100755
> --- a/tests/Benchmark.signaltest/fuego_test.sh
> +++ b/tests/Benchmark.signaltest/fuego_test.sh
> @@ -22,5 +22,10 @@ function test_deploy {
>  }
> 
>  function test_run {
> -    report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./signaltest $BENCHMARK_SIGNALTEST_PARAMS"
> +    if [ -f $BOARD_TESTDIR/fuego.$TESTDIR/signaltest ]; then
> +        report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./signaltest $BENCHMARK_SIGNALTEST_PARAMS"
> +    else
> +        assert_has_program signaltest
> +        report "$PROGRAM_SIGNALTEST $BENCHMARK_SIGNALTEST_PARAMS"
> +    fi
>  }
> diff --git a/tests/Benchmark.sigwaittest/fuego_test.sh b/tests/Benchmark.sigwaittest/fuego_test.sh
> index e419f17..babcab6 100755
> --- a/tests/Benchmark.sigwaittest/fuego_test.sh
> +++ b/tests/Benchmark.sigwaittest/fuego_test.sh
> @@ -27,5 +27,10 @@ function test_run {
>      # The number for getting the lines depends on the cpu number of target machine.
>      target_cpu_number=$(cmd "nproc")
>      getting_line_number=$(( $target_cpu_number + $target_cpu_number ))
> -    report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./sigwaittest $BENCHMARK_SIGWAITTEST_PARAMS | tail -$getting_line_number"
> +    if [ -f $BOARD_TESTDIR/fuego.$TESTDIR/sigwaittest ]; then
> +        report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./sigwaittest $BENCHMARK_SIGWAITTEST_PARAMS | tail -$getting_line_number"
> +    else
> +        assert_has_program sigwaittest
> +        report "$PROGRAM_SIGWAITTEST $BENCHMARK_SIGWAITTEST_PARAMS | tail -$getting_line_number"
> +    fi
>  }
> diff --git a/tests/Benchmark.svsematest/fuego_test.sh b/tests/Benchmark.svsematest/fuego_test.sh
> index 2f6e914..cb0184b 100755
> --- a/tests/Benchmark.svsematest/fuego_test.sh
> +++ b/tests/Benchmark.svsematest/fuego_test.sh
> @@ -27,5 +27,10 @@ function test_run {
>      # The number for getting the lines depends on the cpu number of target machine.
>      target_cpu_number=$(cmd "cat /proc/cpuinfo | grep processor | wc -l")
>      getting_line_number=$(( $target_cpu_number + $target_cpu_number ))
> -    report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./svsematest $BENCHMARK_SVSEMATEST_PARAMS | tail -$getting_line_number"
> +    if [ -f $BOARD_TESTDIR/fuego.$TESTDIR/svsematest ]; then
> +        report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./svsematest $BENCHMARK_SVSEMATEST_PARAMS | tail -$getting_line_number"
> +    else
> +        assert_has_program svsematest
> +        report "$PROGRAM_SVSEMATEST $BENCHMARK_SVSEMATEST_PARAMS | tail -$getting_line_number"
> +    fi
>  }
> diff --git a/tests/Functional.pi_tests/fuego_test.sh b/tests/Functional.pi_tests/fuego_test.sh
> index bf94a63..590fbf3 100755
> --- a/tests/Functional.pi_tests/fuego_test.sh
> +++ b/tests/Functional.pi_tests/fuego_test.sh
> @@ -22,7 +22,12 @@ function test_deploy {
>  }
> 
>  function test_run {
> -    report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./pi_stress $FUNCTIONAL_PI_TESTS_PARAMS"
> +    if [ -f $BOARD_TESTDIR/fuego.$TESTDIR/pi_stress ]; then
> +        report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./pi_stress $FUNCTIONAL_PI_TESTS_PARAMS"
> +    else
> +        assert_has_program pi_stress
> +        report "$PROGRAM_PI_STRESS $FUNCTIONAL_PI_TESTS_PARAMS"
> +    fi
>  }
> 
>  function test_processing {
> --
> 2.17.1
> 

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

[Tim.Bird]

Oops. Reading comprehension error.

> -----Original Message-----
> From: Fuego <fuego-bounces@lists.linuxfoundation.org> On Behalf Of Tim.Bird@sony.com
> 
> > +        report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./cyclictest $BENCHMARK_CYCLICTEST_PARAMS"
> > +    else
> > +        assert_has_program cyclictest
> > +        report "$PROGRAM_CYCLICTEST $BENCHMARK_CYCLICTEST_PARAMS"
> The report line is identical in both branches of the if.  This should
> be put outside the conditional.

Oops.  My bad.  These are not the same.   Please ignore this feedback.
 -- Tim

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

[daniel.sangorrin]

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?

Thanks,
Daniel


> 
>  -- Tim
> 
> >  }
> > diff --git a/tests/Benchmark.hackbench/fuego_test.sh
> > b/tests/Benchmark.hackbench/fuego_test.sh
> > index cc05c74..da209ff 100755
> > --- a/tests/Benchmark.hackbench/fuego_test.sh
> > +++ b/tests/Benchmark.hackbench/fuego_test.sh
> > @@ -16,5 +16,10 @@ function test_deploy {  }
> >
> >  function test_run {
> > -    report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./hackbench $BENCHMARK_HACKBENCH_PARAMS"
> > +    if [ -f $BOARD_TESTDIR/fuego.$TESTDIR/hackbench ]; then
> > +        report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./hackbench $BENCHMARK_HACKBENCH_PARAMS"
> > +    else
> > +        assert_has_program hackbench
> > +        report "$PROGRAM_HACKBENCH $BENCHMARK_HACKBENCH_PARAMS"
> > +    fi
> >  }
> > diff --git a/tests/Benchmark.migratetest/fuego_test.sh
> > b/tests/Benchmark.migratetest/fuego_test.sh
> > index eeaa4f6..0b22818 100755
> > --- a/tests/Benchmark.migratetest/fuego_test.sh
> > +++ b/tests/Benchmark.migratetest/fuego_test.sh
> > @@ -23,5 +23,10 @@ function test_deploy {  }
> >
> >  function test_run {
> > -    report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./${TEST_COMMAND} $BENCHMARK_MIGRATETEST_PARAMS"
> > +    if [ -f $BOARD_TESTDIR/fuego.$TESTDIR/$TEST_COMMAND ]; then
> > +        report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./${TEST_COMMAND} $BENCHMARK_MIGRATETEST_PARAMS"
> > +    else
> > +        assert_has_program $TEST_COMMAND
> > +        report "$PROGRAM_RT_MIGRATE_TEST $BENCHMARK_MIGRATETEST_PARAMS"
> > +    fi
> >  }
> > diff --git a/tests/Benchmark.pmqtest/fuego_test.sh
> > b/tests/Benchmark.pmqtest/fuego_test.sh
> > index 6733e78..8d3f75a 100755
> > --- a/tests/Benchmark.pmqtest/fuego_test.sh
> > +++ b/tests/Benchmark.pmqtest/fuego_test.sh
> > @@ -27,5 +27,11 @@ function test_run {
> >      # The number for getting the lines depends on the cpu number of target machine.
> >      target_cpu_number=$(cmd "nproc")
> >      getting_line_number=$(( $target_cpu_number + $target_cpu_number ))
> > -    report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./pmqtest $BENCHMARK_PMQTEST_PARAMS | tail -$getting_line_number"
> > +
> > +    if [ -f $BOARD_TESTDIR/fuego.$TESTDIR/pmqtest ]; then
> > +        report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./pmqtest $BENCHMARK_PMQTEST_PARAMS | tail -$getting_line_number"
> > +    else
> > +        assert_has_program pmqtest
> > +        report "$PROGRAM_PMQTEST $BENCHMARK_PMQTEST_PARAMS | tail -$getting_line_number"
> > +    fi
> >  }
> > diff --git a/tests/Benchmark.ptsematest/fuego_test.sh
> > b/tests/Benchmark.ptsematest/fuego_test.sh
> > index a626d44..e534412 100755
> > --- a/tests/Benchmark.ptsematest/fuego_test.sh
> > +++ b/tests/Benchmark.ptsematest/fuego_test.sh
> > @@ -27,5 +27,11 @@ function test_run {
> >      # The number for getting the lines depends on the cpu number of target machine.
> >      target_cpu_number=$(cmd "cat /proc/cpuinfo | grep processor | wc -l")
> >      getting_line_number=$(( $target_cpu_number + $target_cpu_number ))
> > -    report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./ptsematest $BENCHMARK_PTSEMATEST_PARAMS | tail -$getting_line_number"
> > +
> > +    if [ -f $BOARD_TESTDIR/fuego.$TESTDIR/ptsematest ]; then
> > +        report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./ptsematest $BENCHMARK_PTSEMATEST_PARAMS | tail -
> $getting_line_number"
> > +    else
> > +        assert_has_program ptsematest
> > +        report "$PROGRAM_PTSEMATEST $BENCHMARK_PTSEMATEST_PARAMS | tail -$getting_line_number"
> > +    fi
> >  }
> > diff --git a/tests/Benchmark.signaltest/fuego_test.sh
> > b/tests/Benchmark.signaltest/fuego_test.sh
> > index 54e08b1..8811d17 100755
> > --- a/tests/Benchmark.signaltest/fuego_test.sh
> > +++ b/tests/Benchmark.signaltest/fuego_test.sh
> > @@ -22,5 +22,10 @@ function test_deploy {  }
> >
> >  function test_run {
> > -    report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./signaltest $BENCHMARK_SIGNALTEST_PARAMS"
> > +    if [ -f $BOARD_TESTDIR/fuego.$TESTDIR/signaltest ]; then
> > +        report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./signaltest $BENCHMARK_SIGNALTEST_PARAMS"
> > +    else
> > +        assert_has_program signaltest
> > +        report "$PROGRAM_SIGNALTEST $BENCHMARK_SIGNALTEST_PARAMS"
> > +    fi
> >  }
> > diff --git a/tests/Benchmark.sigwaittest/fuego_test.sh
> > b/tests/Benchmark.sigwaittest/fuego_test.sh
> > index e419f17..babcab6 100755
> > --- a/tests/Benchmark.sigwaittest/fuego_test.sh
> > +++ b/tests/Benchmark.sigwaittest/fuego_test.sh
> > @@ -27,5 +27,10 @@ function test_run {
> >      # The number for getting the lines depends on the cpu number of target machine.
> >      target_cpu_number=$(cmd "nproc")
> >      getting_line_number=$(( $target_cpu_number + $target_cpu_number ))
> > -    report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./sigwaittest $BENCHMARK_SIGWAITTEST_PARAMS | tail -$getting_line_number"
> > +    if [ -f $BOARD_TESTDIR/fuego.$TESTDIR/sigwaittest ]; then
> > +        report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./sigwaittest $BENCHMARK_SIGWAITTEST_PARAMS | tail -
> $getting_line_number"
> > +    else
> > +        assert_has_program sigwaittest
> > +        report "$PROGRAM_SIGWAITTEST $BENCHMARK_SIGWAITTEST_PARAMS | tail -$getting_line_number"
> > +    fi
> >  }
> > diff --git a/tests/Benchmark.svsematest/fuego_test.sh
> > b/tests/Benchmark.svsematest/fuego_test.sh
> > index 2f6e914..cb0184b 100755
> > --- a/tests/Benchmark.svsematest/fuego_test.sh
> > +++ b/tests/Benchmark.svsematest/fuego_test.sh
> > @@ -27,5 +27,10 @@ function test_run {
> >      # The number for getting the lines depends on the cpu number of target machine.
> >      target_cpu_number=$(cmd "cat /proc/cpuinfo | grep processor | wc -l")
> >      getting_line_number=$(( $target_cpu_number + $target_cpu_number ))
> > -    report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./svsematest $BENCHMARK_SVSEMATEST_PARAMS | tail -$getting_line_number"
> > +    if [ -f $BOARD_TESTDIR/fuego.$TESTDIR/svsematest ]; then
> > +        report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./svsematest $BENCHMARK_SVSEMATEST_PARAMS | tail -
> $getting_line_number"
> > +    else
> > +        assert_has_program svsematest
> > +        report "$PROGRAM_SVSEMATEST $BENCHMARK_SVSEMATEST_PARAMS | tail -$getting_line_number"
> > +    fi
> >  }
> > diff --git a/tests/Functional.pi_tests/fuego_test.sh
> > b/tests/Functional.pi_tests/fuego_test.sh
> > index bf94a63..590fbf3 100755
> > --- a/tests/Functional.pi_tests/fuego_test.sh
> > +++ b/tests/Functional.pi_tests/fuego_test.sh
> > @@ -22,7 +22,12 @@ function test_deploy {  }
> >
> >  function test_run {
> > -    report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./pi_stress $FUNCTIONAL_PI_TESTS_PARAMS"
> > +    if [ -f $BOARD_TESTDIR/fuego.$TESTDIR/pi_stress ]; then
> > +        report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./pi_stress $FUNCTIONAL_PI_TESTS_PARAMS"
> > +    else
> > +        assert_has_program pi_stress
> > +        report "$PROGRAM_PI_STRESS $FUNCTIONAL_PI_TESTS_PARAMS"
> > +    fi
> >  }
> >
> >  function test_processing {
> > --
> > 2.17.1
> >

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

[Tim.Bird]

> -----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

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

[tho1.nguyendat]

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

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

[Tim.Bird]

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

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

[Venkata.Pyla]

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

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

[Tim.Bird]

> -----Original Message-----
> From: Venkata.Pyla@toshiba-tsip.com <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.

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 <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

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

[daniel.sangorrin]

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.

Does the variable  "use_binary_packages=false" have something to do with this?

Thanks,
Daniel




> -----Original Message-----
> From: Fuego <fuego-bounces@lists.linuxfoundation.org> On Behalf Of Tim.Bird@sony.com
> Sent: Wednesday, September 8, 2021 10:14 AM
> To: pyla venkata(ＴＳＩＰ) <Venkata.Pyla@toshiba-tsip.com>
> Cc: fuego@lists.linuxfoundation.org; nguyen dat tho(ＴＳＤＶ Ｅｎｇ　１) <tho1.nguyendat@toshiba.co.jp>
> Subject: Re: [Fuego] [PATCH 4/4] rt: search for the binary if the build phase is skipped
> 
> > -----Original Message-----
> > From: Venkata.Pyla@toshiba-tsip.com <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.
> 
> 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 <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
> _______________________________________________
> Fuego mailing list
> Fuego@lists.linuxfoundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/fuego

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

[Venkata.Pyla]

>-----Original Message-----
>From: sangorrin daniel(サンゴリン ダニエル □ＳＷＣ◯ＡＣＴ)
><daniel.sangorrin@toshiba.co.jp>
>Sent: 08 September 2021 10:34
>To: Tim.Bird@sony.com; pyla venkata(ＴＳＩＰ) <Venkata.Pyla@toshiba-
>tsip.com>
>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
>
>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.
>
>Does the variable  "use_binary_packages=false" have something to do with this?

I think this feature uses the binary package that is built by fuego phase (makepkg) stored in cache folder, it copies again in the Fuego test directory and run from there.
It doesn’t check the host installed binaries.

Thanks,
Venkata.
 
>
>Thanks,
>Daniel
>
>
>
>
>> -----Original Message-----
>> From: Fuego <fuego-bounces@lists.linuxfoundation.org> On Behalf Of
>> Tim.Bird@sony.com
>> Sent: Wednesday, September 8, 2021 10:14 AM
>> To: pyla venkata(ＴＳＩＰ) <Venkata.Pyla@toshiba-tsip.com>
>> Cc: fuego@lists.linuxfoundation.org; nguyen dat tho(ＴＳＤＶ Ｅｎｇ　１)
>> <tho1.nguyendat@toshiba.co.jp>
>> Subject: Re: [Fuego] [PATCH 4/4] rt: search for the binary if the
>> build phase is skipped
>>
>> > -----Original Message-----
>> > From: Venkata.Pyla@toshiba-tsip.com <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.
>>
>> 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 <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
>> _______________________________________________
>> Fuego mailing list
>> Fuego@lists.linuxfoundation.org
>> https://lists.linuxfoundation.org/mailman/listinfo/fuego

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

[Tim.Bird]

> -----Original Message-----
> From: daniel.sangorrin@toshiba.co.jp <daniel.sangorrin@toshiba.co.jp>
> 
> 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 <fuego-bounces@lists.linuxfoundation.org> On Behalf Of Tim.Bird@sony.com
> > Sent: Wednesday, September 8, 2021 10:14 AM
> > To: pyla venkata(ＴＳＩＰ) <Venkata.Pyla@toshiba-tsip.com>
> > Cc: fuego@lists.linuxfoundation.org; nguyen dat tho(ＴＳＤＶ Ｅｎｇ　１) <tho1.nguyendat@toshiba.co.jp>
> > Subject: Re: [Fuego] [PATCH 4/4] rt: search for the binary if the build phase is skipped
> >
> > > -----Original Message-----
> > > From: Venkata.Pyla@toshiba-tsip.com <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.
> >
> > 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 <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
> > _______________________________________________
> > Fuego mailing list
> > Fuego@lists.linuxfoundation.org
> > https://lists.linuxfoundation.org/mailman/listinfo/fuego

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

[daniel.sangorrin]

Hi Tim,

> -----Original Message-----
> From: Tim.Bird@sony.com <Tim.Bird@sony.com>
[..]
> 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 <fuego-bounces@lists.linuxfoundation.org> On Behalf Of
> > > Tim.Bird@sony.com
> > > Sent: Wednesday, September 8, 2021 10:14 AM
> > > To: pyla venkata(ＴＳＩＰ) <Venkata.Pyla@toshiba-tsip.com>
> > > Cc: fuego@lists.linuxfoundation.org; nguyen dat tho(ＴＳＤＶ Ｅｎｇ　１)
> > > <tho1.nguyendat@toshiba.co.jp>
> > > Subject: Re: [Fuego] [PATCH 4/4] rt: search for the binary if the
> > > build phase is skipped
> > >
> > > > -----Original Message-----
> > > > From: Venkata.Pyla@toshiba-tsip.com
> > > > <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.
> > >
> > > 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 <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
> > > _______________________________________________
> > > Fuego mailing list
> > > Fuego@lists.linuxfoundation.org
> > > https://lists.linuxfoundation.org/mailman/listinfo/fuego

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

[Tim.Bird]

> -----Original Message-----
> From: daniel.sangorrin@toshiba.co.jp <daniel.sangorrin@toshiba.co.jp>
> 
> Hi Tim,
> 
> > -----Original Message-----
> > From: Tim.Bird@sony.com <Tim.Bird@sony.com>
> [..]
> > 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).

OK - thanks.

> 
> [...]
> > 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)

OK - sorry this took so long to get back to.

I decided to forego the configuration variable, and I changed the search order to:
 $BOARD_TESTDIR/fuego.$TESTDIR
 extra directories (if specified)
 PATH directories

I also changed the implementation to require less 'cmd' operations on the board
(this is nicer for boards where the ssh latencies are noticeable).  For
boards where TRANSPORT=local, this should not improve the speed much, but
get_program_path should be pretty fast in either case.

I changed Benchmark.Dhrystone to use get_program_path, as we previously
discussed.  This means that this test is already primed to be used in your use
case (which I'll call the "pre-deployed test programs" use case).

Please try it out and let me know if you see any problems.  I wrote a unit test
for get_program_path(), called Functional.fuego_function_gpp_check.  If you encounter
any problems, please run that test, with FUEGO_LOGLEVELS=run:debug, and send
the console log for the test.

Thanks for the original patch and the idea for the feature.

Regards,
 -- Tim

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

[daniel.sangorrin]

Thanks a lot Tim, we will try it soon.
We will also think about the porting to Bullseye and get back to you.

Best regards,
Daniel

> -----Original Message-----
> From: Tim.Bird@sony.com <Tim.Bird@sony.com>
> Sent: Thursday, October 7, 2021 10:56 AM
> To: sangorrin daniel(サンゴリン ダニエル □ＳＷＣ◯ＡＣＴ) <daniel.sangorrin@toshiba.co.jp>; pyla venkata(ＴＳＩＰ)
> <Venkata.Pyla@toshiba-tsip.com>
> 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,
> >
> > > -----Original Message-----
> > > From: Tim.Bird@sony.com <Tim.Bird@sony.com>
> > [..]
> > > 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).
> 
> OK - thanks.
> 
> >
> > [...]
> > > 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)
> 
> OK - sorry this took so long to get back to.
> 
> I decided to forego the configuration variable, and I changed the search order to:
>  $BOARD_TESTDIR/fuego.$TESTDIR
>  extra directories (if specified)
>  PATH directories
> 
> I also changed the implementation to require less 'cmd' operations on the board (this is nicer for boards where the ssh latencies are
> noticeable).  For boards where TRANSPORT=local, this should not improve the speed much, but get_program_path should be pretty fast in
> either case.
> 
> I changed Benchmark.Dhrystone to use get_program_path, as we previously discussed.  This means that this test is already primed to be
> used in your use case (which I'll call the "pre-deployed test programs" use case).
> 
> Please try it out and let me know if you see any problems.  I wrote a unit test for get_program_path(), called
> Functional.fuego_function_gpp_check.  If you encounter any problems, please run that test, with FUEGO_LOGLEVELS=run:debug, and send
> the console log for the test.
> 
> Thanks for the original patch and the idea for the feature.
> 
> Regards,
>  -- Tim

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

[Venkata.Pyla]

Hi Tim,

Thank you for clarifying me, please see my inline comments

>-----Original Message-----
>From: Tim.Bird@sony.com <Tim.Bird@sony.com>
>Sent: 08 September 2021 06:44
>To: pyla venkata(ＴＳＩＰ) <Venkata.Pyla@toshiba-tsip.com>
>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: Venkata.Pyla@toshiba-tsip.com <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.
>
>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?

Our intention is to use the pre-installed binaries in the board while we skip the build and deploy phase of fuego.

>
>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?

If Build and deploy phase is used then it is better to use it from Fuego test directory, else search for the path order you mentioned below case #1, #2

>
>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.

Yes, I think the case #3 is not needed.

>
>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 think this works for us, as we skip the build and deploy phase, so it should use program from the 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?

I think changing the order of the search path in 'get_program_path' should work for us.

> -- Tim
>
>>
>> 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

Re: [Fuego] fixes for the linaro and rt tests

[Tim.Bird]

I'm consolidating feedback into a single message to save time.

> -----Original Message-----
> From: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
> 
> Hi Tim
> 
> Please consider merging these patches.
> 
> [PATCH 1/4] linaro: localhost does not require ssh
> 
> This ones is self explanatory (no SSH if you use the local board)
This looks OK.  I'm wondering if there's another way to detect a local
operation.  Can we check the TRANSPORT (e.g. if [ $TRANSPORT = "local" ] ...)?
Checking the network address seems a bit iffy.

> 
> [PATCH 2/4] linaro: update to python3
> 
> This one uses pip3 because the upstream python scripts are for
> python3 now.

This seems fine.
> 
> I thought about adding the dependencies to the Docker install
> scripts, but since the requirements are in the upstream repository
> I thought it is fine leaving them as they are (you can always
> install them in advance and then skip this phase)
> 
> [PATCH 3/4] hackbench: fix the chart config file
> 
> I think this was some copy paste file, which is fixed now.
> 
Looks good.

> [PATCH 4/4] rt: search for the binary if the build phase is skipped
> 
> This are all very similar fixes where we enable support for
> running binaries that already exist on the board's file system.
> For example, because you install rt-tests via apt-get and then
> you skipped the Fuego build phase.

Interesting idea, and it will save some time.  However, the patch, as is,
doesn't work on a remote board.  I'll respond to that one with some inline
comments.
 -- Tim

Re: [Fuego] fixes for the linaro and rt tests

[daniel.sangorrin]

Hi Tim

Thanks for reviewing the patches

> -----Original Message-----
> From: Tim.Bird@sony.com <Tim.Bird@sony.com>
> > [PATCH 1/4] linaro: localhost does not require ssh
> >
> > This ones is self explanatory (no SSH if you use the local board)
> This looks OK.  I'm wondering if there's another way to detect a local operation.  Can we check the TRANSPORT (e.g. if [ $TRANSPORT
> = "local" ] ...)?
> Checking the network address seems a bit iffy.

You are right, we will try with TRANSPORT or another variable and re-send the patch.

Thanks,
Daniel

Re: [Fuego] fixes for the linaro and rt tests

[tho1.nguyendat]

[-- Attachment #1: Type: text/plain, Size: 1081 bytes --]

Dear Tim,

I updated "patch [1/4] linaro: localhost does not require ssh"
Could you please help me check the attachment?

Thanks,
Tho

-----Original Message-----
From: sangorrin daniel(サンゴリン ダニエル □ＳＷＣ◯ＡＣＴ) <daniel.sangorrin@toshiba.co.jp> 
Sent: Thursday, August 26, 2021 9:46 AM
To: Tim.Bird@sony.com
Cc: fuego@lists.linuxfoundation.org; nguyen dat tho(ＴＳＤＶ Ｅｎｇ　１) <tho1.nguyendat@toshiba.co.jp>
Subject: RE: fixes for the linaro and rt tests

Hi Tim

Thanks for reviewing the patches

> -----Original Message-----
> From: Tim.Bird@sony.com <Tim.Bird@sony.com>
> > [PATCH 1/4] linaro: localhost does not require ssh
> >
> > This ones is self explanatory (no SSH if you use the local board)
> This looks OK.  I'm wondering if there's another way to detect a local 
> operation.  Can we check the TRANSPORT (e.g. if [ $TRANSPORT = "local" ] ...)?
> Checking the network address seems a bit iffy.

You are right, we will try with TRANSPORT or another variable and re-send the patch.

Thanks,
Daniel

[-- Attachment #2: 0001-linaro-localhost-does-not-require-ssh.patch --]
[-- Type: application/octet-stream, Size: 1890 bytes --]

From 14dc2fc73db6bc99c7ef2830bc8230481e64f427 Mon Sep 17 00:00:00 2001
From: Nguyen Dat Tho <tho1.nguyendat@toshiba.co.jp>
Date: Fri, 20 Aug 2021 10:51:37 +0900
Subject: [PATCH] linaro: localhost does not require ssh

When running the test on localhost ssh is not required

Signed-off-by: Nguyen Dat Tho <tho1.nguyendat@toshiba.co.jp>
Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
---
 tests/Functional.linaro/fuego_test.sh | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/tests/Functional.linaro/fuego_test.sh b/tests/Functional.linaro/fuego_test.sh
index 86d3c43..71c77e7 100755
--- a/tests/Functional.linaro/fuego_test.sh
+++ b/tests/Functional.linaro/fuego_test.sh
@@ -19,7 +19,10 @@ function test_pre_check {
     # vi ~/.ssh/config
     #  Host 192.167.1.99 <- replace with your boards ip address ($IPADDR)
     #    IdentityFile ~/.ssh/bbb_id_rsa
-    assert_define SSH_KEY "Please setup SSH_KEY on your board file (fuego-ro/boards/$NODE_NAME.board)"
+    # [Note] when running on localhost SSH_KEY is not required
+    if [ "$TRANSPORT" != "local" ]; then
+        assert_define SSH_KEY "Please setup SSH_KEY on your board file (fuego-ro/boards/$NODE_NAME.board)"
+    fi
 }
 
 function test_build {
@@ -57,7 +60,12 @@ function test_run {
         SKIPFLAG=""
     fi
 
-    test-runner -o ${LOGDIR} $test_or_plan_flag ${REPO_PATH}/$yaml_file $PARAMS -g $LOGIN@$IPADDR $SKIPFLAG -e
+    # SSH is not required when running on localhost
+    if [ "$TRANSPORT" != "local" ]; then
+        test-runner -o ${LOGDIR} $test_or_plan_flag ${REPO_PATH}/$yaml_file $PARAMS -g $LOGIN@$IPADDR $SKIPFLAG -e
+    else
+        test-runner -o ${LOGDIR} $test_or_plan_flag ${REPO_PATH}/$yaml_file $PARAMS $SKIPFLAG -e
+    fi
 }
 
 # FIXTHIS: the log directory is populated with a copy of the whole repository, clean unnecessary files
-- 
2.20.1

Re: [Fuego] fixes for the linaro and rt tests

[Tim.Bird]

> -----Original Message-----
> From: tho1.nguyendat@toshiba.co.jp <tho1.nguyendat@toshiba.co.jp>
> 
> Dear Tim,
> 
> I updated "patch [1/4] linaro: localhost does not require ssh"
> Could you please help me check the attachment?

OK.  I took a look at this.

I'll copy the code from the attachment here, so I can comment.

> From 14dc2fc73db6bc99c7ef2830bc8230481e64f427 Mon Sep 17 00:00:00 2001
> From: Nguyen Dat Tho <tho1.nguyendat@toshiba.co.jp>
> Date: Fri, 20 Aug 2021 10:51:37 +0900
> Subject: [PATCH] linaro: localhost does not require ssh
> 
> When running the test on localhost ssh is not required
> 
> Signed-off-by: Nguyen Dat Tho <tho1.nguyendat@toshiba.co.jp>
> Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
> ---
> tests/Functional.linaro/fuego_test.sh | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/Functional.linaro/fuego_test.sh b/tests/Functional.linaro/fuego_test.sh
> index 86d3c43..71c77e7 100755
> --- a/tests/Functional.linaro/fuego_test.sh
> +++ b/tests/Functional.linaro/fuego_test.sh
> @@ -19,7 +19,10 @@ function test_pre_check {
>      # vi ~/.ssh/config
>      #  Host 192.167.1.99 <- replace with your boards ip address ($IPADDR)
>      #    IdentityFile ~/.ssh/bbb_id_rsa
> -    assert_define SSH_KEY "Please setup SSH_KEY on your board file (fuego-ro/boards/$NODE_NAME.board)"
> +    # [Note] when running on localhost SSH_KEY is not required
> +    if [ "$TRANSPORT" != "local" ]; then
I would prefer to restrict the SSH_KEY check to only TRANSPORT=="ssh".

We have several other transports (including some very special custom ones), that
are password-less from Fuego's perspective.  For example 'ttc', 'lc', and 'ebf'.

So the only TRANSPORT that really requires that SSH_KEY be defined, in order
to be password-less is the 'ssh' TRANSPORT.

I converted this to:
if [ "$TRANSPORT" = "ssh" ]; then

Even this is a bit of a 'proxy' check, that might yield a false positive for the real attribute
you're trying to check for.  The real attribute is password-less ssh operations by the user
account which is running Fuego (ftc).

One way to achieve password-less operation is to the use SSH_KEY, the other is through
very insecure settings on the target board's sshd.config.  Yet another way is through
using 'sshpass'.  But Linaro's test_runner does not support an option for doing this.

In any event, testing TRANSPORT is a close enough proxy for the attribute you're trying
to discover, so the next line is fine.  If people have workarounds for password-less access
to a board, they can always just disable this assert_define.
> +        assert_define SSH_KEY "Please setup SSH_KEY on your board file (fuego-ro/boards/$NODE_NAME.board)"
> +    fi
>  }

Since we know that test_runner won't work with other Fuego Transports (serial, ebf, ttc, lc, ssh2serial, etc.)
I decided to add more checking here, and do an abort_job for all anything besides 'local' and 'ssh'.

> 
>  function test_build {
> @@ -57,7 +60,12 @@ function test_run {
>         SKIPFLAG=""
>      fi
>  
> -    test-runner -o ${LOGDIR} $test_or_plan_flag ${REPO_PATH}/$yaml_file $PARAMS -g $LOGIN@$IPADDR $SKIPFLAG -e
> +    # SSH is not required when running on localhost
> +    if [ "$TRANSPORT" != "local" ]; then
IMHO, This should also be: TRANSPORT == "ssh"
> +        test-runner -o ${LOGDIR} $test_or_plan_flag ${REPO_PATH}/$yaml_file $PARAMS -g $LOGIN@$IPADDR $SKIPFLAG -e
> +    else
> +        test-runner -o ${LOGDIR} $test_or_plan_flag ${REPO_PATH}/$yaml_file $PARAMS $SKIPFLAG -e
> +    fi
>  }
>  
>  # FIXTHIS: the log directory is populated with a copy of the whole repository, clean unnecessary files
> -- 
> 2.20.1

I went ahead and made the changes, and committed the patch.  Please give it a try
and let me know if it works to your satisfaction.
 -- Tim

Here's the eventual patch I committed:

commit f86d3d0235d476d4580b3e829af7795f758172e5
Author: Nguyen Dat Tho <tho1.nguyendat@toshiba.co.jp>
Date:   Fri Aug 20 10:51:37 2021 +0900

    linaro: localhost does not require ssh
    
    When running the test on ssh password-less ssh access to the
    board is required.  test-runner only supports local and ssh
    as the Fuego TRANSPORT.
    
    Signed-off-by: Nguyen Dat Tho <tho1.nguyendat@toshiba.co.jp>
    Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>

diff --git a/tests/Functional.linaro/fuego_test.sh b/tests/Functional.linaro/fuego_test.sh
index 86d3c43..eb6e933 100755
--- a/tests/Functional.linaro/fuego_test.sh
+++ b/tests/Functional.linaro/fuego_test.sh
@@ -19,7 +19,13 @@ function test_pre_check {
     # vi ~/.ssh/config
     #  Host 192.167.1.99 <- replace with your boards ip address ($IPADDR)
     #    IdentityFile ~/.ssh/bbb_id_rsa
-    assert_define SSH_KEY "Please setup SSH_KEY on your board file (fuego-ro/boards/$NODE_NAME.board)"
+    # [Note] when running on ssh, password-less ssh access is required
+    # check the TRANSPORT, and make sure SSH_KEY is provided if needed
+    if [ "$TRANSPORT" = "ssh" ]; then
+        assert_define SSH_KEY "Please setup SSH_KEY on your board file (fuego-ro/boards/$NODE_NAME.board)"
+    elif [ "$TRANSPORT" != "local" ]; then
+        abort_job "Linaro test_runner only works with TRANSPORT of 'ssh' or 'local', not '$TRANSPORT'"
+    fi
 }
 
 function test_build {
@@ -57,7 +63,12 @@ function test_run {
         SKIPFLAG=""
     fi
 
-    test-runner -o ${LOGDIR} $test_or_plan_flag ${REPO_PATH}/$yaml_file $PARAMS -g $LOGIN@$IPADDR $SKIPFLAG -e
+    # use -g option to test-runner, if using 'ssh' transport
+    if [ "$TRANSPORT" = "ssh" ]; then
+        test-runner -o ${LOGDIR} $test_or_plan_flag ${REPO_PATH}/$yaml_file $PARAMS -g $LOGIN@$IPADDR $SKIPFLAG -e
+    else
+        test-runner -o ${LOGDIR} $test_or_plan_flag ${REPO_PATH}/$yaml_file $PARAMS $SKIPFLAG -e
+    fi
 }
 
 # FIXTHIS: the log directory is populated with a copy of the whole repository, clean unnecessary files

> -----Original Message-----
> From: sangorrin daniel(サンゴリン ダニエル □ＳＷＣ◯ＡＣＴ) <daniel.sangorrin@toshiba.co.jp>
> Sent: Thursday, August 26, 2021 9:46 AM
> To: Tim.Bird@sony.com
> Cc: fuego@lists.linuxfoundation.org; nguyen dat tho(ＴＳＤＶ Ｅｎｇ　１) <tho1.nguyendat@toshiba.co.jp>
> Subject: RE: fixes for the linaro and rt tests
> 
> Hi Tim
> 
> Thanks for reviewing the patches
> 
> > -----Original Message-----
> > From: Tim.Bird@sony.com <Tim.Bird@sony.com>
> > > [PATCH 1/4] linaro: localhost does not require ssh
> > >
> > > This ones is self explanatory (no SSH if you use the local board)
> > This looks OK.  I'm wondering if there's another way to detect a local
> > operation.  Can we check the TRANSPORT (e.g. if [ $TRANSPORT = "local" ] ...)?
> > Checking the network address seems a bit iffy.
> 
> You are right, we will try with TRANSPORT or another variable and re-send the patch.
> 
> Thanks,
> Daniel