-
Notifications
You must be signed in to change notification settings - Fork 1.7k
test: [CI] Add failed cases into waives.txt #6423
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: [CI] Add failed cases into waives.txt #6423
Conversation
📝 Walkthrough## Walkthrough
Two new test cases from `examples/test_bert.py::test_llm_bert_general` were added to the skip list in `tests/integration/test_lists/waives.txt`, each linked to a specific bug. Additionally, a pytest timeout decorator of 7200 seconds was added to the test function `test_llm_llama_v3_1_1node_multi_gpus` in `tests/integration/defs/examples/test_llama.py`. No other changes were made.
## Changes
| Cohort / File(s) | Change Summary |
|---------------------------------------------------------------------------------|---------------------------------------------------------------------------------------------------|
| **Test Waivers Update**<br>`tests/integration/test_lists/waives.txt` | Added two new test cases to the skip list for BERT general tests with different model and dataset configurations, referencing bug `https://nvbugs/5421989`. |
| **Test Timeout Addition**<br>`tests/integration/defs/examples/test_llama.py` | Added a `@pytest.mark.timeout(7200)` decorator to the test function `test_llm_llama_v3_1_1node_multi_gpus` to limit its execution time. |
## Estimated code review effort
🎯 1 (Trivial) | ⏱️ ~3 minutes
## Possibly related PRs
- NVIDIA/TensorRT-LLM#6384: Adds new test cases to the same skip list file for waiving failing tests.
- NVIDIA/TensorRT-LLM#6331: Adds different sets of failing tests to the same waiver file.
- NVIDIA/TensorRT-LLM#6394: Adds new test cases to the skip list, managing test waivers in the same file.
## Suggested reviewers
- crazydemo
- yiqingy0
- LarryXFly Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/integration/test_lists/waives.txt
(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: yiqingy0
PR: NVIDIA/TensorRT-LLM#5198
File: jenkins/mergeWaiveList.py:0-0
Timestamp: 2025-07-22T08:33:49.109Z
Learning: In the TensorRT-LLM waive list merging system, removed lines are always located at the end of the merge waive lists, which is why the mergeWaiveList.py script uses reverse traversal - it's an optimization for this specific domain constraint.
tests/integration/test_lists/waives.txt (2)
Learnt from: yiqingy0
PR: #5198
File: jenkins/mergeWaiveList.py:0-0
Timestamp: 2025-07-22T08:33:49.109Z
Learning: In the TensorRT-LLM waive list merging system, removed lines are always located at the end of the merge waive lists, which is why the mergeWaiveList.py script uses reverse traversal - it's an optimization for this specific domain constraint.
Learnt from: moraxu
PR: #6303
File: tests/integration/test_lists/qa/examples_test_list.txt:494-494
Timestamp: 2025-07-28T17:06:08.598Z
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.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Pre-commit Check
f18adf0
to
99676b9
Compare
/bot run |
PR_Github #13313 [ run ] triggered by Bot |
Signed-off-by: xinhe-nv <[email protected]>
Signed-off-by: Xin He (SW-GPU) <[email protected]>
99676b9
to
7f91b80
Compare
PR_Github #13313 [ run ] completed with state |
Signed-off-by: xinhe-nv <[email protected]> Signed-off-by: Xin He (SW-GPU) <[email protected]> Signed-off-by: Lanyu Liao <[email protected]>
waive failed cases.
Summary by CodeRabbit