test(e2e): add cloudflared tunnel URL probe (HTTP + content)#2655
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughSplits the deployment E2E into three subtests: 01a starts the tunnel and polls Changes
Sequence DiagramsequenceDiagram
participant Test as E2E Test
participant CLI as Nemoclaw CLI
participant Status as Nemoclaw Status
participant HTTP as Tunnel URL
participant Dash as OpenClaw Dashboard
Test->>CLI: nemoclaw tunnel start
CLI-->>Test: start result
loop poll status
Test->>Status: nemoclaw status
Status-->>Test: tunnel_url (or empty)
end
alt tunnel_url present
loop curl retries
Test->>HTTP: HTTP GET tunnel_url
HTTP->>Dash: route to origin
Dash-->>HTTP: HTTP 200 + HTML / non-200
HTTP-->>Test: response
end
Test->>Test: verify HTTP 200 and markers
else no tunnel_url
Test-->>Test: skip TC-DEPLOY-01b/01c
end
Test->>CLI: nemoclaw tunnel stop
CLI-->>Test: stop result
loop poll status until absent
Test->>Status: nemoclaw status
Status-->>Test: no tunnel_url
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 9/10 reviews remaining, refill in 6 minutes. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/e2e/test-deployment-services.sh`:
- Around line 357-372: The post-stop check is flaky because it uses a fixed
sleep then a single `nemoclaw status` check; replace that with a retry-poll
loop: after the existing initial sleep, poll `nemoclaw status` (store in
`post_status`) and extract `post_url` up to a configurable timeout/retries
(e.g., loop with short sleeps between attempts) and only call pass
"TC-DEPLOY-01c: Tunnel URL absent after stop" if `post_url` remains empty for
the whole timeout, otherwise call fail with the found `post_url`; keep using the
existing `tunnel_url`, `post_status`, `post_url`, and the
`log`/`skip`/`pass`/`fail` helpers so the rest of the flow is unchanged.
- Around line 308-313: The current single-shot call to `nemoclaw status` stores
output in `status_output` and extracts `tunnel_url` once, which can race because
the tunnel URL appears asynchronously; change this to poll: run `nemoclaw
status` in a loop (e.g., retry every 2–5s) capturing `status_output` each
iteration, attempt to extract `tunnel_url` with the same grep pattern, log each
attempt (use the existing `log` pattern), and break once a URL is found or a
configurable timeout/maximum attempts is reached; ensure variables
`status_output` and `tunnel_url` and the `nemoclaw status` invocation are used
so behavior remains consistent and on timeout the script fails cleanly with a
helpful log message.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 80637055-01df-4557-ad5d-6849ca2d26d7
📒 Files selected for processing (1)
test/e2e/test-deployment-services.sh
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/e2e/test-deployment-services.sh`:
- Around line 305-306: The test currently swallows failures by appending || true
to the nemoclaw tunnel start/stop invocations; change this to capture both
stdout/stderr and the exit code (e.g., assign start_output and start_exit=$?
after running nemoclaw tunnel start) and assert the exit code is 0 before
continuing (log start_output on failure and exit the test); do the same for
nemoclaw tunnel stop (capture stop_output and stop_exit, assert stop_exit is 0),
ensuring you reference the existing start_output/stop_output variables and fail
fast if the commands fail.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: ee025965-991c-46d9-b2a7-f1f17d9c5214
📒 Files selected for processing (1)
test/e2e/test-deployment-services.sh
…start and stop commands before proceeding
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/e2e/test-deployment-services.sh`:
- Around line 302-305: The sandbox existence check uses grep -q "$SANDBOX_NAME"
which treats the name as a regex and can match substrings; change that grep
invocation to a literal fixed-string match using grep -Fq -- "$SANDBOX_NAME"
(preserve the 2>/dev/null redirection and surrounding test) so the conditional
in the test script that uses `if ! nemoclaw list 2>/dev/null | grep -q
"$SANDBOX_NAME"; then` performs a safe, exact literal match.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 8ece41f8-d8c9-46fd-9e64-3347eadc128b
📒 Files selected for processing (1)
test/e2e/test-deployment-services.sh
… into test/e2e-prob-tunnel-url
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/e2e/test-deployment-services.sh`:
- Around line 366-369: Update the log message so it accurately reflects the
command being executed: change the text logged by the call to log that currently
says "Step 4: Running nemoclaw stop..." to reference "nemoclaw tunnel stop" (the
actual command run) so that the log entry for the stop step matches the executed
command (look for the log call near the stop_output=$(nemoclaw tunnel stop ...)
invocation and update its message).
- Around line 381-390: The loop currently swallows failures from the `nemoclaw
status` call (using `|| true`) and treats an empty `post_url` as success,
causing false-positive PASSes; change the logic to capture and check the exit
status of `nemoclaw status` (e.g., store its exit code in a variable like
`status_exit` or a boolean `status_ok`) instead of masking errors, only treat
the test as passed if `status_ok` is true and `post_url` is empty after the
loop, and call `fail` (via the existing `fail` helper) when `nemoclaw status`
could not be read or when `post_url` remains non-empty; update the uses of
`post_status`/`post_url` and the final if/else to reflect this distinction.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 6ec57bbb-d3a9-431b-bea3-2adc8f83f11b
📒 Files selected for processing (1)
test/e2e/test-deployment-services.sh
…ing TC-DEPLOY-01c
cv
approved these changes
Apr 29, 2026
DemianHeyGen
pushed a commit
to DemianHeyGen/NemoClaw
that referenced
this pull request
Apr 30, 2026
…2655) <!-- markdownlint-disable MD041 --> ## Summary Adds TC-DEPLOY-01b to test-deployment-services.sh: an HTTP + content probe of the cloudflared tunnel URL to catch the case where a tunnel publishes a public URL but the data plane never actually serves the OpenClaw dashboard. Also splits the existing TC-DEPLOY-01 into 01a/01b/01c. ## Related Issue Closes NVIDIA#2636 ## Changes - Split `test_deploy_01_start_stop()` into three sub-TCs: - **TC-DEPLOY-01a** — `nemoclaw tunnel start` surfaces a public URL in `status` (existing behavior, relabeled) - **TC-DEPLOY-01b** — *(new)* tunnel URL serves the OpenClaw dashboard - **TC-DEPLOY-01c** — `nemoclaw tunnel stop` removes URL from `status` (existing behavior, relabeled) - New TC-DEPLOY-01b probe: - Strict HTTP 200 only - Body fingerprint: `<title>OpenClaw Control</title>` (verified-stable markers from a live probe) - Replaced 6×5 s polling of `nemoclaw status` with a single 5 s wait + single status read (single source of truth, more readable) - Gated TC-DEPLOY-01c on `tunnel_url` being confirmed in `status` so `post_url == ""` no longer false-PASSes when the URL was never present to begin with - Updated file-header `Covers:` block and function-level comments to reflect the 01a/01b/01c split ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates ## Verification <!-- Check each item you ran and confirmed. Leave unchecked items you skipped. --> - [ ] `npx prek run --all-files` passes - [ ] `npm test` passes - [ ] Tests added or updated for new or changed behavior - [ ] No secrets, API keys, or credentials committed - [ ] Docs updated for user-facing behavior changes - [ ] `make docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- <!-- DCO sign-off required by CI. Run: git config user.name && git config user.email --> Signed-off-by: Hung Le <hple@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Split deployment E2E into three staged subtests: start/URL discovery, HTTP content verification, and stop/URL removal. * Added upfront guard to skip tunnel tests when sandbox is absent. * Improved polling and retries for start/stop flows with stricter success/failure checks and cleanup attempts. * Active HTTP probing until HTTP 200 and expected dashboard markers are found. * Increased sandbox onboarding timeout from 600s to 1800s. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
hunglp6d
added
E2E
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds TC-DEPLOY-01b to test-deployment-services.sh: an HTTP + content probe of the cloudflared tunnel URL to catch the case where a tunnel publishes a public URL but the data plane never actually serves the OpenClaw dashboard. Also splits the existing TC-DEPLOY-01 into 01a/01b/01c.
Related Issue
Closes #2636
Changes
test_deploy_01_start_stop()into three sub-TCs:nemoclaw tunnel startsurfaces a public URL instatus(existing behavior, relabeled)nemoclaw tunnel stopremoves URL fromstatus(existing behavior, relabeled)<title>OpenClaw Control</title>(verified-stable markers from a live probe)nemoclaw statuswith a single 5 s wait + single status read (single source of truth, more readable)tunnel_urlbeing confirmed instatussopost_url == ""no longer false-PASSes when the URL was never present to begin withCovers:block and function-level comments to reflect the 01a/01b/01c splitType of Change
Verification
npx prek run --all-filespassesnpm testpassesmake docsbuilds without warnings (doc changes only)Signed-off-by: Hung Le hple@nvidia.com
Summary by CodeRabbit