8000 chore: add unit test for X11 eviction by spikecurtis · Pull Request #18565 · coder/coder · GitHub
[go: up one dir, main page]

Skip to content

chore: add unit test for X11 eviction #18565

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 27, 2025
Merged

Conversation

spikecurtis
Copy link
Contributor
@spikecurtis spikecurtis commented Jun 25, 2025

relates to #18263

Adds a unit test for X11 listener eviction when all ports in the allowed range are in use.

@spikecurtis spikecurtis requested a review from mafredri June 25, 2025 10:30
@spikecurtis spikecurtis marked this pull request as ready for review June 25, 2025 10:30
@spikecurtis spikecurtis force-pushed the spike/18263-x11-network branch from b60589d to 212e2a5 Compare June 25, 2025 10:37
@spikecurtis spikecurtis force-pushed the spike/18263-eviction-unit-test branch from 7f2e241 to a68240d Compare June 25, 2025 10:37
@spikecurtis spikecurtis changed the title chore: unit test for X11 eviction chore: add unit test for X11 eviction Jun 26, 2025
@spikecurtis spikecurtis force-pushed the spike/18263-x11-network branch from 212e2a5 to a272b09 Compare June 26, 2025 10:41
@spikecurtis spikecurtis force-pushed the spike/18263-eviction-unit-test branch from a68240d to 0d55551 Compare June 26, 2025 10:41
Copy link
Member
@mafredri mafredri left a comment

Choose a reason for hiding this comment

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

Nice test!

8000
return -1, false
}
ctx := slog.With(sshCtx, slog.F("session_id", fmt.Sprintf("%x", serverConn.SessionID())))
Copy link
Member

Choose a reason for hiding this comment

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

This is an under-appreciated feature of slog, nice to see it in action!

// Calculate how many simultaneous X11 sessions we can create given the
// configured port range.
startPort := agentssh.X11StartPort + agentssh.X11DefaultDisplayOffset
maxSessions := agentssh.X11MaxPort - startPort + 1
Copy link
Member

Choose a reason for hiding this comment

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

Suggestion: If we made agentssh.X11MaxPort an option rather than constant, we could decrease the number sessions we have to create. This is definitely an optional suggestion, though.

@spikecurtis spikecurtis force-pushed the spike/18263-x11-network branch from a272b09 to e2d3899 Compare June 27, 2025 09:42
@spikecurtis spikecurtis force-pushed the spike/18263-eviction-unit-test branch 2 times, most recently from 1153c62 to 8a1e923 Compare June 27, 2025 09:55
@spikecurtis spikecurtis force-pushed the spike/18263-x11-network branch from e2d3899 to f3ebe91 Compare June 27, 2025 10:07
@spikecurtis spikecurtis force-pushed the spike/18263-eviction-unit-test branch from 8a1e923 to fb1e1bb Compare June 27, 2025 10:07
@spikecurtis spikecurtis force-pushed the spike/18263-x11-network branch from f3ebe91 to 449c022 Compare June 27, 2025 10:24
@spikecurtis spikecurtis force-pushed the spike/18263-eviction-unit-test branch from fb1e1bb to 1d279c8 Compare June 27, 2025 10:25
@spikecurtis spikecurtis force-pushed the spike/18263-x11-network branch 2 times, most recently from 702c154 to 011e763 Compare June 27, 2025 10:43
@spikecurtis spikecurtis force-pushed the spike/18263-eviction-unit-test branch from 1d279c8 to 3275c03 Compare June 27, 2025 10:43
@spikecurtis spikecurtis changed the base branch from spike/18263-x11-network to graphite-base/18565 June 27, 2025 10:56
@spikecurtis spikecurtis force-pushed the spike/18263-eviction-unit-test branch from 3275c03 to b4283f8 Compare June 27, 2025 10:56
@spikecurtis spikecurtis force-pushed the graphite-base/18565 branch from 011e763 to a5bfb20 Compare June 27, 2025 10:56
@graphite-app graphite-app bot changed the base branch from graphite-base/18565 to main June 27, 2025 10:57
@spikecurtis spikecurtis force-pushed the spike/18263-eviction-unit-test branch from b4283f8 to a6ab9a1 Compare June 27, 2025 10:57
@spikecurtis spikecurtis merged commit 66f22d7 into main Jun 27, 2025
35 checks passed
Copy link
Contributor Author

Merge activity

@spikecurtis spikecurtis deleted the spike/18263-eviction-unit-test branch June 27, 2025 11:13
@github-actions github-actions bot locked and limited conversation to collaborators Jun 27, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0