Skip to content

test: fill signal, sandbox, and TaskWrapper edge-case stubs - #32

Merged
alessiodevoto merged 2 commits into
NVIDIA-NeMo:mainfrom
skundu42:test/edge-cases-signal-sandbox-task-wrapper
Aug 3, 2026
Merged

test: fill signal, sandbox, and TaskWrapper edge-case stubs#32
alessiodevoto merged 2 commits into
NVIDIA-NeMo:mainfrom
skundu42:test/edge-cases-signal-sandbox-task-wrapper

Conversation

@skundu42

Copy link
Copy Markdown
Contributor

Summary

  • Implemented edge-case coverage for concurrent signals during generation (test_signal_edge_cases.py)
  • Implemented sandboxed CodeAct REPL edge cases: state mutation, tool brokering, and execute_python NameError recovery (test_sandbox_edge_cases.py)
  • Added TaskWrapper contract tests for task.abort / task.replan / task.current, skipped until that builtin is implemented (test_task_wrapper_edge_cases.py)

Test plan

  • uv run pytest tests/edge_cases/test_signal_edge_cases.py tests/edge_cases/test_sandbox_edge_cases.py tests/edge_cases/test_task_wrapper_edge_cases.py -v

Signed-off-by: Sandipan Kundu <sandipank.dev@gmail.com>
@alessiodevoto alessiodevoto self-assigned this Jul 31, 2026
Comment thread tests/edge_cases/test_task_wrapper_edge_cases.py Outdated
Comment thread tests/edge_cases/test_signal_edge_cases.py Outdated
@alessiodevoto

Copy link
Copy Markdown
Collaborator

Hi @skundu42! Thanks for opening this PR 🙂

I ran locally and got 7 passed, 3 skipped, I noticed the TaskWrapper tests but we actually don't have that implemented yet, so I’m not comfortable merging the TaskWrapper part as task.abort, task.replan, and task.current are not implemented today. Please remove tests/edge_cases/test_task_wrapper_edge_cases.py.

For the signal tests, I think those make sense but could you please make the lock assertion deterministic, for example replace the asyncio.sleep(0.05) checks with an explicit event/counter in the new GatedFakeLLM, and assert that the nested generation has not entered the LLM call before the outer generation releases the lock ? I left a comment in the code too!

@skundu42

Copy link
Copy Markdown
Contributor Author

@alessiodevoto Thanks for the review. I have made the requested changes. Let me know if this works.

Also, please feel free to assign me other issues directly. I am planning to be an active contributor in this.

@alessiodevoto

Copy link
Copy Markdown
Collaborator

Hi @skundu42 thanks for updating ! LGTM, merging 🙂 !

@alessiodevoto
alessiodevoto merged commit bc1e0fc into NVIDIA-NeMo:main Aug 3, 2026
5 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.

2 participants