[https://nvbugs/6260897][fix] Relax line-589 assertion to accept either IN_PROGRESS or SUCCESS (FAILURE…

[tensorrt-cicd]

Summary

• Root cause: Test asserts wait(timeout_ms=0) always returns IN_PROGRESS, but on fast intra-host NIXL loopback the progress thread can complete a 40MB WRITE before the Python caller polls, so wait observes NIXL_SUCCESS first and correctly returns TransferState.SUCCESS.
• Fix: Relax line-589 assertion to accept either IN_PROGRESS or SUCCESS (FAILURE would still surface as a real bug), keep all subsequent SUCCESS / data-equality assertions, and remove the stale waives.txt entry so the test runs again.
• Automated fix generated by repair-bot

Test plan

• Verify fix on the same GPU type as the original failure
• Check for regressions in related tests

Links

• Bug: https://nvbugs/6260897

Summary by CodeRabbit

Tests

• Removed test waiver, re-enabling transfer agent timeout handling verification
• Relaxed assertion to accept either IN_PROGRESS or SUCCESS states, accommodating scenarios where transfers complete immediately on fast paths

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR fixes a test that was previously being skipped due to timing sensitivity. The test test_nixl_wait_in_progress_on_zero_timeout now accepts either IN_PROGRESS or SUCCESS when waiting with zero timeout, acknowledging that a transfer may complete immediately on a fast code path. The corresponding waiver entry is removed.

Changes

Transfer Wait Timeout Test Fix

Layer / File(s)	Summary
Zero-timeout wait assertion and waiver removal tests/unittest/bindings/test_transfer_agent_bindings.py, tests/integration/test_lists/waives.txt	Test assertion for status.wait(timeout_ms=0) relaxed to accept either TransferState.IN_PROGRESS or TransferState.SUCCESS, and the SKIP waiver for this test removed from the skip list.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Possibly related PRs

• NVIDIA/TensorRT-LLM#14854: Both PRs modify tests/integration/test_lists/waives.txt by changing waived/skipped integration tests in the same mechanism.

Suggested reviewers

• bo-nv
• suyoggupta
• pcastonguay

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically summarizes the main change: relaxing an assertion at line 589 to accept either IN_PROGRESS or SUCCESS instead of just IN_PROGRESS, addressing the root cause documented in the NVBugs ticket.
Description check	✅ Passed	The PR description includes a clear summary with root cause, fix explanation, test plan verification, and relevant links. It covers the why and what of the changes as required by the template.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

tests/unittest/bindings/test_transfer_agent_bindings.py (1)

805-806: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Apply the same assertion fix to the Mooncake test.

The Mooncake test test_mooncake_wait_in_progress_on_zero_timeout uses a strict assertion that expects IN_PROGRESS, but it can encounter the same race condition as the NIXL test: on fast loopback paths, the progress thread may complete the transfer before wait(timeout_ms=0) is called. Consider relaxing this assertion to match the NIXL test fix.

Proposed fix for consistency

+        # With timeout_ms=0, wait checks status once and returns immediately.
+        # Either IN_PROGRESS (transfer still running) or SUCCESS (transfer
+        # already completed by the progress thread on a fast loopback path)
+        # is a valid non-blocking observation; FAILURE would indicate a bug.
         result = status.wait(timeout_ms=0)
-        assert result == tab.TransferState.IN_PROGRESS
+        assert result in (tab.TransferState.IN_PROGRESS, tab.TransferState.SUCCESS)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unittest/bindings/test_transfer_agent_bindings.py` around lines 805 -
806, The Mooncake test test_mooncake_wait_in_progress_on_zero_timeout currently
asserts strictly that status.wait(timeout_ms=0) ==
tab.TransferState.IN_PROGRESS; relax this to accept either
tab.TransferState.IN_PROGRESS or tab.TransferState.COMPLETED (same approach as
the NIXL test) to handle the race where the progress thread may finish before
wait is called—check the result of status.wait(timeout_ms=0) and assert it is in
{tab.TransferState.IN_PROGRESS, tab.TransferState.COMPLETED}.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@tests/unittest/bindings/test_transfer_agent_bindings.py`:
- Around line 805-806: The Mooncake test
test_mooncake_wait_in_progress_on_zero_timeout currently asserts strictly that
status.wait(timeout_ms=0) == tab.TransferState.IN_PROGRESS; relax this to accept
either tab.TransferState.IN_PROGRESS or tab.TransferState.COMPLETED (same
approach as the NIXL test) to handle the race where the progress thread may
finish before wait is called—check the result of status.wait(timeout_ms=0) and
assert it is in {tab.TransferState.IN_PROGRESS, tab.TransferState.COMPLETED}.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 0820ab15-e524-4535-9048-b9a40cdcede2

📥 Commits

Reviewing files that changed from the base of the PR and between 9827c21 and d5ed80a.

📒 Files selected for processing (2)

• tests/integration/test_lists/waives.txt
• tests/unittest/bindings/test_transfer_agent_bindings.py

💤 Files with no reviewable changes (1)

• tests/integration/test_lists/waives.txt

[Shixiaowei02]

/bot run --disable-fail-fast

[tensorrt-cicd]

PR_Github #59375 [ run ] triggered by Bot. Commit: 8054efc Link to invocation

[tensorrt-cicd]

PR_Github #59375 [ run ] completed with state SUCCESS. Commit: 8054efc
/LLM/main/L0_MergeRequest_PR pipeline #47849 completed with status: 'SUCCESS'

CI Report

Link to invocation

[BowenFu]

LGTM — test-only relaxation that matches a genuine race: a non-blocking wait(timeout_ms=0) can legitimately observe SUCCESS when the progress thread finishes the loopback WRITE before the poll. FAILURE is still asserted and all downstream SUCCESS/data-equality checks remain, so no real error is masked.