Fix uv streamlit test - #1078
Conversation
Streamlit is an optional dep; without --extra dev, collecting test_review_gui.py fails before Newton marker filtering. Signed-off-by: Qian Lin <qianl@nvidia.com>
Kit UI fails under native uv; the sim-preview subprocess test now disables the visualizer while the GUI runner still defaults to Kit. Signed-off-by: Qian Lin <qianl@nvidia.com>
Greptile SummaryThe PR makes the review-GUI simulation preview test runnable in native uv CI by installing development extras, moving the test into the CI-collected core suite, and adding a headless camera-enabled SimApp launch mode.
Confidence Score: 5/5The PR appears safe to merge, with no concrete blocking or independently actionable non-blocking defects identified. The new CI dependency flag references an existing optional extra, the interactive visualizer remains the default, and the test-only headless selection is consistently propagated through the client, server, and boot layers. Important Files Changed
Sequence DiagramsequenceDiagram
participant Test as Review GUI test
participant Client as SimApp client
participant Server as SimApp server
participant Boot as SimApp boot
participant Isaac as Isaac Sim
Test->>Client: "spawn(enable_visualizer=False)"
Client->>Server: launch --headless
Server->>Boot: "launch_simulation_app(enable_visualizer=False)"
Boot->>Isaac: "headless=True, enable_cameras=True"
Isaac-->>Server: SimulationApp
Test->>Server: preview request over Unix socket
Server-->>Test: preview response
Reviews (1): Last reviewed commit: "Allow review GUI SimApp to boot headless..." | Re-trigger Greptile |
| # Fix dubious ownership issues in cache | ||
| git config --global --add safe.directory '*' | ||
| uv sync ${{ matrix.uv_flags }} | ||
| uv sync ${{ matrix.uv_flags }} --extra dev |
There was a problem hiding this comment.
🟡 Will this break the Docker test steps too?
Moving test_review_gui.py into isaaclab_arena/tests/ means every step that collects that directory now imports streamlit at module top (via streamlit_ui). This --extra dev covers the native-uv job, but the Docker steps install with pip install -e . and no dev extra — and with no continue-on-collection-errors, a missing streamlit fails the whole phase at collection, not just this one test.
Does the isaaclab_arena-latest image already bundle streamlit? If not, would it be cleaner to keep the test in isaaclab_arena_examples/tests/ and add that directory to the CI pytest calls, so a dev-only, examples-testing test doesn't become a hard dependency of the core suite?
🤖 Isaac Lab-Arena Review BotSummaryFixes CI collection of the review-GUI test suite: syncs the package's Design, Boundaries & ScopeRelocating Findings🟡 Warning: .github/workflows/ci.yml:442 — The moved test makes Test CoverageNo new product logic — this is a test-relocation + CI fix. The subprocess test correctly carries VerdictMinor fixes needed — confirm the Docker test steps have streamlit (or run the test from the examples tests dir) before relying on this in CI. |
No description provided.