[https://nvbugs/6162128][tests] Skip nano v3 E2E tests entirely on G/B300#14185
Conversation
|
/bot run |
📝 WalkthroughWalkthroughThis PR adjusts pytest skip markers in the multimodal LLM API test file to control test execution across hardware generations. The ChangesTest Skip Marker Configuration
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/integration/defs/accuracy/test_llm_api_pytorch_multimodal.py (1)
547-552: QA list updates are unnecessary for this marker-only gating change.This keeps an already-listed test selected while enforcing hardware exclusions via pytest markers, which is the expected pattern for these integration QA lists.
As per coding guidelines: “hardware gating is expected to happen inside the test code/pytest markers,” and
llm_function_core.txtalready includesTestNanoV3Omni::test_auto_dtypeparametrizations.🤖 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/accuracy/test_llm_api_pytorch_multimodal.py` around lines 547 - 552, The QA list change is unnecessary — revert any edits to QA selection for TestNanoV3Omni and keep the test listed only via existing qa files; instead enforce hardware exclusion using the pytest marker already applied (skip_post_blackwell_ultra) on the TestNanoV3Omni class so the test remains selected in llm_function_core.txt (for TestNanoV3Omni::test_auto_dtype) while gating by marker in the test file where TestNanoV3Omni is defined.
🤖 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/accuracy/test_llm_api_pytorch_multimodal.py`:
- Around line 547-552: The QA list change is unnecessary — revert any edits to
QA selection for TestNanoV3Omni and keep the test listed only via existing qa
files; instead enforce hardware exclusion using the pytest marker already
applied (skip_post_blackwell_ultra) on the TestNanoV3Omni class so the test
remains selected in llm_function_core.txt (for TestNanoV3Omni::test_auto_dtype)
while gating by marker in the test file where TestNanoV3Omni is defined.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 907013ef-aa75-4681-8e1e-51ebeb7d45cc
📒 Files selected for processing (1)
tests/integration/defs/accuracy/test_llm_api_pytorch_multimodal.py
|
PR_Github #48600 [ run ] triggered by Bot. Commit: |
|
PR_Github #48600 [ run ] completed with state
|
|
/bot run |
|
PR_Github #48647 [ run ] triggered by Bot. Commit: |
|
PR_Github #48647 [ run ] completed with state
|
…B300 Signed-off-by: William Zhang <133824995+2ez4bz@users.noreply.github.com>
2ez4bz
force-pushed
the
dev-unwaive-nano-v3-test
branch
from
660bd0a to
be84637
Compare
|
/bot run |
|
PR_Github #48939 [ run ] triggered by Bot. Commit: |
|
PR_Github #48939 [ run ] completed with state
|
|
/bot run |
|
PR_Github #48951 [ run ] triggered by Bot. Commit: |
|
PR_Github #48951 [ run ] completed with state
|
|
/bot run |
|
PR_Github #48985 [ run ] triggered by Bot. Commit: |
|
PR_Github #48985 [ run ] completed with state |
…B300 (NVIDIA#14185) The `llm-models` repo is out of sync on GB300 nodes. Skip problematic test `TestNanoV3Omni` entirely on these nodes since they do not meaningfully improve coverage anyway. Signed-off-by: William Zhang <133824995+2ez4bz@users.noreply.github.com>
5 participants
Summary by CodeRabbit
Description
The
llm-modelsrepo is out of sync on GB300 nodes.Skip problematic test
TestNanoV3Omnientirely on thesenodes since they do not meaningfully improve coverage
anyway.
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-compatibleorapi-breaking. Forapi-breaking, includeBREAKINGin 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.