labgrid icon indicating copy to clipboard operation
labgrid copied to clipboard

[RFC] pytestplugin: make "target" a parametrized fixture

Open QSchulz opened this issue 1 year ago • 5 comments

Description

This allows users to run their pytest test scripts with an LG_ENV/--lg-env that has multiple targets instead of forcing the user to define one named "main" in order to run the tests.

One of the downsides of this implementation is that autogenerated doc wouldn't define the "target" fixture anymore as it is entirely programmatically defined.

I'm investigating using labgrid for validating U-Boot with multiple pytest tests inside a single file, for multiple targets. Not sure it's the best practice but I intuitively went for that.

Essentially:

---
targets:
    px30-ringneck-1:
        features:
            - something
        options:
            emmc_path: '/mmc@ff370000'
    [...]
    rk3399-puma-1:
        features:
            - bootablespi
            - somethingelse
        options:
            emmc_path: '/mmc@ff390000'
    [...]

# bootloader_command fixture depends on target fixture

def test_uboot_initial(bootloader_flash, bootloader_command):
    stdout = bootloader_command.run_check('version')
    assert 'U-Boot' in '\n'.join(stdout)


@pytest.mark.lg_feature('bootablespi')
def test_uboot_properfromspi(bootloader_flash, bootloader_command, env, target):
    [...]

I believe this would allow users to have one big YAML file for all their targets, or merge YAML files for the same target into one YAML file so with one call to pytest, multiple targets can be checked. It would also allow to have one big pytest file with all tests to run on all targets, with parametrizing their arguments via target:options/target:features.

I checked the output of pytest --lg-env local.yaml --collect-only test.py and the tests are listed twice, once for each target in the YAML file. I then ran the tests and because I don't have enough HW for the infra for two boards, it complained for the second board that it was lacking resources, but the first board ran the tests just fine.

Checklist

  • [ ] Documentation for the feature (N/A?)
  • [ ] Tests for the feature (N/A?)
  • [ ] PR has been tested (somewhat; I don't have enough HW for the setup of two boards but the second board complains about lacking resources when I run pytest)

RFC because maybe there was a reason for only allowing a "main"-named target to run the pytest tests and also because I wasn't really able to make sure the second target was used for the second test run (though there are good hints it is).

Also I believe this may be a bit too enthusiastic, we probably want to add a selection mechanism instead of simply adding all targets from the file? e.g. maybe add an --lg-target option which can be repeated to collect which targets to include.

Also not entirely sure which part of the documentation we would need to update for that.

QSchulz avatar Dec 03 '24 08:12 QSchulz

Codecov Report

Attention: Patch coverage is 85.71429% with 2 lines in your changes missing coverage. Please review.

Project coverage is 56.2%. Comparing base (d98677c) to head (1f54a62). Report is 2 commits behind head on master.

:white_check_mark: All tests successful. No failed tests found.

Files with missing lines Patch % Lines
labgrid/pytestplugin/fixtures.py 84.6% 2 Missing :warning:
Additional details and impacted files
@@          Coverage Diff           @@
##           master   #1554   +/-   ##
======================================
  Coverage    56.1%   56.2%           
======================================
  Files         170     170           
  Lines       13225   13232    +7     
======================================
+ Hits         7432    7438    +6     
- Misses       5793    5794    +1     
Flag Coverage Δ
3.10 56.2% <85.7%> (+<0.1%) :arrow_up:
3.11 56.2% <85.7%> (+<0.1%) :arrow_up:
3.12 56.2% <85.7%> (+<0.1%) :arrow_up:
3.13 56.1% <85.7%> (+<0.1%) :arrow_up:
3.9 56.2% <85.7%> (+<0.1%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Dec 03 '24 08:12 codecov[bot]

We are also interested to join yaml configs of different boards, but selecting would be better approach for our usecase. env.get_target() is already prepared for a resource name.

jremmet avatar Dec 03 '24 08:12 jremmet

I'm investigating using labgrid for validating U-Boot with multiple pytest tests inside a single file, for multiple targets. Not sure it's the best practice but I intuitively went for that. … Essentially:

---
targets:
    px30-ringneck-1:
        features:
            - something
        options:
            emmc_path: '/mmc@ff370000'
    [...]
    rk3399-puma-1:
        features:
            - bootablespi
            - somethingelse
        options:
            emmc_path: '/mmc@ff390000'
    [...]

I believe this would allow users to have one big YAML file for all their targets, or merge YAML files for the same target into one YAML file so with one call to pytest, multiple targets can be checked. It would also allow to have one big pytest file with all tests to run on all targets, with parametrizing their arguments via target:options/target:features.

RFC because maybe there was a reason for only allowing a "main"-named target to run the pytest tests and also because I wasn't really able to make sure the second target was used for the second test run (though there are good hints it is).

The use-case for defining multiple targets in the same environment is tests which need to access multiple targets. For example one target creates a WiFi AP and the other target connects to it. If you have one set of tests (perhaps generalized via the target:options/features mechanism) that you want to run on multiple different boards, use multiple environment yaml files.

If you'd use a parameterized fixture, pytest (at least without xdist, which isn't really compatible with labgrid) would run all test sequentially, although there is no conflict between tests on different targets. With one environment file per test setup (so only one target in the common case), you can simply run multiple pytest process in parallel (e.g. via a CI runner).

Furthermore, by not mixing multiple unrelated targets in one file, it's easy to see everything that's need for a test to run. In the wifi test example above, you'd maybe have one env for CI and another one for your personal lab at home. Both would use the same target names, so the differences would be abstracted for the pytest code. You'd define your own pair of target fixtures in conftest.py (e.g. target_ap and target_client).

What could certainly be improved is a way to specify which remote place to use in the environment, perhaps via reservations and tags. That would allow using the same environment file with multiple labs, at least if the places were built similarly enough.

jluebbe avatar Dec 03 '24 09:12 jluebbe

What could certainly be improved is a way to specify which remote place to use in the environment, perhaps via reservations and tags. That would allow using the same environment file with multiple labs, at least if the places were built similarly enough.

Not sure what you mean here as there's already:

---
targets:
  main:
    resources:
      RemotePlace:
        name: 'my-place'

And you can use templating with LG_PLACE if desired.

I guess we can close this PR then as I understand this is clearly not a fit in labgrid's architecture.

I don't like that the target needs to be named "main" and I think we could make this selectable or define it in the environment file itself, e.g.:

---
dut: ringneck-px30_1
targets:
  ringneck-px30_1:
  [...]
  wifi-ap_1:
  [...]

I'm not sure if the name of the target is used in error messages but if we do, having "main" in all environment files seem a bit counterproductive for debugging.

But also, this has nothing to do with this MR and it's not a blocker for me so probably won't work on it unless it bothers me a lot in the future :)

QSchulz avatar Dec 03 '24 10:12 QSchulz

I really like the idea of the --lg-target option that can default to main. We currently are in the situation where we want to have all our targets in one big file that is easily shared among several repos containing that run labgrid/pytest tests as many of our test cases don't require any special HW config.

siredmar avatar Jun 23 '25 10:06 siredmar