[TRTLLM-7250][fix] Add failed cases into waives.txt #7801
Conversation
|
/bot run --skip-test |
📝 WalkthroughWalkthroughAdded four new SKIP entries to tests/integration/test_lists/waives.txt to skip specified multimodal E2E and benchmark tests, each annotated with corresponding nvbugs references. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/integration/test_lists/waives.txt (2)
355-355: Benchmark serving waiver added—confirm blast radius.If failures are isolated to certain backends or GPUs, consider scoping with the appropriate “full:/” prefix to avoid globally skipping this benchmark.
352-355: Optional: add light metadata to aid cleanup.Consider appending a short note like “added 2025-09-17; owner: LLM QA” inside the parentheses after the bug link to make later un-waive sweeps easier. Many entries lack this, so treat as optional consistency improvement.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/integration/test_lists/waives.txt(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: tongyuantongyu
PR: NVIDIA/TensorRT-LLM#7781
File: tests/integration/test_lists/waives.txt:313-313
Timestamp: 2025-09-17T02:48:52.691Z
Learning: In TensorRT-LLM, `tests/integration/test_lists/waives.txt` is specifically for waiving/skipping tests, while other test list files like those in `test-db/` and `qa/` directories are for different test execution contexts (pre-merge, post-merge, QA tests). The same test appearing in both waives.txt and execution list files is intentional - the test is part of test suites but will be skipped due to the waiver.
📚 Learning: 2025-09-17T02:48:52.691Z
Learnt from: tongyuantongyu
PR: NVIDIA/TensorRT-LLM#7781
File: tests/integration/test_lists/waives.txt:313-313
Timestamp: 2025-09-17T02:48:52.691Z
Learning: In TensorRT-LLM, `tests/integration/test_lists/waives.txt` is specifically for waiving/skipping tests, while other test list files like those in `test-db/` and `qa/` directories are for different test execution contexts (pre-merge, post-merge, QA tests). The same test appearing in both waives.txt and execution list files is intentional - the test is part of test suites but will be skipped due to the waiver.
Applied to files:
tests/integration/test_lists/waives.txt
📚 Learning: 2025-09-09T09:40:45.658Z
Learnt from: fredricz-20070104
PR: NVIDIA/TensorRT-LLM#7645
File: tests/integration/test_lists/qa/llm_function_core.txt:648-648
Timestamp: 2025-09-09T09:40:45.658Z
Learning: In TensorRT-LLM test lists, it's common and intentional for the same test to appear in multiple test list files when they serve different purposes (e.g., llm_function_core.txt for comprehensive core functionality testing and llm_function_core_sanity.txt for quick sanity checks). This duplication allows tests to be run in different testing contexts.
Applied to files:
tests/integration/test_lists/waives.txt
📚 Learning: 2025-08-29T14:07:45.863Z
Learnt from: EmmaQiaoCh
PR: NVIDIA/TensorRT-LLM#7370
File: tests/unittest/trt/model_api/test_model_quantization.py:24-27
Timestamp: 2025-08-29T14:07:45.863Z
Learning: In TensorRT-LLM's CI infrastructure, pytest skip markers (pytest.mark.skip) are properly honored even when test files have __main__ blocks that call test functions directly. The testing system correctly skips tests without requiring modifications to the __main__ block execution pattern.
Applied to files:
tests/integration/test_lists/waives.txt
📚 Learning: 2025-07-28T17:06:08.621Z
Learnt from: moraxu
PR: NVIDIA/TensorRT-LLM#6303
File: tests/integration/test_lists/qa/examples_test_list.txt:494-494
Timestamp: 2025-07-28T17:06:08.621Z
Learning: In TensorRT-LLM testing, it's common to have both CLI flow tests (test_cli_flow.py) and PyTorch API tests (test_llm_api_pytorch.py) for the same model. These serve different purposes: CLI flow tests validate the traditional command-line workflow, while PyTorch API tests validate the newer LLM API backend. Both are legitimate and should coexist.
Applied to files:
tests/integration/test_lists/waives.txt
🔇 Additional comments (1)
tests/integration/test_lists/waives.txt (1)
352-354: Waivers present; test file not found — confirm test location & scope
- The three waiver entries in tests/integration/test_lists/waives.txt (lines 352–354) each appear once.
- rg returned "test_e2e.py: No such file or directory", so I could not verify that def test_ptp_quickstart_multimodal exists; confirm the test's filepath/name or update the waiver target. If the failures are platform-specific, restrict the waiver to the affected HW bucket to avoid over-skipping.
|
PR_Github #18976 [ run ] triggered by Bot |
|
PR_Github #18976 [ run ] completed with state |
xinhe-nv
force-pushed
the
user/qa/post_update_waive_20250917_LLM_FUNCTION_TEST_1327
branch
from
6fe0a81 to
4187a46
Compare
|
/bot reuse-pipeline |
xinhe-nv
changed the title
|
/bot run --skip-test |
|
PR_Github #19071 [ reuse-pipeline ] triggered by Bot |
|
/bot reuse-pipeline |
|
PR_Github #19075 [ reuse-pipeline ] triggered by Bot |
|
PR_Github #19071 [ reuse-pipeline ] completed with state |
|
PR_Github #19073 [ run ] triggered by Bot |
|
PR_Github #19075 [ reuse-pipeline ] completed with state |
|
/bot reuse-pipeline --number 18976 |
|
PR_Github #19085 [ reuse-pipeline ] triggered by Bot |
|
PR_Github #19073 [ run ] completed with state |
|
PR_Github #19085 [ reuse-pipeline ] completed with state |
|
/bot help |
GitHub Bot Help
Provide a user friendly way for developers to interact with a Jenkins server. Run See details below for each supported subcommand.
Launch build/test pipelines. All previously running jobs will be killed.
kill
Kill all running builds associated with pull request. skip
Skip testing for latest commit on pull request. reuse-pipeline
Reuse a previous pipeline to validate current commit. This action will also kill all currently running builds associated with the pull request. IMPORTANT NOTE: This is dangerous since lack of user care and validation can cause top of tree to break. |
|
/bot run --reuse-test pipeline-id 18976 |
|
PR_Github #19089 Bot args parsing error: usage: /bot run [--reuse-test [optionalpipeline-id]] [--disable-fail-fast] |
|
/bot run --reuse-test 18976 |
|
PR_Github #19093 [ run ] triggered by Bot |
xinhe-nv
force-pushed
the
user/qa/post_update_waive_20250917_LLM_FUNCTION_TEST_1327
branch
2 times, most recently
from
bae24a3 to
51039e6
Compare
|
PR_Github #19093 [ run ] completed with state |
xinhe-nv
force-pushed
the
user/qa/post_update_waive_20250917_LLM_FUNCTION_TEST_1327
branch
from
51039e6 to
e84743f
Compare
|
/bot reuse-pipeline |
Signed-off-by: xinhe-nv <[email protected]>
xinhe-nv
force-pushed
the
user/qa/post_update_waive_20250917_LLM_FUNCTION_TEST_1327
branch
from
e84743f to
3b13c01
Compare
|
PR_Github #19121 [ reuse-pipeline ] triggered by Bot |
|
PR_Github #19121 [ reuse-pipeline ] completed with state |
xinhe-nv
deleted the
user/qa/post_update_waive_20250917_LLM_FUNCTION_TEST_1327
branch
Signed-off-by: xinhe-nv <[email protected]>
Signed-off-by: xinhe-nv <[email protected]>
waive failed cases until fix is merged to main branch
Summary by CodeRabbit