Skip to content

test: fix resolver tunnel port allocation and bind errors - #336996

Merged
Christof Marti (chrmarti) merged 2 commits into
mainfrom
chrmarti/fix-browser-tools-http-fixture
Sep 21, 2026
Merged

Christof Marti (chrmarti) merged 2 commits into
mainfrom
chrmarti/fix-browser-tools-http-fixture

Conversation

@chrmarti

@chrmarti Christof Marti (chrmarti) commented Sep 21, 2026

Copy link
Copy Markdown
Collaborator

Summary

Narrowed to the test resolver only. The browser-tools HTTP fixture is handled independently by #336988; this PR no longer changes that test.

  • When the requested local port equals the remote port (the test resolver runs both on the same machine), let the OS allocate a free local port instead of assuming remotePort + 1 is available. The adjacent port can be occupied or reserved on Windows, and remote port 65535 would otherwise produce an invalid local port.
  • Reject tunnel creation on a server bind error instead of leaving the promise pending.
  • Preserve explicitly requested distinct local ports and existing privileged-port behavior.

This addresses the resolver failure behind the rollback in #336381. It is not an additional failure established in Azure build 475409, and #336988 does not require this change because it uses the browser remote proxy without explicitly opening a tunnel.

Validation

  • After narrowing: both test extensions compile with zero errors; scoped hygiene (including ESLint) and git diff --check pass.
  • Direct compiled-resolver probes passed: remote port 65535 allocates a valid local port; an occupied explicitly requested local port rejects with EADDRINUSE.
  • The resolver implementation is unchanged from ec5a082570d, validated by Azure build 475477: succeeded, with all Electron and Remote integration jobs passing on Windows x64, Linux x64, and macOS ARM64. That revision still included the now-removed HTTP fixture, which explicitly exercised tunnel creation in remote tests.
  • The scope-only follow-up commit has not been rerun in Azure.

Use a loopback HTTP fixture with remote forwarding and verify rendered content. Let the test resolver allocate a free tunnel port instead of assuming the adjacent port is available, and reject bind errors rather than hanging.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings September 21, 2026 09:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The focused changes correctly address the reported test instability without introducing unresolved issues.

Review effort: Balanced
Findings: None

What changed in this PR

Stabilizes browser HTTP navigation tests by replacing an external dependency with a local fixture and improving test-resolver tunnel allocation and failure handling.

Changes:

  • Uses an ephemeral loopback HTTP fixture with remote forwarding.
  • Allocates OS-selected tunnel ports and rejects bind failures.
  • Ensures browser, tunnel, server, and connection cleanup.
File Description
extensions/​vscode-test-resolver/​src/​extension.ts Improves tunnel port selection and bind-error handling.
extensions/​vscode-api-tests/​src/​singlefolder-tests/​browser.tools.test.ts Adds deterministic HTTP navigation coverage and robust cleanup.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@github-actions

github-actions Bot commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

Base: 94a39f4c Current: 943b4783

No screenshot changes.

Narrow this PR to test resolver tunnel port allocation and bind-error handling.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@chrmarti Christof Marti (chrmarti) changed the title test: stabilize browser HTTP navigation and remote forwarding test: fix resolver tunnel port allocation and bind errors Sep 21, 2026
@chrmarti
Christof Marti (chrmarti) marked this pull request as ready for review September 21, 2026 10:57
@chrmarti
Christof Marti (chrmarti) merged commit b761e4e into main Sep 21, 2026
33 checks passed
@chrmarti
Christof Marti (chrmarti) deleted the chrmarti/fix-browser-tools-http-fixture branch September 21, 2026 14:23
@vs-code-engineering vs-code-engineering Bot added this to the 1.140.0 milestone Sep 21, 2026
@chrmarti Christof Marti (chrmarti) added the ~release-cherry-pick Trigger: cherry-pick this PR to the latest release branch label Sep 21, 2026
@vs-code-engineering vs-code-engineering Bot added release-cherry-pick Automated cherry-pick between release and main branches and removed ~release-cherry-pick Trigger: cherry-pick this PR to the latest release branch labels Sep 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-cherry-pick Automated cherry-pick between release and main branches

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants