Skip to content

test(desktop): deflake Command Code browser login port release - #5479

Open
orangeCatDeveloper wants to merge 1 commit into
apache:mainfrom
orangeCatDeveloper:test/commandcode-browser-login-port-flake
Open

orangeCatDeveloper wants to merge 1 commit into
apache:mainfrom
orangeCatDeveloper:test/commandcode-browser-login-port-flake

Conversation

@orangeCatDeveloper

@orangeCatDeveloper orangeCatDeveloper commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Summary

CommandCodeBrowserLoginController tests intermittently flaked on an occupied port is skipped for the next one in the CLI range with 46961 !== 46960. Two defects caused the port collision:

  1. isPortFree called occupyPort, unintentionally retaining an active net.Server in netServers until afterEach.
  2. Tests completing or settling attempts (notably two simultaneous starts) did not await the winner's loopback port release before ending, racing the subsequent test against OS socket teardown.

Refs #5395

Verification

Numbers (before -> after):

  • Leaked probe servers in netServers: 1 -> 0 (isPortFree immediately closes upon verifying instead of holding the port until afterEach).
  • Winner port release verification in two simultaneous starts: unawaited (racing) -> 100% awaited (assert.equal(await isPortFree(Number(callback.port)), true)).
  • Suite stability: flaked in CI run 35314799012 (46961 !== 46960) -> 50/50 passed (0 failures).
# Before (CI run 35314799012):
✖ an occupied port is skipped for the next one in the CLI range (5.455341ms)
  AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
  46961 !== 46960

# After (50 consecutive runs):
✔ CommandCodeBrowserLoginController (324.882208ms)
ℹ tests 20
ℹ suites 2
ℹ pass 20
ℹ fail 0
50/50 passed

Checks run:

  • npm run format:check (biome passed)
  • npm run lint (biome passed)
  • npm run typecheck in apps/desktop (passed)
  • npm run check:tui-copy (ok)
  • node scripts/check-renderer-architecture.mjs --base origin/main in apps/desktop (passed)
  • npm run build:main and node --test "dist/main/__tests__/commandcode-browser-login.test.js" in apps/desktop (20/20 passed)

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: GitHub Copilot diagnosed the port race, updated probe behavior and teardown assertions, and verified test suites. Commit includes the Generated-by: GitHub Copilot trailer.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

isPortFree previously called occupyPort, retaining an active net.Server in
netServers until afterEach rather than probing and releasing immediately.
Additionally, tests completing or settling attempts (notably the simultaneous
starts test) did not await the winner's loopback port release before entering
the next test, leading to EADDRINUSE races where the next test skipped the
closing port and unexpectedly bound a higher port number.

Generated-by: GitHub Copilot
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions github-actions Bot added the effort/S Under 100 readable lines label Sep 18, 2026
@orangeCatDeveloper

Copy link
Copy Markdown
Contributor Author

@Astro-Han

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/S Under 100 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant