[https://nvbugs/6418017][fix] Keep test_medusa collectible but replace its body with a lightweight assertion…#16004
Conversation
PR NVIDIA#15763 removed examples/medusa/ as part of the legacy TensorRT-backend cleanup, but tests/integration/defs/triton_server/build_model.sh still ran 'pushd examples/medusa' for the medusa target, causing the Triton smoke test to fail with 'pushd: examples/medusa: No such file or directory'. Replace the body of test_medusa with a lightweight assertion that examples/medusa/ remains absent under llm_root. This keeps the pytest node id resolvable (the harness runs the test by exact node id) while removing the broken build/test.sh invocation. The dead 'medusa' branches in build_model.sh and test.sh are no longer reachable and can be pruned in a follow-up cleanup. Signed-off-by: trtllm-agent <296075020+trtllm-agent@users.noreply.github.com>
📝 WalkthroughWalkthroughThe ChangesMedusa Test Update
Estimated code review effort: 1 (Trivial) | ~3 minutes Compact metadataRelated issues: None found Related PRs: None found Suggested labels: None found Suggested reviewers: None found Poem A rabbit hopped where medusa slept, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/integration/defs/triton_server/test_triton.py (1)
298-298: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider reusing the existing
tensorrt_llm_medusa_example_rootfixture for path consistency.
conftest.pyalready exposestensorrt_llm_medusa_example_root(derived fromllm_backend_root/../examples/medusa). Recomputing the path here viallm_rootduplicates that logic; using the fixture keeps a single source of truth if the layout changes.🤖 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/triton_server/test_triton.py` at line 298, The Medusa example path is being recomputed manually in the Triton integration test, which duplicates existing path logic. Update the test to use the existing tensorrt_llm_medusa_example_root fixture instead of building medusa_example_dir from llm_root, so the path source stays consistent with conftest.py and the test remains aligned with any future layout changes.
🤖 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/triton_server/test_triton.py`:
- Line 298: The Medusa example path is being recomputed manually in the Triton
integration test, which duplicates existing path logic. Update the test to use
the existing tensorrt_llm_medusa_example_root fixture instead of building
medusa_example_dir from llm_root, so the path source stays consistent with
conftest.py and the test remains aligned with any future layout changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 0ba85a84-f831-41be-9e21-9b7a8faeb2e4
📒 Files selected for processing (1)
tests/integration/defs/triton_server/test_triton.py
Summary
Test plan
Links
Summary by CodeRabbit