[None][infra] Waive 1 failed cases for main in pre-merge 39661 - #14520
[None][infra] Waive 1 failed cases for main in pre-merge 39661#14520ZhanruiSunCh wants to merge 1 commit into
Conversation
Bug(s): 6216105 Requested by: @chienchunhung Signed-off-by: ZhanruiSunCh <184402041+ZhanruiSunCh@users.noreply.github.com>
📝 WalkthroughWalkthroughThis PR adds a single test waiver entry to mark the DGX_H100 disaggregated context-prefill and generation-prefill test case for TinyLlama as skipped, with a reference to the associated nvbugs issue. ChangesTest waiver configuration
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/integration/test_lists/waives.txt (1)
273-273: Waiver entry format looks correct; QA list updates are not required for this PR.This is a waiver-only change in
tests/integration/test_lists/waives.txt(no new/modified integration test definitions), so no update is needed intests/integration/test_lists/qa/*or test-db perf list files for this PR.As per coding guidelines, for waiver/list-only updates you should explicitly confirm QA list updates are unnecessary. Based on learnings, using the short nvbugs URL format in this file is preferred and is followed here.
🤖 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/test_lists/waives.txt` at line 273, Summary: waiver-only change is fine but you must explicitly state QA list updates are not required; update the PR/commit message or add a short confirmation in the change describing that this is a waiver-only edit and no changes to tests/integration/test_lists/qa/* or perf lists are needed. Specifically reference the waiver entry "DGX_H100/disaggregated/test_disaggregated.py::test_disaggregated_ctxpp2_genpp2[TinyLlama-1.1B-Chat-v1.0] SKIP (https://nvbugs/6216105)" and note the preferred short nvbugs URL format was used.
🤖 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/test_lists/waives.txt`:
- Line 273: Summary: waiver-only change is fine but you must explicitly state QA
list updates are not required; update the PR/commit message or add a short
confirmation in the change describing that this is a waiver-only edit and no
changes to tests/integration/test_lists/qa/* or perf lists are needed.
Specifically reference the waiver entry
"DGX_H100/disaggregated/test_disaggregated.py::test_disaggregated_ctxpp2_genpp2[TinyLlama-1.1B-Chat-v1.0]
SKIP (https://nvbugs/6216105)" and note the preferred short nvbugs URL format
was used.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 5c348d73-7061-42aa-8c57-e21032782277
📒 Files selected for processing (1)
tests/integration/test_lists/waives.txt
Auto-generated Waive PR
Created by: TensorRT LLM CI Report (requested by @chienchunhung)
Target branch:
mainBug(s): 6216105
Waive entries added
This PR was auto-generated by TensorRT LLM CI Report. Please review the waive entries before merging.
Summary by CodeRabbit