Skip to content

util/helper: add stderr param to ProcessWrapper#1787

Open
ImHoppy wants to merge 1 commit into
labgrid-project:masterfrom
ImHoppy:util/processwrapper
Open

util/helper: add stderr param to ProcessWrapper#1787
ImHoppy wants to merge 1 commit into
labgrid-project:masterfrom
ImHoppy:util/processwrapper

Conversation

@ImHoppy

@ImHoppy ImHoppy commented Dec 11, 2025

Copy link
Copy Markdown

Description

Add a stderr parameter to the ProcessWrapper, allowing the separation of stderr from the output. In my driver, I use this feature to ignore stderr by redirecting it to subprocess.DEVNULL

what do you use the feature for?
It allows calling ProcessWrapper without having stderr polluting the standard output.

how does labgrid benefit as a testing library from the feature?
This feature is used in my custom IpmiPowerDriver to check the status without having debug messages mixed.

how did you verify the feature works?
I tested the feature with a DUT in my labs.

if hardware is needed for the feature, which hardware is supported and which
hardware did you test with?

No specific hardware is required


Checklist

  • Documentation for the feature
  • Tests for the feature
  • The arguments and descriptions in doc/configuration.rst have been updated
  • Add a section in doc/usage.rst to explain how to use the feature
  • Add a section in doc/development.rst to explain how developers can use the feature
  • PR has been tested locally
  • Man pages have been regenerated

@ImHoppy
ImHoppy force-pushed the util/processwrapper branch from d219663 to c6430e2 Compare December 12, 2025 08:25
@codecov

codecov Bot commented Dec 17, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 45.4%. Comparing base (b666734) to head (0e84b68).
⚠️ Report is 104 commits behind head on master.

Additional details and impacted files
@@          Coverage Diff           @@
##           master   #1787   +/-   ##
======================================
  Coverage    45.4%   45.4%           
======================================
  Files         177     177           
  Lines       13863   13865    +2     
======================================
+ Hits         6300    6302    +2     
  Misses       7563    7563           
Flag Coverage Δ
3.10 45.4% <100.0%> (+<0.1%) ⬆️
3.11 45.4% <100.0%> (+<0.1%) ⬆️
3.12 45.4% <100.0%> (+<0.1%) ⬆️
3.13 45.4% <100.0%> (+<0.1%) ⬆️
3.14 45.4% <100.0%> (+<0.1%) ⬆️

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

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

@jluebbe

jluebbe commented Dec 17, 2025

Copy link
Copy Markdown
Member

Please add the description to the commit.

Emantor
Emantor previously approved these changes Jan 30, 2026
@Emantor
Emantor requested a review from jluebbe January 30, 2026 13:26
@ImHoppy
ImHoppy force-pushed the util/processwrapper branch from d286991 to bf25dea Compare March 2, 2026 14:49
@Emantor
Emantor force-pushed the util/processwrapper branch from bf25dea to 0e84b68 Compare March 4, 2026 11:13

@Bastian-Krause Bastian-Krause left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Passing stderr=subprocess.DEVNULL or stderr=None is fine. But stderr=subprocess.PIPE would never drain the pipe nor close it. That's a problem. Maybe raise NotImplementedError if stderr is something else than None or subprocess.DEVNULL?

@Bastian-Krause Bastian-Krause added the needs author info Requires more information from the PR/Issue author label Jul 8, 2026
Allow separation of stderr from the output. For example, stderr can be
ignored by redirecting it to subprocess.DEVNULL or prevent capturing by
passing None, similar to how subprocess handles it.

Signed-off-by: Hoppy <dev.hoppy@gmail.com>
[bst: use default similar to subprocess: subprocess.STDOUT means merging
stdout and stderr, None means no capturing; raise on unsupported stderr
values]
Signed-off-by: Bastian Krause <bst@pengutronix.de>
@Bastian-Krause Bastian-Krause removed the needs author info Requires more information from the PR/Issue author label Jul 17, 2026
@Bastian-Krause

Copy link
Copy Markdown
Member

Force-pushed: I've added the requested changes myself:

  • use default similar to subprocess: subprocess.STDOUT means merging stdout and stderr, None means no capturing
  • raise on unsupported stderr values
  • adjusted the commit message accordingly

Attention: Since you're not using a real name, I've created #1940 to address that. Do not merge until #1940 is merged.

@Bastian-Krause Bastian-Krause added the do not merge yet Something's blocking this from being merged label Jul 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do not merge yet Something's blocking this from being merged enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants