-
Notifications
You must be signed in to change notification settings - Fork 307
Fix flaky test by using dynamic port allocation instead of static counter #1691
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
base: master
Are you sure you want to change the base?
Conversation
Co-authored-by: tg123 <170430+tg123@users.noreply.github.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Copilot The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
1 similar comment
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Copilot The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Co-authored-by: tg123 <170430+tg123@users.noreply.github.com>
|
/LGTM I thought it is a global webhost issue, seems websocket test only |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes a flaky test issue caused by port conflicts when tests run in parallel. The solution replaces a static port counter with OS-assigned dynamic ports, following the same pattern already established by MockKubeApiServer in the codebase.
Key Changes
- Switched from static port allocation (incrementing from 13255) to OS-assigned ports using
http://127.0.0.1:0 - Eagerly start the web host in the constructor to acquire and retrieve the assigned port via
IServerAddressesFeature - Removed redundant
Host.StartAsync()call from test since host is now started in base class constructor
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/KubernetesClient.Tests/WebSocketTestBase.cs | Replaced static port counter with OS-assigned port allocation, added logic to retrieve actual port from server features, and made property setters explicit |
| tests/KubernetesClient.Tests/PodExecTests.cs | Removed redundant Host.StartAsync() call now that host is started in base class constructor |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| ServerBaseAddress = new Uri(serverAddress); | ||
| WebSocketBaseAddress = new Uri(serverAddress.Replace("http://", "ws://")); |
Copilot
AI
Nov 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The string replacement approach serverAddress.Replace("http://", "ws://") could be fragile. Consider using UriBuilder for a more robust conversion:
var uriBuilder = new UriBuilder(serverAddress)
{
Scheme = "ws"
};
WebSocketBaseAddress = uriBuilder.Uri;This handles edge cases better (e.g., https/wss, preserving all URI components).
| WebSocketBaseAddress = new Uri(serverAddress.Replace("http://", "ws://")); | |
| var uriBuilder = new UriBuilder(serverAddress) | |
| { | |
| Scheme = "ws" | |
| }; | |
| WebSocketBaseAddress = uriBuilder.Uri; |
The
ExecDefaultContainerStdOuttest was failing intermittently with "address already in use" errors when tests ran in parallel. The root cause was a static port counter inWebSocketTestBasethat incremented from 13255, causing port conflicts when multiple test instances started simultaneously.Changes
http://127.0.0.1:0instead of incrementing port counterIServerAddressesFeatureHost.StartAsync()call from test since host is already runningprivate settoServerBaseAddressandWebSocketBaseAddresspropertiesThis follows the same pattern already used in
MockKubeApiServerelsewhere in the codebase.Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
crl.comodoca.com/home/REDACTED/work/csharp/.codeql-scratch/dbs/csharp/working/.dotnet/dotnet /home/REDACTED/work/csharp/.codeql-scratch/dbs/csharp/working/.dotnet/dotnet restore --no-dependencies /home/REDACTED/work/csharp/csharp/src/KubernetesClient.Classic/KubernetesClient.Classic.csproj --packages /home/REDACTED/work/csharp/.codeql-scratch/dbs/csharp/working/packages /p:DisableImplicitNuGetFallbackFolder=true --verbosity normal /p:TargetFrameworkRootPath=/home/REDACTED/work/csharp/.codeql-scratch/dbs/csharp/working/emptyFakeDotnetRoot /p:NetCoreTargetingPackRoot=/home/REDACTED/work/csharp/.codeql-scratch/dbs/csharp/working/emptyFakeDotnetRoot(dns block)idp.issuer.url/usr/share/dotnet/dotnet /usr/share/dotnet/dotnet exec --runtimeconfig /home/REDACTED/work/csharp/csharp/tests/KubernetesClient.Tests/bin/Debug/net8.0/KubernetesClient.Tests.runtimeconfig.json --depsfile /home/REDACTED/work/csharp/csharp/tests/KubernetesClient.Tests/bin/Debug/net8.0/KubernetesClient.Tests.deps.json /home/REDACTED/work/csharp/csharp/tests/KubernetesClient.Tests/bin/Debug/net8.0/testhost.dll --port 45675 --endpoint 127.0.0.1:045675 --role client --parentprocessid 6013 --telemetryoptedin false(dns block)/usr/share/dotnet/dotnet /usr/share/dotnet/dotnet exec --runtimeconfig /home/REDACTED/work/csharp/csharp/tests/KubernetesClient.Tests/bin/Debug/net9.0/KubernetesClient.Tests.runtimeconfig.json --depsfile /home/REDACTED/work/csharp/csharp/tests/KubernetesClient.Tests/bin/Debug/net9.0/KubernetesClient.Tests.deps.json /home/REDACTED/work/csharp/csharp/tests/KubernetesClient.Tests/bin/Debug/net9.0/testhost.dll --port 45053 --endpoint 127.0.0.1:045053 --role client --parentprocessid 6052 --telemetryoptedin false(dns block)/usr/share/dotnet/dotnet /usr/share/dotnet/dotnet exec --runtimeconfig /home/REDACTED/work/csharp/csharp/tests/KubernetesClient.Tests/bin/Debug/net10.0/KubernetesClient.Tests.runtimeconfig.json --depsfile /home/REDACTED/work/csharp/csharp/tests/KubernetesClient.Tests/bin/Debug/net10.0/KubernetesClient.Tests.deps.json /home/REDACTED/work/csharp/csharp/tests/KubernetesClient.Tests/bin/Debug/net10.0/testhost.dll --port 41007 --endpoint 127.0.0.1:041007 --role client --parentprocessid 6053 --telemetryoptedin false(dns block)ocsp.comodoca.com/home/REDACTED/work/csharp/.codeql-scratch/dbs/csharp/working/.dotnet/dotnet /home/REDACTED/work/csharp/.codeql-scratch/dbs/csharp/working/.dotnet/dotnet restore --no-dependencies /home/REDACTED/work/csharp/csharp/src/KubernetesClient.Classic/KubernetesClient.Classic.csproj --packages /home/REDACTED/work/csharp/.codeql-scratch/dbs/csharp/working/packages /p:DisableImplicitNuGetFallbackFolder=true --verbosity normal /p:TargetFrameworkRootPath=/home/REDACTED/work/csharp/.codeql-scratch/dbs/csharp/working/emptyFakeDotnetRoot /p:NetCoreTargetingPackRoot=/home/REDACTED/work/csharp/.codeql-scratch/dbs/csharp/working/emptyFakeDotnetRoot(dns block)If you need me to access, download, or install something from one of these locations, you can either:
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.