KEMBAR78
[None][chore] Remove closed bugs by xinhe-nv · Pull Request #7591 · NVIDIA/TensorRT-LLM · GitHub
Skip to content

[None][chore] Remove closed bugs #7591

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Sep 9, 2025
Merged

[None][chore] Remove closed bugs #7591

merged 1 commit into from
Sep 9, 2025

Conversation

@xinhe-nv
Copy link
Collaborator

@xinhe-nv xinhe-nv commented Sep 7, 2025 •
edited by coderabbitai bot
Loading

Remove closed bugs from waives.txt

Summary by CodeRabbit

  • Tests

    • Updated integration test waivers: unskipped several previously waived scenarios and added new waivers for others.
    • Adjusted coverage across select model/back-end combinations and hardware profiles.
    • No impact on runtime features or APIs.

  • Chores

    • Maintained test matrix to reflect current stability and coverage targets.

@xinhe-nv xinhe-nv force-pushed the user/qa/unwaive_20250907 branch from cc54fc6 to 089e6ab Compare September 9, 2025 03:38
@xinhe-nv
unwaive tests
5ac5e46 
Signed-off-by: Xin He (SW-GPU) <200704525+xinhe-nv@users.noreply.github.com>
@xinhe-nv xinhe-nv force-pushed the user/qa/unwaive_20250907 branch from 089e6ab to 5ac5e46 Compare September 9, 2025 03:42
@xinhe-nv xinhe-nv marked this pull request as ready for review September 9, 2025 03:43
@xinhe-nv xinhe-nv enabled auto-merge (squash) September 9, 2025 03:43
@xinhe-nv
Copy link
Collaborator Author

xinhe-nv commented Sep 9, 2025

/bot run

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 9, 2025

📝 Walkthrough

Walkthrough

Updated tests/integration/test_lists/waives.txt to remove several existing waiver entries and add new ones, changing which integration tests are skipped. No source or test logic changes beyond modifying the waiver list.

Changes

Cohort / File(s) Summary of Changes
Test waiver list updates
tests/integration/test_lists/waives.txt		 Removed waiver entries for multiple tests (e.g., llmapi speculative decoding MTP, TinyLlama MPI single-GPU, several accuracy backends and FP8 cases). Added new waiver entries for selected H100 Llama4ScoutInstruct FP8 tests and L40S disaggregated serving scenarios, updating which tests are skipped.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • crazydemo
  • LarryXFly
  • yiqingy0

Pre-merge checks (1 passed, 1 warning, 1 inconclusive)

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The current description only states “Remove closed bugs from waives.txt” and does not follow the repository’s required PR template, as it lacks sections such as “## Description,” “## Test Coverage,” and the “## PR Checklist.” Expand the pull request description using the provided template by adding detailed “## Description,” “## Test Coverage,” and “## PR Checklist” sections with the relevant context and validation steps.
Title Check ❓ Inconclusive The title “[None][chore] Remove closed bugs” follows the templated format but is too vague and does not clearly convey that the pull request updates the waives.txt file to both remove certain test skip entries and add new ones. Please revise the title to specifically highlight the main change, for example “[None][chore] Update waives.txt to remove obsolete test skips and add new waivers.”
✅ Passed checks (1 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

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.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
		  - name: "Undocumented Breaking Changes"
			  mode: "warning"
			  instructions: |
				  Flag potential breaking changes that are not documented:
				  1. Identify changes to public APIs/exports, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints (including removed/renamed items and changes to types, required params, return values, defaults, or behavior).
				  2. Ignore purely internal/private changes (e.g., code not exported from package entry points or marked internal).
				  3. Verify documentation exists: a "Breaking Change" section in the PR description and updates to CHANGELOG.md.

Please share your feedback with us on this Discord post.

📜 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 5c616da and 5ac5e46.

📒 Files selected for processing (1)
  • tests/integration/test_lists/waives.txt (0 hunks)
💤 Files with no reviewable changes (1)
  • 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
✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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 #18145 [ run ] triggered by Bot

@tensorrt-cicd
Copy link
Collaborator

PR_Github #18145 [ run ] completed with state SUCCESS
/LLM/main/L0_MergeRequest_PR pipeline #13597 completed with status: 'SUCCESS'

@xinhe-nv xinhe-nv merged commit 8a52015 into NVIDIA:main Sep 9, 2025
7 checks passed
@xinhe-nv xinhe-nv deleted the user/qa/unwaive_20250907 branch September 9, 2025 08:09
gergely-magyar pushed a commit to gergely-magyar/TensorRT-LLM that referenced this pull request Sep 9, 2025
@xinhe-nv @gergely-magyar
[None][chore] Remove closed bugs (NVIDIA#7591)
0a95837 
Signed-off-by: Xin He (SW-GPU) <200704525+xinhe-nv@users.noreply.github.com>
Signed-off-by: Gergely Magyar <gergely.magyar@visma.com>
Wong4j pushed a commit to Wong4j/TensorRT-LLM that referenced this pull request Sep 20, 2025
@xinhe-nv @Wong4j
[None][chore] Remove closed bugs (NVIDIA#7591)
bf804a3 
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

Reviewers

@LarryXFly LarryXFly LarryXFly approved these changes

@crazydemo crazydemo Awaiting requested review from crazydemo

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@xinhe-nv @tensorrt-cicd @LarryXFly