[None][fix] Pipe stderr separately in subprocess calls to improve error reporting in Allure (#14750)

[yufeiwu-nv]

Signed-off-by: yufeiwu-nv 230315618+yufeiwu-nv@users.noreply.github.com

Summary by CodeRabbit

• Tests
  • Enhanced error output capture in performance test runners to improve diagnostic information collection.

Description

Test Coverage

PR Checklist

Please review the following before submitting your PR:

• PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.

• PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.

• Test cases are provided for new code paths (see test instructions)

• If PR introduces API changes, an appropriate PR label is added - either api-compatible or api-breaking. For api-breaking, include BREAKING in the PR title.

• Any new dependencies have been scanned for license and vulnerabilities

• CODEOWNERS updated if ownership changes

• Documentation updated as needed

• Update tava architecture diagram if there is a significant design change in PR.

• The reviewers assigned automatically/manually are appropriate for the PR.

• Please check this after reviewing the above items as appropriate for this PR.

GitHub Bot Help

To see a list of available CI bot commands, please comment /bot help.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR adds explicit stderr piping to subprocess invocations in two perf test utility classes. The PerfBenchScriptTestCmds and PerfServeScriptTestCmds command runners now capture stderr separately via subprocess.PIPE when executing prepare-dataset commands, ensuring output is properly decoded and accumulated.

Changes

Stderr piping in perf test command runners

Layer / File(s)	Summary
Stderr piping in perf test command runners tests/integration/defs/perf/utils.py	PerfBenchScriptTestCmds.run_cmd and PerfServeScriptTestCmds.run_cmd both updated to add stderr=subprocess.PIPE to subprocess.check_output() calls for prepare-dataset command execution, ensuring stderr is captured and properly decoded into the accumulated output.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The PR description contains only the template boilerplate with no actual content filled in for Description, Test Coverage, or substantive PR details.	Fill in the Description section explaining the issue and solution, and the Test Coverage section listing relevant tests that safeguard these changes.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main change: piping stderr separately in subprocess calls to improve error reporting in Allure, with a specific issue reference.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

tests/integration/defs/perf/utils.py (1)

281-283: ⚡ Quick win

Coverage follow-up needed for stderr-forwarding paths.

Coverage status: needs follow-up outside PR for tests/integration/defs/perf/utils.py.
Please add regression tests covering both PerfBenchScriptTestCmds.run_cmd and PerfServeScriptTestCmds.run_cmd failure paths to assert CalledProcessError.stderr is populated and reported when a prepare-dataset subcommand fails. Suggested test file(s): tests/integration/defs/perf/test_utils.py (or equivalent existing perf utils test module).

As per coding guidelines, for files under tests/**, review must provide actionable coverage feedback with concrete file names and sufficiency status.

Also applies to: 425-427

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/integration/defs/perf/utils.py` around lines 281 - 283, Add regression
tests that exercise the stderr-forwarding failure paths of
PerfBenchScriptTestCmds.run_cmd and PerfServeScriptTestCmds.run_cmd by invoking
a prepare-dataset subcommand stub that fails and raises
subprocess.CalledProcessError; assert that the exception's stderr is populated
and that the test harness/reporting surface (e.g., captured Allure output or
returned error string) contains that stderr. Create the tests in
tests/integration/defs/perf/test_utils.py (or the existing perf utils test
module), mock or run a controlled failing command, catch CalledProcessError, and
assert CalledProcessError.stderr is not None and its contents are
forwarded/reported by the code paths in run_cmd.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@tests/integration/defs/perf/utils.py`:
- Around line 281-283: Add regression tests that exercise the stderr-forwarding
failure paths of PerfBenchScriptTestCmds.run_cmd and
PerfServeScriptTestCmds.run_cmd by invoking a prepare-dataset subcommand stub
that fails and raises subprocess.CalledProcessError; assert that the exception's
stderr is populated and that the test harness/reporting surface (e.g., captured
Allure output or returned error string) contains that stderr. Create the tests
in tests/integration/defs/perf/test_utils.py (or the existing perf utils test
module), mock or run a controlled failing command, catch CalledProcessError, and
assert CalledProcessError.stderr is not None and its contents are
forwarded/reported by the code paths in run_cmd.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: d6e86260-684d-4f70-8c15-0615a565c676

📥 Commits

Reviewing files that changed from the base of the PR and between c7683f2 and 2e14fcf.

📒 Files selected for processing (1)

• tests/integration/defs/perf/utils.py