diff mbox series

[v5,08/20] test: Introduce the concept of a role

Message ID 20240828220846.1205813-9-sjg@chromium.org
State Changes Requested
Delegated to: Tom Rini
Headers show
Series labgrid: Provide an integration with Labgrid | expand

Commit Message

Simon Glass Aug. 28, 2024, 10:08 p.m. UTC
In Labgrid there is the concept of a 'role', which is similar to the
U-Boot board ID in U-Boot's pytest subsystem.

The role indicates both the target and information about the U-Boot
build to use. It can also provide any amount of other configuration.
The information is obtained using the 'labgrid-client query' operation.

Make use of this in tests, so that only the role is required in gitlab
and other situations. The board type and other things can be queried
as needed.

Use a new 'u-boot-test-getrole' script to obtain the requested
information.

With this it is possible to run lab tests in gitlab with just a single
'ROLE' variable for each board.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v5:
- Add a few more comments
- Comment out the debugging, which might be useful later

 test/py/conftest.py | 38 ++++++++++++++++++++++++++++++++++----
 1 file changed, 34 insertions(+), 4 deletions(-)

Comments

neil.armstrong@linaro.org Aug. 29, 2024, 2:19 p.m. UTC | #1
Hi,

On 29/08/2024 00:08, Simon Glass wrote:
> In Labgrid there is the concept of a 'role', which is similar to the
> U-Boot board ID in U-Boot's pytest subsystem.
> 
> The role indicates both the target and information about the U-Boot
> build to use. It can also provide any amount of other configuration.
> The information is obtained using the 'labgrid-client query' operation.
> 
> Make use of this in tests, so that only the role is required in gitlab
> and other situations. The board type and other things can be queried
> as needed.
> 
> Use a new 'u-boot-test-getrole' script to obtain the requested
> information.
> 
> With this it is possible to run lab tests in gitlab with just a single
> 'ROLE' variable for each board.

Can't this be in the hook script ? I mean allmost no CI system have a 1:1
usage of board_type & board_identity, but we use those fields and transform
them accordingly.

u-boot-test-getrole is labgrid only, all script receives board_type & board_identity,
so why add some labgrind specific python here ?

Neil

> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
> Changes in v5:
> - Add a few more comments
> - Comment out the debugging, which might be useful later
> 
>   test/py/conftest.py | 38 ++++++++++++++++++++++++++++++++++----
>   1 file changed, 34 insertions(+), 4 deletions(-)
> 
> diff --git a/test/py/conftest.py b/test/py/conftest.py
> index 6547c6922c6..03dfd8ab562 100644
> --- a/test/py/conftest.py
> +++ b/test/py/conftest.py
> @@ -23,6 +23,7 @@ from pathlib import Path
>   import pytest
>   import re
>   from _pytest.runner import runtestprotocol
> +import subprocess
>   import sys
>   
>   # Globals: The HTML log file, and the connection to the U-Boot console.
> @@ -79,6 +80,7 @@ def pytest_addoption(parser):
>       parser.addoption('--gdbserver', default=None,
>           help='Run sandbox under gdbserver. The argument is the channel '+
>           'over which gdbserver should communicate, e.g. localhost:1234')
> +    parser.addoption('--role', help='U-Boot board role (for Labgrid)')
>       parser.addoption('--no-prompt-wait', default=False, action='store_true',
>           help="Assume that U-Boot is ready and don't wait for a prompt")
>   
> @@ -130,12 +132,40 @@ def get_details(config):
>               str: Build directory
>               str: Source directory
>       """
> -    board_type = config.getoption('board_type')
> -    board_identity = config.getoption('board_identity')
> +    role = config.getoption('role')
> +
> +    # Get a few provided parameters
>       build_dir = config.getoption('build_dir')
> +    if role:
> +        # When using a role, build_dir and build_dir_extra are normally not set,
> +        # since they are picked up from Labgrid via the u-boot-test-getrole
> +        # script
> +        board_identity = role
> +        cmd = ['u-boot-test-getrole', role, '--configure']
> +        env = os.environ.copy()
> +        if build_dir:
> +            env['U_BOOT_BUILD_DIR'] = build_dir
> +        proc = subprocess.run(cmd, capture_output=True, encoding='utf-8',
> +                              env=env)
> +        if proc.returncode:
> +            raise ValueError(proc.stderr)
> +        # For debugging
> +        # print('conftest: lab:', proc.stdout)
> +        vals = {}
> +        for line in proc.stdout.splitlines():
> +            item, value = line.split(' ', maxsplit=1)
> +            k = item.split(':')[-1]
> +            vals[k] = value
> +        # For debugging
> +        # print('conftest: lab info:', vals)
> +        board_type, default_build_dir, source_dir = (vals['board'],
> +            vals['build_dir'], vals['source_dir'])
> +    else:
> +        board_type = config.getoption('board_type')
> +        board_identity = config.getoption('board_identity')
>   
> -    source_dir = os.path.dirname(os.path.dirname(TEST_PY_DIR))
> -    default_build_dir = source_dir + '/build-' + board_type
> +        source_dir = os.path.dirname(os.path.dirname(TEST_PY_DIR))
> +        default_build_dir = source_dir + '/build-' + board_type
>       if not build_dir:
>           build_dir = default_build_dir
>
Simon Glass Aug. 29, 2024, 3:01 p.m. UTC | #2
Hi Neil,

On Thu, 29 Aug 2024 at 08:19, <neil.armstrong@linaro.org> wrote:
>
> Hi,
>
> On 29/08/2024 00:08, Simon Glass wrote:
> > In Labgrid there is the concept of a 'role', which is similar to the
> > U-Boot board ID in U-Boot's pytest subsystem.
> >
> > The role indicates both the target and information about the U-Boot
> > build to use. It can also provide any amount of other configuration.
> > The information is obtained using the 'labgrid-client query' operation.
> >
> > Make use of this in tests, so that only the role is required in gitlab
> > and other situations. The board type and other things can be queried
> > as needed.
> >
> > Use a new 'u-boot-test-getrole' script to obtain the requested
> > information.
> >
> > With this it is possible to run lab tests in gitlab with just a single
> > 'ROLE' variable for each board.
>
> Can't this be in the hook script ? I mean allmost no CI system have a 1:1
> usage of board_type & board_identity, but we use those fields and transform
> them accordingly.

Thanks for looking at this series.

I don't really understand this comment...can you expand a bit? The
role corresponds to the board_identity in test.py. If you are
suggesting that we should drop it, then that would mean we would not
be able to support one board with two different U-Boot builds.

>
> u-boot-test-getrole is labgrid only, all script receives board_type & board_identity,
> so why add some labgrind specific python here ?

Because with this integration, board_type is not needed*. Providing it
is redundant since it is 100% determined by the board_identify. Also,
some other things are needed, like where the build is, for interactive
use.

This integration is intended to support:

a) The existing use cases and labs (without modification to
U-Boot...the u-boot-test-hooks changes should not affect things)
b) Moving that lab to Labgrid without losing functionality
c) Interactive use (the ub-int and ub-pyt, etc. scripts)

Regards,
SImon

> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> > Changes in v5:
> > - Add a few more comments
> > - Comment out the debugging, which might be useful later
> >
> >   test/py/conftest.py | 38 ++++++++++++++++++++++++++++++++++----
> >   1 file changed, 34 insertions(+), 4 deletions(-)
> >
> > diff --git a/test/py/conftest.py b/test/py/conftest.py
> > index 6547c6922c6..03dfd8ab562 100644
> > --- a/test/py/conftest.py
> > +++ b/test/py/conftest.py
> > @@ -23,6 +23,7 @@ from pathlib import Path
> >   import pytest
> >   import re
> >   from _pytest.runner import runtestprotocol
> > +import subprocess
> >   import sys
> >
> >   # Globals: The HTML log file, and the connection to the U-Boot console.
> > @@ -79,6 +80,7 @@ def pytest_addoption(parser):
> >       parser.addoption('--gdbserver', default=None,
> >           help='Run sandbox under gdbserver. The argument is the channel '+
> >           'over which gdbserver should communicate, e.g. localhost:1234')
> > +    parser.addoption('--role', help='U-Boot board role (for Labgrid)')
> >       parser.addoption('--no-prompt-wait', default=False, action='store_true',
> >           help="Assume that U-Boot is ready and don't wait for a prompt")
> >
> > @@ -130,12 +132,40 @@ def get_details(config):
> >               str: Build directory
> >               str: Source directory
> >       """
> > -    board_type = config.getoption('board_type')
> > -    board_identity = config.getoption('board_identity')
> > +    role = config.getoption('role')
> > +
> > +    # Get a few provided parameters
> >       build_dir = config.getoption('build_dir')
> > +    if role:
> > +        # When using a role, build_dir and build_dir_extra are normally not set,
> > +        # since they are picked up from Labgrid via the u-boot-test-getrole
> > +        # script
> > +        board_identity = role
> > +        cmd = ['u-boot-test-getrole', role, '--configure']
> > +        env = os.environ.copy()
> > +        if build_dir:
> > +            env['U_BOOT_BUILD_DIR'] = build_dir
> > +        proc = subprocess.run(cmd, capture_output=True, encoding='utf-8',
> > +                              env=env)
> > +        if proc.returncode:
> > +            raise ValueError(proc.stderr)
> > +        # For debugging
> > +        # print('conftest: lab:', proc.stdout)
> > +        vals = {}
> > +        for line in proc.stdout.splitlines():
> > +            item, value = line.split(' ', maxsplit=1)
> > +            k = item.split(':')[-1]
> > +            vals[k] = value
> > +        # For debugging
> > +        # print('conftest: lab info:', vals)
> > +        board_type, default_build_dir, source_dir = (vals['board'],
> > +            vals['build_dir'], vals['source_dir'])
> > +    else:
> > +        board_type = config.getoption('board_type')
> > +        board_identity = config.getoption('board_identity')
> >
> > -    source_dir = os.path.dirname(os.path.dirname(TEST_PY_DIR))
> > -    default_build_dir = source_dir + '/build-' + board_type
> > +        source_dir = os.path.dirname(os.path.dirname(TEST_PY_DIR))
> > +        default_build_dir = source_dir + '/build-' + board_type
> >       if not build_dir:
> >           build_dir = default_build_dir
> >
>

* If building is needed, then you don't need CROSS_COMPILE, BL31, TEE,
etc. either. All of that is handled within Labgrid configuration.
diff mbox series

Patch

diff --git a/test/py/conftest.py b/test/py/conftest.py
index 6547c6922c6..03dfd8ab562 100644
--- a/test/py/conftest.py
+++ b/test/py/conftest.py
@@ -23,6 +23,7 @@  from pathlib import Path
 import pytest
 import re
 from _pytest.runner import runtestprotocol
+import subprocess
 import sys
 
 # Globals: The HTML log file, and the connection to the U-Boot console.
@@ -79,6 +80,7 @@  def pytest_addoption(parser):
     parser.addoption('--gdbserver', default=None,
         help='Run sandbox under gdbserver. The argument is the channel '+
         'over which gdbserver should communicate, e.g. localhost:1234')
+    parser.addoption('--role', help='U-Boot board role (for Labgrid)')
     parser.addoption('--no-prompt-wait', default=False, action='store_true',
         help="Assume that U-Boot is ready and don't wait for a prompt")
 
@@ -130,12 +132,40 @@  def get_details(config):
             str: Build directory
             str: Source directory
     """
-    board_type = config.getoption('board_type')
-    board_identity = config.getoption('board_identity')
+    role = config.getoption('role')
+
+    # Get a few provided parameters
     build_dir = config.getoption('build_dir')
+    if role:
+        # When using a role, build_dir and build_dir_extra are normally not set,
+        # since they are picked up from Labgrid via the u-boot-test-getrole
+        # script
+        board_identity = role
+        cmd = ['u-boot-test-getrole', role, '--configure']
+        env = os.environ.copy()
+        if build_dir:
+            env['U_BOOT_BUILD_DIR'] = build_dir
+        proc = subprocess.run(cmd, capture_output=True, encoding='utf-8',
+                              env=env)
+        if proc.returncode:
+            raise ValueError(proc.stderr)
+        # For debugging
+        # print('conftest: lab:', proc.stdout)
+        vals = {}
+        for line in proc.stdout.splitlines():
+            item, value = line.split(' ', maxsplit=1)
+            k = item.split(':')[-1]
+            vals[k] = value
+        # For debugging
+        # print('conftest: lab info:', vals)
+        board_type, default_build_dir, source_dir = (vals['board'],
+            vals['build_dir'], vals['source_dir'])
+    else:
+        board_type = config.getoption('board_type')
+        board_identity = config.getoption('board_identity')
 
-    source_dir = os.path.dirname(os.path.dirname(TEST_PY_DIR))
-    default_build_dir = source_dir + '/build-' + board_type
+        source_dir = os.path.dirname(os.path.dirname(TEST_PY_DIR))
+        default_build_dir = source_dir + '/build-' + board_type
     if not build_dir:
         build_dir = default_build_dir