Skip to content

[https://nvbugs/6427411][test] Re-enable PP regression tests - #17117

Open
mingyangHao wants to merge 1 commit into
NVIDIA:mainfrom
mingyangHao:user/mingyangh/unwaive-nvbug-6427411
Open

[https://nvbugs/6427411][test] Re-enable PP regression tests#17117
mingyangHao wants to merge 1 commit into
NVIDIA:mainfrom
mingyangHao:user/mingyangh/unwaive-nvbug-6427411

Conversation

@mingyangHao

@mingyangHao mingyangHao commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

Remove the remaining PP regression waivers added after #15920. The host-side greedy stop change was reverted by #16163, so these tests should run in CI again.

Dev Engineer Review

  • tests/integration/test_lists/waives.txt removes 30 obsolete pipeline parallelism waivers.
  • The change re-enables the affected regression tests in CI.
  • No public API, configuration, or production-code changes are included.
  • The scope matches the reverted host-side greedy stop change.
  • No test-list format or duplicate-entry issues were identified from the provided change summary.

QA Engineer Review

  • No test-db/ or qa/ files were modified.
  • The change removes CI waiver entries from tests/integration/test_lists/waives.txt.
  • The affected pipeline parallelism regression tests should run in CI again.
  • CBTS coverage data is unavailable.

Verdict: needs follow-up

Description

Test Coverage

PR Checklist

Please review the following before submitting your PR:

  • PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.

  • PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.

  • Test cases are provided for new code paths (see test instructions)

  • If PR introduces API changes, an appropriate PR label is added - either api-compatible or api-breaking. For api-breaking, include BREAKING in the PR title.

  • Any new dependencies have been scanned for license and vulnerabilities

  • CODEOWNERS updated if ownership changes

  • Documentation updated as needed

  • Update tava architecture diagram if there is a significant design change in PR.

  • The reviewers assigned automatically/manually are appropriate for the PR.

  • Please check this after reviewing the above items as appropriate for this PR.

GitHub Bot Help

To see a list of available CI bot commands, please comment /bot help.

Remove the remaining PP regression waivers added after NVIDIA#15920. The host-side greedy stop change was reverted by NVIDIA#16163, so these tests should run in CI again.

Signed-off-by: Mingyang Hao <200044211+mingyangHao@users.noreply.github.com>
@mingyangHao

Copy link
Copy Markdown
Collaborator Author

/bot run

@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Updated tests/integration/test_lists/waives.txt with refreshed model and serving configurations. Removed obsolete Ray, LLM API, statistics, and logits-processor waivers.

Changes

Integration waiver maintenance

Layer / File(s) Summary
Refresh integration test waivers
tests/integration/test_lists/waives.txt
Added current DeepSeekV3Lite, Llama, MiniMaxM2, and Nemotron entries. Removed obsolete disaggregated-serving, Ray, LLM API, statistics, and logits-processor skips.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: bowenfu

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly identifies the NVBugs issue, test-related change, and re-enablement of PP regression tests.
Description check ✅ Passed The description explains the issue and solution clearly, but it does not explicitly list relevant test coverage.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Fix failing CI checks
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #63003 [ run ] triggered by Bot. Commit: 01c40d9 Link to invocation

@mingyangHao
mingyangHao requested a review from shuyixiong July 31, 2026 09:08

@BowenFu BowenFu left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified the premise rather than taking it on trust: #16163 is an exact line-for-line inverse of #15920's commit 4f1c3cd (179 changed lines each, the revert diff is the precise complement), nothing on main has re-introduced the host-side greedy-stop path, and main has exactly 30 6427411 lines — this removes all 30 and adds no new waiver. NVBugs 6427411 is already Bug - Fixed / Verify-to-Close with FixInfo pointing at #16163.

One non-blocking coverage note: 28 of the 30 are scheduled pre-merge (individually in l0_dgx_h100.yml / l0_dgx_b200.yml / l0_gb200_multi_gpus.yml, or via the -m "gpu2"/"gpu4" aggregates), so this PR's own /bot run re-exercises them. The two exceptions are post-merge only — TestNemotronV3Super::test_nvfp4_parallelism[TP4_PP2] (l0_dgx_b200.yml:247) and TestNemotronV3Ultra::test_nvfp4_parallelism[ADP2_PP2] (l0_gb200_multi_gpus.yml:85). The bug's own checklist asks for --extra-stage/--stage-list in exactly this case, so consider re-running with those two stages before merge.

FYI three of the removed entries also have a full:GB300/ or full:B300/ line under a different bug (6388153, 6322073); those are post-merge platform-scoped, so the tests correctly stay skipped there while returning to pre-merge.

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #63003 [ run ] completed with state SUCCESS. Commit: 01c40d9
/LLM/main/L0_MergeRequest_PR pipeline #51111 completed with status: 'SUCCESS'

CI Report

Link to invocation

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