Stabilize standalone demo smoke tests - #6982
Conversation
Stop classifying output produced after the smoke supervisor intentionally terminates a healthy demo as runtime-fatal output. Keep teardown logs for diagnostics and cover the phase boundary with a regression test.
Defer Matplotlib initialization until images are saved and disable image saving in the standalone smoke case. This keeps camera readiness focused on sensor initialization and avoids cold font-cache work consuming the startup budget.
Camera-enabled Kit startup consistently needs about nine minutes in CI, so the existing ten-minute timeout leaves too little margin for runner and cache variation. Increase the camera-specific limit to fifteen minutes and remove the plotting bypass that CI showed did not improve startup.
Greptile SummaryStabilizes standalone demo smoke tests by disabling optional camera image saving in the camera smoke case and suppressing fatal-pattern classification during intentional teardown.
Confidence Score: 4/5The PR should not merge until fatal output produced just before intentional teardown is distinguished from output caused by teardown. Disabling monitoring before draining stdout creates a reachable race where a genuine soak-time traceback is collected only after monitoring is off and therefore does not fail the smoke test. Files Needing Attention: source/isaaclab/test/app/standalone_script_cases.py Important Files Changed
Reviews (1): Last reviewed commit: "Reduce camera demo smoke startup time" | Re-trigger Greptile |
| if ready_at is not None and now - ready_at >= soak_time: | ||
| stopped_after_soak = True | ||
| # Preserve shutdown logs without treating errors caused by intentional teardown as runtime failures. | ||
| monitor_fatal_patterns = False |
There was a problem hiding this comment.
There was a problem hiding this comment.
Isaac Lab Review Bot
The camera-specific 900-second startup budget is appropriately scoped, but the teardown change can suppress fatal output emitted during the healthy-soak window if that output remains buffered when monitoring is disabled.
- Design and architecture: Keeping the extended timeout in the camera demo’s per-script override preserves the default timeout for other demos. The supervisor also owns the correct teardown boundary, but that boundary must distinguish output produced before SIGTERM from teardown-generated output.
- API: No public API or data-structure changes are introduced:
run_until_ready,SmokeResult, andassert_smoke_passedretain their existing interfaces. The.skipchangelog fragment is appropriate for this test-only change. - Implementation: At the end of the soak,
monitor_fatal_patternsis disabled before pending stdout is drained. Because each selector iteration reads at most 65536 bytes, output already emitted during the soak can remain queued and then be consumed bycommunicate()without fatal classification. Drain currently available output while monitoring remains enabled, then disable monitoring immediately before intentional termination.
Minor fixes needed. Posted 1 actionable finding inline.
Automated review; human maintainers own approval decisions.
| if ready_at is not None and now - ready_at >= soak_time: | ||
| stopped_after_soak = True | ||
| # Preserve shutdown logs without treating errors caused by intentional teardown as runtime failures. | ||
| monitor_fatal_patterns = False |
There was a problem hiding this comment.
🔵 Suggestion · Implementation — Disable fatal detection after draining pending output
Each loop iteration reads at most 65536 bytes, and monitoring is cleared as soon as the soak elapses. Output the demo already emitted during the healthy soak but still queued in the pipe is later consumed by communicate() with detection off, so a genuine soak-time failure can be classified as teardown noise. Drain the currently available stdout with monitoring still enabled, then clear the flag immediately before sending SIGTERM.
Read all currently buffered child output while fatal monitoring is still active before sending the intentional termination signal. This preserves detection of failures emitted during the healthy soak while continuing to ignore shutdown-generated tracebacks.
Description
Standalone demo smoke tests had two independent flaky failure modes:
SIGTERM. Kit can emit Material Library, USD caching, and Replicator asyncio tracebacks during that forced teardown, after the demo reached readiness and completed its healthy soak.A CI experiment that disabled image saving and deferred the camera demo's Matplotlib import still took 526 seconds. That rules out optional plotting and confirms the long phase belongs to cold camera-enabled Kit/RTX initialization; the experimental demo change has been removed from the final diff.
This change fixes both test boundaries:
Example failures:
No new dependencies are required.
Type of change
Screenshots
Not applicable; this changes headless smoke-test supervision and a case-specific timeout.
Validation
uv run --frozen python -m pytest source/isaaclab/test/app/test_standalone_scripts.py::test_subprocess_supervisor_ignores_fatal_output_after_intentional_teardown -quv run --frozen python -m pytest source/isaaclab/test/app/test_standalone_scripts.py::test_commands_respect_script_launcher_capabilities -quv run --frozen python -m pytest source/isaaclab/test/app/test_standalone_scripts.py::test_subprocess_supervisor_classifies_buffered_fatal_output_before_intentional_teardown -qdevelop: passed.uv run --frozen isaaclab -f: passed after staging and again after commit.Checklist
pre-commitchecks withuv run isaaclab -fsource/<pkg>/changelog.d/for every touched package (do not editCHANGELOG.rstor bumpextension.toml? CI handles that)CONTRIBUTORS.mdor my name already exists there