fix(site): wait until port is available in e2e - #15537
#15537Merged
dannykopping merged 5 commits intoNov 18, 2024
maincoder/coder:mainfrom
fix-e2e-212-wait-for-servercoder/coder:fix-e2e-212-wait-for-serverCopy head branch name to clipboard
Merged
fix(site): wait until port is available in e2e#15537dannykopping merged 5 commits intomaincoder/coder:mainfrom fix-e2e-212-wait-for-servercoder/coder:fix-e2e-212-wait-for-serverCopy head branch name to clipboard
dannykopping merged 5 commits into
maincoder/coder:mainfrom
fix-e2e-212-wait-for-servercoder/coder:fix-e2e-212-wait-for-serverCopy head branch name to clipboard
Conversation
mtojek
marked this pull request as ready for review
November 15, 2024 16:47
Signed-off-by: Danny Kopping <danny@coder.com>
dannykopping
approved these changes
Nov 18, 2024
Contributor
There was a problem hiding this comment.
Thanks for the assist on this one @mtojek!
Tested this locally and it works, let's try it out in CI.
dannykopping
enabled auto-merge (squash)
November 18, 2024 10:20
jakehwll
added a commit
that referenced
this pull request
Jul 1, 2026
> 🤖 This PR was written by Coder Agents on behalf of Jake Howell. Stack: 1. #26575 `fix(site/e2e): close mock external-auth servers in teardown` ← this PR 2. #26793 `fix(site/e2e): accept 404 from external auth reset hook` 3. #26795 `fix(site/src): refresh provider state after device-flow exchange` 4. #26798 `fix(site/e2e): reset both providers in external auth hook` 5. #26648 `chore(site/e2e): re-enable externalAuth suite` The externalAuth e2e suite has been skipped since #17235 because `createServer` in `site/e2e/helpers.ts` started 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 next `beforeAll` failed with `EADDRINUSE`, eventually timing out in `waitForPort`. `createServer` now returns a `{ app, close }` pair. The web flow closes in `afterAll`; the device flow uses `try/finally`. `closeAllConnections()` is called before `close()` so teardown stays bounded if keep-alive connections linger. The suite remains `test.describe.skip` here; #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 <details> <summary>Decision log</summary> Discussed the full options list with @jakehwll before drafting. Picked option A (minimal teardown) because: - Two prior PRs (#15537, #16528) attacked symptoms (port probing, longer timeout) without addressing the leaked listener. - Kayla's diagnosis on coder/internal#356 pointed at exactly this case: nothing else in CI is grabbing the port, the listener from the previous run is still bound. - A is mechanical and orthogonal: it adds a real teardown without changing port allocation, fixture wiring, or what gets mocked. If the flake persists after A, we know to escalate to a worker-scoped fixture or dynamically allocated ports. Returning `close` rather than the raw `http.Server` encapsulates the `closeAllConnections` + `close` choreography so callers don't repeat it. `closeAllConnections` is optional-chained because it landed in Node 18.2; coder/coder runs newer, but the chain costs nothing. The device test uses `try/finally` rather than a shared `afterEach` to keep per-test state local. The web flow's `afterAll` mirrors its `beforeAll`. </details>
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
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.
Related: coder/internal#212
This PR modifies the logic responsible for creating a server in E2E tests to check if the port is free. Alternatively, we could refactor the framework to dynamically create server instances, but this solution might be a cheaper quick win.
Note:
I'll leave it as is now, it might be worth asking somebody with a frontend skillset to double-check this contribution.