Repository navigation
Conversation
The run_with_timeout() helper in e2e-timeout.sh sent only SIGTERM via the timeout command. When a nemoclaw process ignored SIGTERM (e.g. blocked on an uninterruptible SSH child), the inner per-command timeout had no effect and the outer overall E2E timeout (2700s) had to clean up, stalling all subsequent test cases. Add --kill-after=10 so SIGKILL is sent 10 seconds after SIGTERM, ensuring hung processes are forcibly terminated. Signed-off-by: Hai Nguyen <haingu@nvidia.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
✨ Thanks for submitting this PR that proposes a fix to the E2E testing framework by adding a timeout kill after parameter to prevent hung tests. This change improves the reliability of the E2E tests by ensuring that hung processes are forcibly terminated. Related open PRs: Related open issues: |
Summary
The
run_with_timeout()helper intest/e2e/e2e-timeout.shsent only SIGTERM via thetimeoutcommand. When a NemoClaw process ignored SIGTERM (e.g., blocked on an uninterruptible SSH child during TC-SBX-08 Process Recovery), the inner per-command timeout was ineffective and the outer overall E2E timeout (2700s) had to clean up, stalling all subsequent test cases with exit code 124.This adds
--kill-after=10so SIGKILL is sent 10 seconds after SIGTERM, ensuring hung processes are forcibly terminated.Related Issue
Fixes #2983
Changes
_TIMEOUT_KILL_AFTER=10constant intest/e2e/e2e-timeout.sh--kill-after=$_TIMEOUT_KILL_AFTERto$TIMEOUT_CMDinrun_with_timeout()Type of Change
Verification
npx prek run --all-filespassesbash -n test/e2e/e2e-timeout.shpasses (syntax check)AI Disclosure
Signed-off-by: Hai Nguyen haingu@nvidia.com