remote/client: make acquire/release operate on all RemotePlaces in env

[Bastian-Krause]

Description
If an environment config with multiple RemotePlaces is given to labgrid-client via -c/--config, only the first RemotePlace is used on acquire/lock and release/unlock.

Having multiple RemotePlaces is meant for multi place testing, so all RemotePlaces need to be acquired/released.

This is now implemented in a best effort way: If an error occurs, the remaining places are still tried to be acquired/released. A combined error message is shown if the operation failed on multiple places.
When called with -d, tracebacks of all errors are shown. If any of the operations failed, the process exits with 1.

CI pipelines should use this pattern:

$ export LG_ENV=env.yaml
$ labgrid-client release || true
$ labgrid-client acquire

Checklist

• Documentation for the feature
• Tests for the feature
• PR has been tested

[codecov]

Codecov Report

❌ Patch coverage is 15.90909% with 37 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.3%. Comparing base (2daa13d) to head (829079f).
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
labgrid/remote/client.py	15.9%	37 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##           master   #1748     +/-   ##
========================================
- Coverage    45.4%   45.3%   -0.1%     
========================================
  Files         172     172             
  Lines       13503   13541     +38     
========================================
+ Hits         6131    6137      +6     
- Misses       7372    7404     +32     

Flag	Coverage Δ
3.10	45.3% <15.9%> (-0.1%)	⬇️
3.11	45.3% <15.9%> (-0.1%)	⬇️
3.12	45.3% <15.9%> (-0.1%)	⬇️
3.13	45.2% <15.9%> (-0.1%)	⬇️
3.9	45.3% <15.9%> (-0.1%)	⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[dstrub18]

Cool! Great work, Basti!

This is now implemented in a best effort way: If an error occurs, the remaining places are still tried to be acquired/released

You could make a case for failing early if you fail to acquire just 1 place. If an env file has multiple places, it is likely that all of them are needed for what you're trying to test.

I'm not quite sure how other labgrid users would want this to be implemented. If there are any strong voices from within pengutronix or other users, I'd await their review.

[Bastian-Krause]

Cool! Great work, Basti!

Thanks!

This is now implemented in a best effort way: If an error occurs, the remaining places are still tried to be acquired/released

You could make a case for failing early if you fail to acquire just 1 place. If an env file has multiple places, it is likely that all of them are needed for what you're trying to test.

I'd like acquire and release work the same way. If release fails early, the operation will leave places acquired. To overcome this, we would probably need some kind of rollback mechanism for acquire if any of the places fail (but that doesn't really make sense for release, does it?), – but I'm hesitant to implement this. That's why I chose the best effort way.

I'm not quite sure how other labgrid users would want this to be implemented. If there are any strong voices from within pengutronix or other users, I'd await their review.

Yes, input welcome! Since the whole multi place acquire/release is not atomic in any way, we should not over-engineer this, though.

[nlabriet]

It's nice, but make sure to limit the acquire/release to the place given by --place or LG_PLACE if provided.
Otherwise, all the boards in my test bench will get acquired, not only the one I want.

[jluebbe]

It's nice, but make sure to limit the acquire/release to the place given by --place or LG_PLACE if provided. Otherwise, all the boards in my test bench will get acquired, not only the one I want.

One test environment file should only contain the places you want to use to use in combination for testing the combined system.

[nlabriet]

One test environment file should only contain the places you want to use to use in combination for testing the combined system.

I missed this.
As I have a peculiar setting (see #1194), I will try to set the BeagleBone Black resources as resources to the DUT. This will allow me to have an environment file with only one main target.