labgrid-project / labgrid

Embedded systems control library for development, testing and installation
https://labgrid.readthedocs.io/
Other
327 stars 164 forks source link

Selecting the target in pytest #1375

Open sjg20 opened 4 months ago

sjg20 commented 4 months ago

The pytest code for selecting a target currently looks like this (fixtures.py):

@pytest.fixture(scope="session")
def target(env):
    """Return the default target `main` configured in the supplied
    configuration file."""
    target = env.get_target()
    if target is None:
        raise UserError("Using target fixture without 'main' target in config")

    return target

This works well with a single place, called 'main'. But when there are multiple places, we need a way to select the correct one.

I propose adding a --lg-target option to fixtures.py so that this can be specified.

Then my script to check a particular commit (use with 'git bisect run') can be something like this:

pytest --lg-env env_rpi_try.cfg \ --lg-target ${board} \ --lg-log --lg-colored-steps \ -k test_uboot_smoke

Does this seems reasonable?

Emantor commented 4 months ago

The intention here is for your pytest files to provide fixtures for individual devices. Something like:

@pytest.fixture(scope="session")
def devicea(env):
    return env.get_target("devicea")

@pytest.fixture(scope="session")
def deviceb(env):
    return env.get_target("deviceb")

And than use those as test fixtures.

sjg20 commented 4 months ago

That's fine, but in my case I have a lab of 33 boards and the tests for each are the same...so the above becomes quite unwieldy. I would need to duplicate all the tests 33 times just so they can use a different board

Emantor commented 4 months ago

In this case you'll want to pass a different environment configuration per test. You can keep the same tests, but the target changes per environment file.

sjg20 commented 4 months ago

Again, this becomes fiddly. I would like to have the env for the lab in one file, rather than editing so many files.

So you are not keen on the feature I propose?

Emantor commented 4 months ago

Again, this becomes fiddly. I would like to have the env for the lab in one file, rather than editing so many files.

So you are not keen on the feature I propose?

No, unfortunately it does not fit the locking model for labgrid. If you use the pytest plugin for tests, labgrid requires all places which are used inside the environment file to be locked to your user. This works well if you are the only one using the lab, however does not work at all if you have multiple users accessing different hardware in the same infrastructure. That is why different environment files are required if the same tests are going to run on different hardware.

sjg20 commented 4 months ago

Oh dear I didn't know that. Can that be changed? To me it seem better to lock a place when it is needed, rather than locking all places mentioned in the environment file?

I am starting to see that each command-line program has its own file...is that right?

  1. export.yaml for the exporter, contains the devices
  2. places.yaml for the crossbar, contains the places and their devices
  3. env.cfg for the client, contains the devices including the actually class names inside labgrid

Do I have that right?

Emantor commented 4 months ago
  1. Exporter YAML configuration expose resources (devices) to the network and registers them with the coordinator. The exporter also updates availability on the exporter side, you can register resources which may be available and labgrid will correctly mark them as available as they appear/are plugged in.
  2. The coordinator places YAML configuration assembles resources into places. Places lock all resources when they are acquired, preventing access to the wrong device and alerting the user on misconfiguration, i.e. two devices wanting to use the same resource.
  3. The device-configuration YAML describes targets for test runs or to use with a local client. The resources within a target can be described as a RemotePlace and labgrid will than fetch the resource information from the coordinator, see Remote Resources and places in the documentation. The device configuration can also contain a list of local resources to setup the test target, enabling use of labgrid without the remote infrastructure (no coordinator, no exporter). The Running your first test chapter within the labgrid docs uses this setup.
jluebbe commented 4 months ago

Again, this becomes fiddly. I would like to have the env for the lab in one file, rather than editing so many files.

So you are not keen on the feature I propose?

This would be contrary to the intention we have with environment yaml files: they should describe the environment in which a test suite (or other labgrid-based tool) runs. So it maps a role in that test suite to a specific device (or an instance from a group via reservations). At the driver level, it contains the necessary information that may be different when e.g. testing the same software on different boards. Via the images: key, it can refer to board-specific images. With the imports: key, it can pull in a Strategy to handle different ways to booting the board.

If a test suite uses multiple devices in different roles, for example a network sender and receiver running on different boards, an environment would define multiple targets (e.g. sender and receiver). They could refer to specific places or instances from a shared or separate groups (although that part is not fully implemented yet).

That way, one environment yaml can be used to run the same test suite (or other tool) against many different boards in one lab or even against different labs (which might have tools in other paths or use different power control mechanisms). In the end this leads to reusable test suites, as the details of how to access the target(s) are abstracted through the environment.

sjg20 commented 2 months ago

Having now implemented things a bit more, here is my response.

My environment file contains a load of roles which map to particular devices, mostly 1:1. Without the ability to specify the role, I must put each of these environments in a separate file, perhaps using the role as the filename.

From what I can see, combining the environment into one file (and providing a way to select a particular role) makes quite a bit of sense.

Emantor commented 2 months ago

Having now implemented things a bit more, here is my response.

My environment file contains a load of roles which map to particular devices, mostly 1:1. Without the ability to specify the role, I must put each of these environments in a separate file, perhaps using the role as the filename.

From what I can see, combining the environment into one file (and providing a way to select a particular role) makes quite a bit of sense.

This does work as long as only one user is used in the lab and won't if multiple users should be supported. The pytest plugin requires that all RemotePlaces inside the device-configuration are acquired by the user. The reason is simple, not acquired places can be changed by other users, so you could get test failures by accident because another user modified the place.

Another reason why only the required places should be described inside the configuration file was described by @jluebbe above, Labgrid is not intended to be a build-system (see Design Decisions), so you need a place where you can hand over artifact locations to labgrid and this is intended to be done in the device configuration yaml file.

sjg20 commented 1 month ago

Re multiple users, I am certainly getting errors if I try to run two tests at the same time on the same board, so I believe this issue is resolved with my PR. I added a -a option to automatically acquire the place and then release it afterwards.

If I am to replace the use of tbot to run the lab, I do need to be able to use things interactively. Each board type has its own set of files which are needed and its own method of writing them to the board storage, etc. That logic belongs in the U-Boot strategy IMO. Putting it outside Labgrid entirely just involves writing all the logic in another place and would be quite a significant limitation to the integration.