Skip to content

Conversation

@xinhe-nv
Copy link
Collaborator

@xinhe-nv xinhe-nv commented Dec 12, 2025

waive failed cases.

Summary by CodeRabbit

  • Tests
    • Updated test skip entries for disaggregated serving tests.

✏️ Tip: You can customize this high-level summary in your review settings.

@xinhe-nv xinhe-nv force-pushed the user/qa/post_update_waive_20251212_LLM_FUNCTION_TEST_1742 branch 2 times, most recently from 78d9e94 to 0d8782e Compare December 12, 2025 08:27
@xinhe-nv xinhe-nv marked this pull request as ready for review December 12, 2025 08:28
@xinhe-nv
Copy link
Collaborator Author

/bot run --skip-test

@xinhe-nv xinhe-nv enabled auto-merge (squash) December 12, 2025 08:28
@xinhe-nv xinhe-nv changed the title [None][chore] Add failed cases into waives.txt [TRTLLM-8638][fix] Add failed cases into waives.txt Dec 12, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 12, 2025

📝 Walkthrough

Walkthrough

Two new test skip entries were added to the waives list for disaggregated Serving integration tests targeting Llama 3.1 8B Instruct model variants. No executable code or functional changes were introduced.

Changes

Cohort / File(s) Change Summary
Test Skip List
tests/integration/test_lists/waives.txt
Added two SKIP entries for disaggregated Serving accuracy tests: test_ctx_pp_gen_tp_asymmetric[MMLU-gen_tp=2-ctx_pp=2] and test_tp_pp_symmetric[MMLU-tp2pp2] for TestLlama3_1_8BInstruct

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

  • Single file modification with straightforward additions to a test skip list
  • No code logic, functional changes, or dependencies to evaluate

Possibly related PRs

Suggested reviewers

  • crazydemo
  • jieli-matrix
  • StanleySun639
  • LarryXFly

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: adding failed test cases to the waives.txt file.
Description check ✅ Passed The description is minimal but provides the essential information: it explains the action (waiving failed cases) and includes a reference link to the test report showing the failures.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fd3d3a5 and 0d8782e.

📒 Files selected for processing (1)
  • tests/integration/test_lists/waives.txt (1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: tongyuantongyu
Repo: NVIDIA/TensorRT-LLM PR: 7781
File: tests/integration/test_lists/waives.txt:313-313
Timestamp: 2025-09-17T02:48:52.732Z
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.
Learnt from: fredricz-20070104
Repo: NVIDIA/TensorRT-LLM PR: 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.
Learnt from: EmmaQiaoCh
Repo: NVIDIA/TensorRT-LLM PR: 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.
Learnt from: pengbowang-nv
Repo: NVIDIA/TensorRT-LLM PR: 7192
File: tests/integration/test_lists/test-db/l0_dgx_b200.yml:56-72
Timestamp: 2025-08-26T09:49:04.956Z
Learning: In TensorRT-LLM test configuration files, the test scheduling system handles wildcard matching with special rules that prevent duplicate test execution even when the same tests appear in multiple yaml files with overlapping GPU wildcards (e.g., "*b200*" and "*gb200*").
📚 Learning: 2025-09-17T02:48:52.732Z
Learnt from: tongyuantongyu
Repo: NVIDIA/TensorRT-LLM PR: 7781
File: tests/integration/test_lists/waives.txt:313-313
Timestamp: 2025-09-17T02:48:52.732Z
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
Repo: NVIDIA/TensorRT-LLM PR: 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-07-22T08:33:49.109Z
Learnt from: yiqingy0
Repo: NVIDIA/TensorRT-LLM 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.

Applied to files:

  • tests/integration/test_lists/waives.txt
📚 Learning: 2025-08-29T14:07:45.863Z
Learnt from: EmmaQiaoCh
Repo: NVIDIA/TensorRT-LLM PR: 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-08-26T09:49:04.956Z
Learnt from: pengbowang-nv
Repo: NVIDIA/TensorRT-LLM PR: 7192
File: tests/integration/test_lists/test-db/l0_dgx_b200.yml:56-72
Timestamp: 2025-08-26T09:49:04.956Z
Learning: In TensorRT-LLM test configuration files, the test scheduling system handles wildcard matching with special rules that prevent duplicate test execution even when the same tests appear in multiple yaml files with overlapping GPU wildcards (e.g., "*b200*" and "*gb200*").

Applied to files:

  • tests/integration/test_lists/waives.txt
📚 Learning: 2025-07-28T17:06:08.621Z
Learnt from: moraxu
Repo: NVIDIA/TensorRT-LLM PR: 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
⏰ 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). (3)
  • GitHub Check: Check PR Checklist Resolution
  • GitHub Check: Check PR Title Format
  • GitHub Check: Pre-commit Check
🔇 Additional comments (1)
tests/integration/test_lists/waives.txt (1)

433-434: LGTM!

Both entries are correctly formatted and consistent with existing waive list entries. The format matches the GPU-specific prefix pattern (e.g., full:sm89/, full:L40S/) and the test path structure. The bug reference aligns with a related entry at line 301 for the same test suite on a different GPU model, which is appropriate for capturing known failures across hardware variants.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@xinhe-nv xinhe-nv force-pushed the user/qa/post_update_waive_20251212_LLM_FUNCTION_TEST_1742 branch from 0d8782e to 9d3a97e Compare December 12, 2025 09:05
@xinhe-nv
Copy link
Collaborator Author

/bot run --skip-test

@tensorrt-cicd
Copy link
Collaborator

PR_Github #28019 [ run ] triggered by Bot. Commit: 9d3a97e

@xinhe-nv xinhe-nv force-pushed the user/qa/post_update_waive_20251212_LLM_FUNCTION_TEST_1742 branch from 9d3a97e to cc43405 Compare December 12, 2025 09:18
@tensorrt-cicd
Copy link
Collaborator

PR_Github #28024 [ run ] triggered by Bot. Commit: cc43405

@tensorrt-cicd
Copy link
Collaborator

PR_Github #28019 [ run ] completed with state ABORTED. Commit: 9d3a97e

@xinhe-nv xinhe-nv force-pushed the user/qa/post_update_waive_20251212_LLM_FUNCTION_TEST_1742 branch from cc43405 to 4d7a2bd Compare December 12, 2025 11:11
@tensorrt-cicd
Copy link
Collaborator

PR_Github #28024 [ run ] completed with state SUCCESS. Commit: cc43405
/LLM/main/L0_MergeRequest_PR pipeline #21403 (Partly Tested) completed with status: 'SUCCESS'

@xinhe-nv xinhe-nv force-pushed the user/qa/post_update_waive_20251212_LLM_FUNCTION_TEST_1742 branch 2 times, most recently from d02b79f to 405c73c Compare December 13, 2025 13:16
@xinhe-nv
Copy link
Collaborator Author

/bot reuse-pipeline

@tensorrt-cicd
Copy link
Collaborator

PR_Github #28306 [ reuse-pipeline ] triggered by Bot. Commit: 3e86f8a

@xinhe-nv
Copy link
Collaborator Author

/bot reuse-pipeline

@tensorrt-cicd
Copy link
Collaborator

PR_Github #28315 [ reuse-pipeline ] triggered by Bot. Commit: c8e2b7d

@tensorrt-cicd
Copy link
Collaborator

PR_Github #28306 [ reuse-pipeline ] completed with state ABORTED. Commit: 3e86f8a
Can't reuse PR_Github #28024 (Partly Tested) with status: SUCCESS

@tensorrt-cicd
Copy link
Collaborator

PR_Github #28315 [ reuse-pipeline ] completed with state SUCCESS. Commit: c8e2b7d
Reusing PR_Github #28024 (Partly Tested) for commit c8e2b7d

@xinhe-nv
Copy link
Collaborator Author

/bot reuse-pipeline

@tensorrt-cicd
Copy link
Collaborator

PR_Github #28330 [ reuse-pipeline ] triggered by Bot. Commit: bc0260e

@xinhe-nv
Copy link
Collaborator Author

/bot reuse-pipeline

@tensorrt-cicd
Copy link
Collaborator

PR_Github #28335 [ reuse-pipeline ] triggered by Bot. Commit: 246c010

@tensorrt-cicd
Copy link
Collaborator

PR_Github #28330 [ reuse-pipeline ] completed with state ABORTED. Commit: bc0260e
Can't reuse PR_Github #28024 (Partly Tested) with status: SUCCESS

Signed-off-by: Xin He (SW-GPU) <200704525+xinhe-nv@users.noreply.github.com>
@xinhe-nv xinhe-nv force-pushed the user/qa/post_update_waive_20251212_LLM_FUNCTION_TEST_1742 branch from 1155d2a to acafd4c Compare December 15, 2025 06:49
@xinhe-nv
Copy link
Collaborator Author

/bot reuse-pipeline

@tensorrt-cicd
Copy link
Collaborator

PR_Github #28343 [ reuse-pipeline ] triggered by Bot. Commit: acafd4c

@tensorrt-cicd
Copy link
Collaborator

PR_Github #28335 [ reuse-pipeline ] completed with state ABORTED. Commit: 246c010
Can't reuse PR_Github #28024 (Partly Tested) with status: SUCCESS

@tensorrt-cicd
Copy link
Collaborator

PR_Github #28343 [ reuse-pipeline ] completed with state SUCCESS. Commit: acafd4c
Reusing PR_Github #28024 (Partly Tested) for commit acafd4c

@xinhe-nv xinhe-nv merged commit 3c98b25 into NVIDIA:main Dec 15, 2025
5 checks passed
@xinhe-nv xinhe-nv deleted the user/qa/post_update_waive_20251212_LLM_FUNCTION_TEST_1742 branch December 15, 2025 07:20
sherry-1001 pushed a commit to sherry-1001/TensorRT-LLM that referenced this pull request Dec 16, 2025
Signed-off-by: Xin He (SW-GPU) <200704525+xinhe-nv@users.noreply.github.com>
codego7250 pushed a commit to codego7250/TensorRT-LLM that referenced this pull request Dec 19, 2025
Signed-off-by: Xin He (SW-GPU) <200704525+xinhe-nv@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants