KEMBAR78
[None][chore] Add failed cases into waives.txt by xinhe-nv · Pull Request #7682 · NVIDIA/TensorRT-LLM · GitHub
Skip to content

Conversation

@xinhe-nv
Copy link
Collaborator

@xinhe-nv xinhe-nv commented Sep 10, 2025

waive failed cases.

Summary by CodeRabbit

  • Tests
    • Marked a specific LLM API PyTorch integration test as skipped in the test suite.
    • Affects automated and local test runs only; no runtime or user-facing behavior changes.
    • No other tests modified. No code, documentation, or UI updates included.

@xinhe-nv xinhe-nv force-pushed the user/qa/post_update_waive_20250910_LLM_FUNCTION_TEST_1313 branch from b0c9d55 to 9b059b3 Compare September 15, 2025 01:19
@xinhe-nv xinhe-nv marked this pull request as ready for review September 15, 2025 01:19
@xinhe-nv
Copy link
Collaborator Author

/bot run --skip-test

@xinhe-nv xinhe-nv enabled auto-merge (squash) September 15, 2025 01:19
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 15, 2025

📝 Walkthrough

Walkthrough

Adds one SKIP entry to tests/integration/test_lists/waives.txt for accuracy/test_llm_api_pytorch.py::TestLlama3_2_3B::test_auto_dtype referencing nvbugs/5471106.

Changes

Cohort / File(s) Summary of Changes
Test waivers
tests/integration/test_lists/waives.txt
Added a single SKIP entry for accuracy/test_llm_api_pytorch.py::TestLlama3_2_3B::test_auto_dtype with reference to nvbugs/5471106

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Suggested reviewers

  • crazydemo
  • LarryXFly

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The PR description is a single line "waive failed cases." and does not follow the repository's required description_template. It omits critical information such as which specific test(s) are waived (the raw summary shows a SKIP added for accuracy/test_llm_api_pytorch.py::TestLlama3_2_3B::test_auto_dtype pointing to nvbugs/5471106), the rationale, Test Coverage entries, and the completed PR Checklist. Because these required sections are missing or incomplete, the description fails the template. Please update the PR description to follow the repository template: add an @coderabbitai summary or full "Description" that lists the exact test(s) being waived (include test path and name), reference the bug (nvbugs/5471106), explain the rationale and expected re-enable timeline, and add a "Test Coverage" section listing relevant tests. Attach CI failure logs or links to justify the waive and indicate whether the waive is temporary, and complete the PR Checklist items. After updating, request reviewer acknowledgement and re-run CI if necessary before merging.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The PR title "[None][chore] Add failed cases into waives.txt" succinctly reflects the primary change: adding waives entries to the repository. It follows the repository's expected title format with a ticket marker and type and directly relates to the changeset (updates to waives.txt). The title is clear and specific enough for a reviewer scanning history.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • 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 1b29c2e and 9b059b3.

📒 Files selected for processing (1)
  • tests/integration/test_lists/waives.txt (1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
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.
📚 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
📚 Learning: 2025-08-26T09:49:04.956Z
Learnt from: pengbowang-nv
PR: NVIDIA/TensorRT-LLM#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
⏰ 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
🔇 Additional comments (1)
tests/integration/test_lists/waives.txt (1)

348-348: Narrow SKIP scope and remove duplicate entries

  • tests/integration/test_lists/waives.txt:348 contains a global SKIP for accuracy/test_llm_api_pytorch.py::TestLlama3_2_3B::test_auto_dtype (https://nvbugs/5471106). The same selector also appears in QA lists at tests/integration/test_lists/qa/llm_function_core_sanity.txt:107 and tests/integration/test_lists/qa/llm_function_core.txt:344. waives.txt has platform‑scoped entries for the same bug on other tests at lines 295 and 298.
  • Action: if this failure is GPU/platform‑specific, replace the global line with a platform scope (example: full:L40S/accuracy/test_llm_api_pytorch.py::TestLlama3_2_3B::test_auto_dtype SKIP (https://nvbugs/5471106)) or add per‑platform duplicates; otherwise keep the global SKIP and remove redundant QA‑list entries.
  • Confirm the test selector is correct — accuracy/test_llm_api_pytorch.py was not found locally; update path/class/method if needed.

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.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

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.

❤️ Share

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

@tensorrt-cicd
Copy link
Collaborator

PR_Github #18532 [ run ] triggered by Bot

@tensorrt-cicd
Copy link
Collaborator

PR_Github #18532 [ run ] completed with state SUCCESS
/LLM/main/L0_MergeRequest_PR pipeline #13909 (Partly Tested) completed with status: 'SUCCESS'

Signed-off-by: xinhe-nv <200704525+xinhe-nv@users.noreply.github.com>
@xinhe-nv xinhe-nv force-pushed the user/qa/post_update_waive_20250910_LLM_FUNCTION_TEST_1313 branch from 9b059b3 to 93832a5 Compare September 15, 2025 03:20
@xinhe-nv
Copy link
Collaborator Author

/bot reuse-pipeline

@tensorrt-cicd
Copy link
Collaborator

PR_Github #18556 [ reuse-pipeline ] triggered by Bot

@tensorrt-cicd
Copy link
Collaborator

PR_Github #18556 [ reuse-pipeline ] completed with state SUCCESS
Reusing PR_Github #18532 (Partly Tested) for commit 93832a5

@xinhe-nv xinhe-nv merged commit b69e3e9 into NVIDIA:main Sep 15, 2025
5 checks passed
@xinhe-nv xinhe-nv deleted the user/qa/post_update_waive_20250910_LLM_FUNCTION_TEST_1313 branch September 15, 2025 05:33
Wong4j pushed a commit to Wong4j/TensorRT-LLM that referenced this pull request Sep 20, 2025
Signed-off-by: xinhe-nv <200704525+xinhe-nv@users.noreply.github.com>
MrGeva pushed a commit to nv-auto-deploy/TensorRT-LLM that referenced this pull request Sep 21, 2025
Signed-off-by: xinhe-nv <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.

3 participants