Skip to content

refactor(e2e): assert browser tunnel idle survival - #1676

Merged
skevetter merged 1 commit into
mainfrom
refactor/e2e-ide-browser-idle
Oct 23, 2026
Merged

skevetter merged 1 commit into
mainfrom
refactor/e2e-ide-browser-idle

Conversation

@skevetter

@skevetter skevetter commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Replaces the fixed 8-second sleep in e2e/tests/ide/browser_returns.go with an 8-second gomega.Consistently check that the helper PID remains alive, then retains the TCP probe proving the port-forward still accepts connections.
  • The idle interval still has no client connections, so the existing idle-survival assertion is preserved.

Why

A fixed sleep only checks the listener once after the interval. Checking helper liveness throughout the same idle period gives a direct failure signal while preserving the spec's intent and duration.

Validation

  • mise run cli:format, mise run cli:lint:ci, go vet ./e2e/..., go test ./e2e/framework/...: pass.
  • IDE Ginkgo dry run selected the target; the focused spec passed locally.
  • Local full IDE run: 10 passed, 7 failed in the separate VS Code Settings JSONC specs.
  • On this PR head, Test ide on ubuntu-latest passed. All 110 current-head checks are terminal: 94 success, 3 path-conditional skipped, 3 Netlify neutral.

CodeRabbit CLI

coderabbit review --agent --committed --fresh: 0 findings on 08a456b.

Auto-merge

Eligible: yes. This PR changes one e2e spec file only; non-test source lines changed: 0. The e2e spec diff is 8 raw lines (4 under the half-weight rule). Final merge still requires current-head reviewer gates.

Next

None.

Summary by CodeRabbit

  • Tests
    • Updated browser-tunnel testing to verify that the supporting process remains active throughout the full idle period, rather than checking only after the wait has ended.
    • Kept the existing listener-acceptance check unchanged, preserving coverage of the subsequent connection behavior.

@skevetter skevetter added the e2e-refactor e2e refactor label Oct 23, 2026
@netlify

netlify Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for devsydev canceled.

Name Link
🔨 Latest commit 08a456b
🔍 Latest deploy log https://app.netlify.com/projects/devsydev/deploys/6adbd7a50f7a93afe4ab4870

@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 3a54cc06-d26c-4a8c-856e-4122f8a3c7bd
📥 Commits

Reviewing files that changed from the base of the PR and between e3a530e and 08a456b.

📒 Files selected for processing: 1
  • e2e/tests/ide/browser_returns.go

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

📝 Walkthrough

The browser-tunnel idle-timeout test now checks that the helper process remains alive at 500 ms intervals during an 8-second idle period. The listener-acceptance check remains unchanged.

Changes: Browser tunnel idle-timeout test

Layer / File(s) Summary
Idle-period liveness check
e2e/tests/ide/browser_returns.go
The test polls the helper process every 500 ms for 8 seconds instead of waiting without checking its status.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Merge Risk: ⚪ Minimal · up to 08a45

The focused test change is mergeable with no actionable risk identified.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Description check Warning The description clearly explains the change, motivation, and validation results. However, it omits several template sections, including Type of change, AI assistance, Checklist, and Notes for reviewer… Update the description to follow the repository template. Add the Type of change selection, AI assistance disclosure, issue reference such as "Fixes #N", checklist results, and any applicable Notes for reviewers. Keep the existing summary a…
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title is concise, specific, and accurately describes the main change: replacing the browser tunnel idle sleep with an assertion of helper-process survival.
Docstring Coverage Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
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.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files.

Full details: Description check

Explanation

The description clearly explains the change, motivation, and validation results. However, it omits several template sections, including Type of change, AI assistance, Checklist, and Notes for reviewers, and it does not provide the required issue reference or explicit checklist status.

Resolution

Update the description to follow the repository template. Add the Type of change selection, AI assistance disclosure, issue reference such as "Fixes #N", checklist results, and any applicable Notes for reviewers. Keep the existing summary and validation details.

✨ Finishing Touches
✨ Simplify code
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@netlify

netlify Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for images-devsy-sh canceled.

Name Link
🔨 Latest commit 08a456b
🔍 Latest deploy log https://app.netlify.com/projects/images-devsy-sh/deploys/6adbd7a576ab551515d92834

@skevetter
skevetter force-pushed the refactor/e2e-ide-browser-idle branch from 1cb66e7 to 08a456b Compare October 23, 2026 21:54
@skevetter
skevetter marked this pull request as ready for review October 23, 2026 22:49

Copy link
Copy Markdown
Contributor Author

@greptileai review

@mergify

mergify Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

This pull request does not currently match the merge queue conditions, so it cannot be queued from here. The box comes back if it matches again.

@greptile-apps

greptile-apps Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[Low impact] This test-only change appears safe to merge.

Summary

Replaces an eight-second sleep with repeated checks that the browser-tunnel helper stays alive.

  • The idle-survival spec checks the helper throughout the no-connection window.

Reviews (1) · Last reviewed commit: "refactor(e2e): assert browser tunnel idl..." · Reviewed by Greptile

@skevetter
skevetter merged commit 125447d into main Oct 23, 2026
115 checks passed
@skevetter
skevetter deleted the refactor/e2e-ide-browser-idle branch October 23, 2026 22:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant