RFC: Add BackgroundProcessProtocol, let SSHDriver/ShellDriver implement it

[Bastian-Krause]

Description The CommandProtocol allows executing commands on the target. CommandProtocol.run() blocks until the process terminated and stdout/stderr/exit code are returned. Some use cases require non-blocking communication with the process because something else needs to be done concurrently (think of a listening netcat on the target and another netcat on the testing host connecting to it). Maybe even multiple commands need to be run concurrently (if the underlying communication mechanism allows that).

The BackgroundProcessProtocol allows exactly that. It defines a method to start a command in background, a method to read stdout/stderr from that background process and a method to check if the background process terminated (and if so with which exit code). The abstract BackgroundProcessProtocol provides an example how to use it. Users of this protocol have to implement the timeout handling themselves. In order to simplify it labgrid.util.timeout can now be used as a context manager (see example).

This PR implements this protocol for the SSHDriver and the ShellDriver. These driver probably benefit most from this feature.

Checklist

• [x] Documentation for the feature (in the BackgroundProcessProtocol)
• [ ] Tests for the feature
• [x] The arguments and description in doc/configuration.rst have been updated
• [ ] Add a section on how to use the feature to doc/usage.rst
• [ ] Add a section on how to use the feature to doc/development.rst
• [ ] CHANGES.rst has been updated
• [x] PR has been tested

[codecov[bot]]

Codecov Report

Merging #552 into master will decrease coverage by 0.3%. The diff coverage is 35.3%.

@@           Coverage Diff            @@
##           master    #552     +/-   ##
========================================
- Coverage    56.8%   56.5%   -0.4%     
========================================
  Files         127     128      +1     
  Lines        8730    8872    +142     
========================================
+ Hits         4965    5015     +50     
- Misses       3765    3857     +92

Impacted Files	Coverage Δ
labgrid/protocol/__init__.py	100% <100%> (ø)	:arrow_up:
labgrid/protocol/backgroundprocessprotocol.py	100% <100%> (ø)
labgrid/driver/sshdriver.py	38.8% <26.3%> (-7.2%)	:arrow_down:
labgrid/util/expect.py	73.9% <28.5%> (-19.9%)	:arrow_down:
labgrid/driver/shelldriver.py	29.3% <35.6%> (+1%)	:arrow_up:
labgrid/util/timeout.py	86.3% <50%> (-13.7%)	:arrow_down:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 526cfe5...78775dc. Read the comment docs.

[Bastian-Krause]

Thanks for your input!

An internal discussion lead to the following:

• the BackgroundProcessProtocol will be merged into the CommandProtocol, all mentions of "background" will be removed
• instead of having read/poll/stop methods, separate Popen-like (per-driver) process handle objects with these methods (as well as expect()) will be introduced (defined by an abstract process handle in labgrid.protocol.commandprotocol)
• read(timeout=) will be read(wait=) to minimize confusion of fatal timeouts and non-fatal waits
• the read method will try to match everything but the marker, then it will match the marker (as opposed to matching the marker, reading "before" and then ShellDriver.console._expect.read(size=len(before)))
• the read method will return bytes, no VT100 substitution, no decoding, no newline translation (we cannot be sure that we read complete sequences, so leave this to the user)
• CommandProtocol.run() will perform these clean ups as before
• when instantiating process handle the driver (to allow calls to ShellDriver.expect() and access of other driver instance variables) and a timeout will be passed to it
• the process handle will be usable as a context manager
• if available coreutils' timeout will be used to make things easier
• the SSHDriver will use pexpect.spawn() instead of subprocess.Popen()

[jremmet]

the read method will try to match everything but the marker, then it will match the marker (as opposed to matching the marker, reading "before" and then ShellDriver.console._expect.read(size=len(before)))

I tried to work on this issue. But due to the non greedy behaviour in expect and that a negative lookahead will block till the marker is read, I run into issues. Especially if read with wait=0. My solution still uses read and also check if "before" contains the start of a marker. WIP/shelldriver_popen

[dnltz]

Hey @Bastian-Krause, is this draft still planned or dropped? We see a need for this and would like to remove our workaround for this.

[Bastian-Krause]

@dnltz I worked for a customer on this. Unfortunately the concrete use case vanished into thin air, so the work stalled here. If you are interested to work on this (see current state of discussions above), please do so. If you want us to work on this, please contact our sales team.

[Bastian-Krause]

Superseded by #835.