Skip to content

[https://nvbugs/6647310][chore] Unwaive TestLagunaXS::test_nvfp4 - #18219

Merged
sunnyqgg merged 1 commit into
NVIDIA:mainfrom
sunnyqgg:unwaive-6647310
Aug 26, 2026
Merged

[https://nvbugs/6647310][chore] Unwaive TestLagunaXS::test_nvfp4#18219
sunnyqgg merged 1 commit into
NVIDIA:mainfrom
sunnyqgg:unwaive-6647310

Conversation

@sunnyqgg

@sunnyqgg sunnyqgg commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Unwaive accuracy/test_llm_api_pytorch.py::TestLagunaXS::test_nvfp4 (waived under https://nvbugs/6647310).
  • The hang was an infra issue: after the preceding test's MPI worker died, SessionPrefetcher.take() handed over a prefetched pool with no live worker, and the next test polled worker_init_status_queue until the 3600 s pytest timeout.
  • The root-cause fix was merged in [TRTLLM-15302][fix] Reject dead prefetched MPI pools #17642 (commit 9d396de): take() now verifies every recorded worker's (pid, start_time) identity before handover, discards dead pools, and falls back to synchronous session creation.

Changes

  • tests/integration/test_lists/waives.txt: remove the TestLagunaXS::test_nvfp4 SKIP entry.

Test plan

  • Pre-merge CI (l0_b200.yml schedules TestLagunaXS::test_nvfp4).

Dev Engineer Review

  • Removed the TestLagunaXS::test_nvfp4 waiver and its nvbugs/6647310 reference from tests/integration/test_lists/waives.txt.
  • The change matches the one-line test-list format and does not affect other tests.
  • The entry is no longer waived after the infrastructure fix in #17642.
  • No code, API, performance, or error-handling changes are included.

QA Engineer Review

  • No test-db or qa files were modified.
  • Removed the accuracy/test_llm_api_pytorch.py::TestLagunaXS::test_nvfp4 entry from waives.txt.
  • The l0_b200.yml schedule is expected to provide CI coverage.
  • Verdict: needs follow-up because CBTS coverage data is unavailable.

@coderabbitai

coderabbitai Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: a15be304-5a66-43d3-b68d-ce52cfe81531

📥 Commits

Reviewing files that changed from the base of the PR and between 0a373ef and 58f6a32.

📒 Files selected for processing (1)
  • tests/integration/test_lists/waives.txt
💤 Files with no reviewable changes (1)
  • tests/integration/test_lists/waives.txt

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.


Walkthrough

Removed the skipped test_nvfp4 waiver and its associated NVBug reference from the integration test waiver list.

Changes

Cohort / File(s) Summary
Test waiver removal
tests/integration/test_lists/waives.txt
Removes the accuracy/test_llm_api_pytorch.py::TestLagunaXS::test_nvfp4 waiver entry.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to 58f6a

This PR only removes the waiver for the specified test; no actionable merge-blocking risk remains beyond normal checks and review.

Possibly related PRs

🚥 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. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
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 reference, labels the change as a chore, and states that the TestLagunaXS::test_nvfp4 waiver is removed.
Description check ✅ Passed The description explains the issue, identifies the root-cause fix, states the exact file change, and provides relevant pre-merge CI coverage. It uses Summary, Changes, and Test plan sections instead o…
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files.

Full details: Description check

Explanation

The description explains the issue, identifies the root-cause fix, states the exact file change, and provides relevant pre-merge CI coverage. It uses Summary, Changes, and Test plan sections instead of the template's Description and Test Coverage headings, and it omits the PR Checklist, but the required technical information is mostly complete.

✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@sunnyqgg

Copy link
Copy Markdown
Collaborator Author

/bot run

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #69281 [ run ] triggered by Bot. Commit: 58f6a32 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #69281 [ run ] completed with state SUCCESS. Commit: 58f6a32
/LLM/main/L0_MergeRequest_PR pipeline #56634 completed with status: 'SUCCESS'

CI Report

Link to invocation

@sunnyqgg
sunnyqgg requested a review from brnguyen2 August 26, 2026 05:08
@sunnyqgg

Copy link
Copy Markdown
Collaborator Author

@brnguyen2 could you help review this PR? thanks

@sunnyqgg
sunnyqgg merged commit 0cb928b into NVIDIA:main Aug 26, 2026
13 of 15 checks passed
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