fix(site/e2e): close mock external-auth servers in teardown - #26575
#26575Merged
jakehwll merged 5 commits intoJul 1, 2026
maincoder/coder:mainfrom
jakehwll/devex-413-flake-e2e-externalauthspectscoder/coder:jakehwll/devex-413-flake-e2e-externalauthspectsCopy head branch name to clipboard
Merged
fix(site/e2e): close mock external-auth servers in teardown#26575jakehwll merged 5 commits intomaincoder/coder:mainfrom jakehwll/devex-413-flake-e2e-externalauthspectscoder/coder:jakehwll/devex-413-flake-e2e-externalauthspectsCopy head branch name to clipboard
jakehwll merged 5 commits into
maincoder/coder:mainfrom
jakehwll/devex-413-flake-e2e-externalauthspectscoder/coder:jakehwll/devex-413-flake-e2e-externalauthspectsCopy head branch name to clipboard
Conversation
createServer now returns a close handle so callers can release the listener after the test. The web flow closes in afterAll; the device flow uses try/finally. This prevents leaked listeners from triggering EADDRINUSE on the next run, the long-standing root cause of the externalAuth.spec.ts flake (coder/internal#356).
This comment has been minimized.
This comment has been minimized.
Contributor
Author
Layer 1 stress proof, side-by-side, same harness, same port, both code paths pasted verbatim from the respective Run 1:
|
jakehwll
marked this pull request as ready for review
June 24, 2026 03:09
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 863cce9910
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Reorders the teardown sequence so server.close() runs first, then server.closeAllConnections(). The previous order let a connection accepted between the two calls escape the force-close, keeping close() pending until the keep-alive timeout. Addresses review on #26575.
aslilac
approved these changes
Jun 25, 2026
This was referenced Jun 29, 2026
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 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.
Stack:
fix(site/e2e): close mock external-auth servers in teardown← this PRfix(site/e2e): accept 404 from external auth reset hookfix(site/src): refresh provider state after device-flow exchangefix(site/e2e): reset both providers in external auth hookchore(site/e2e): re-enable externalAuth suiteThe externalAuth e2e suite has been skipped since #17235 because
createServerinsite/e2e/helpers.tsstarted an express server but never gave callers a way to close it. On retries or repeated runs, the listener from the previous invocation was still bound to the hardcoded port and the nextbeforeAllfailed withEADDRINUSE, eventually timing out inwaitForPort.createServernow returns a{ app, close }pair. The web flow closes inafterAll; the device flow usestry/finally.closeAllConnections()is called beforeclose()so teardown stays bounded if keep-alive connections linger.The suite remains
test.describe.skiphere; #26648 flips the skip off once the rest of the stack is in.Refs https://linear.app/codercom/issue/DEVEX-413
Refs coder/internal#356
Decision log
Discussed the full options list with @jakehwll before drafting. Picked option A (minimal teardown) because:
Returning
closerather than the rawhttp.Serverencapsulates thecloseAllConnections+closechoreography so callers don't repeat it.closeAllConnectionsis optional-chained because it landed in Node 18.2; coder/coder runs newer, but the chain costs nothing.The device test uses
try/finallyrather than a sharedafterEachto keep per-test state local. The web flow'safterAllmirrors itsbeforeAll.