Skip to content

test(cuda.core): wait for the event-record target to turn pending - #3170

Open
saiyambharara wants to merge 1 commit into
NVIDIA:mainfrom
saiyambharara:fix-3106-racy-event-graph-tests
Open

saiyambharara wants to merge 1 commit into
NVIDIA:mainfrom
saiyambharara:fix-3106-racy-event-graph-tests

Conversation

@saiyambharara

Copy link
Copy Markdown

Description

closes #3106

is_done reports True for an event that was never recorded. Checking it once right after the host callback starts races the driver marking the event pending. Poll with the existing _wait_until instead. A wrong record target still fails after 5s.

Locally, 200 repeats pass. The original race only showed on the MCDM runner.

Checklist

  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

Signed-off-by: Saiyam Bharara <saiyambharara@gmail.com>
@copy-pr-bot

copy-pr-bot Bot commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions github-actions Bot added the cuda.core Everything related to the cuda.core module label Oct 20, 2026
@coderabbitai

coderabbitai Bot commented Oct 20, 2026

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: NVIDIA/cuda-python/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Enterprise
  • Run ID: 890062ee-4b1a-4e7f-bbc4-e5d02f872005
📥 Commits

Reviewing files that changed from the base of the PR and between b288fc8 and 5f9bd01.

📒 Files selected for processing: 1
  • cuda_core/tests/graph/test_graph_node_update.py

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

📝 Summary

Summary by CodeRabbit

  • Tests
    • Updated event-record testing to wait until the expected event reports an incomplete state, improving reliability for asynchronous behavior.

The event-record test now waits for the expected event to report is_done as False instead of checking the value immediately.

Changes: Event-record test

Layer / File(s) Summary
Wait for event state
cuda_core/tests/graph/test_graph_node_update.py
The event-record execution check waits for the expected event to report is_done as False.

Assessment against linked issues

Objective Addressed Explanation
Remove the race in the event-record test by waiting for the original event to become pending [#3106] ✅

Suggested reviewers: andy-jost

Priority: ⬇️ Low

Change: Bug fix · Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to 5f9bd

The test now waits reliably for the event state while retaining timeout protection and callback cleanup.

✨ Finishing Touches: 💡 1
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cuda.core Everything related to the cuda.core module

Projects

None yet

1 participant