Skip to content
This repository was archived by the owner on Dec 19, 2018. It is now read-only.

Commit cda079a

Browse files
mikeharderpranavkm
authored andcommitted
Change SelfHostDeployer to use dynamic ports by default (#1383)
* Should significantly reduce flaky test failures due to AddressInUse exceptions * Addresses #1296
1 parent 958d41f commit cda079a

File tree

2 files changed

+44
-8
lines changed

2 files changed

+44
-8
lines changed

src/Microsoft.AspNetCore.Server.IntegrationTesting/Common/TestUriHelper.cs

Lines changed: 40 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,39 +11,72 @@ public static class TestUriHelper
1111
{
1212
public static Uri BuildTestUri()
1313
{
14-
return new UriBuilder("http", "localhost", GetNextPort()).Uri;
14+
return BuildTestUri(null);
1515
}
1616

1717
public static Uri BuildTestUri(string hint)
18+
{
19+
// If this method is called directly, there is no way to know the server type or whether status messages
20+
// are enabled. It's safest to assume the server is WebListener (which doesn't support binding to dynamic
21+
// port "0") and status messages are not enabled (so the assigned port cannot be scraped from console output).
22+
return BuildTestUri(hint, serverType: ServerType.WebListener, statusMessagesEnabled: false);
23+
}
24+
25+
internal static Uri BuildTestUri(string hint, ServerType serverType, bool statusMessagesEnabled)
1826
{
1927
if (string.IsNullOrEmpty(hint))
2028
{
21-
return BuildTestUri();
29+
if (serverType == ServerType.Kestrel && statusMessagesEnabled)
30+
{
31+
// Most functional tests use this codepath and should directly bind to dynamic port "0" and scrape
32+
// the assigned port from the status message, which should be 100% reliable since the port is bound
33+
// once and never released. Binding to dynamic port "0" on "localhost" (both IPv4 and IPv6) is not
34+
// supported, so the port is only bound on "127.0.0.1" (IPv4). If a test explicitly requires IPv6,
35+
// it should provide a hint URL with "localhost" (IPv4 and IPv6) or "[::1]" (IPv6-only).
36+
return new UriBuilder("http", "127.0.0.1", 0).Uri;
37+
}
38+
else
39+
{
40+
// If the server type is not Kestrel, or status messages are disabled, there is no status message
41+
// from which to scrape the assigned port, so the less reliable GetNextPort() must be used. The
42+
// port is bound on "localhost" (both IPv4 and IPv6), since this is supported when using a specific
43+
// (non-zero) port.
44+
return new UriBuilder("http", "localhost", GetNextPort()).Uri;
45+
}
2246
}
2347
else
2448
{
2549
var uriHint = new Uri(hint);
2650
if (uriHint.Port == 0)
2751
{
52+
// Only a few tests use this codepath, so it's fine to use the less reliable GetNextPort() for simplicity.
53+
// The tests using this codepath will be reviewed to see if they can be changed to directly bind to dynamic
54+
// port "0" on "127.0.0.1" and scrape the assigned port from the status message (the default codepath).
2855
return new UriBuilder(uriHint) { Port = GetNextPort() }.Uri;
2956
}
3057
else
3158
{
59+
// If the hint contains a specific port, return it unchanged.
3260
return uriHint;
3361
}
3462
}
3563
}
3664

3765
// Copied from https://github.com/aspnet/KestrelHttpServer/blob/47f1db20e063c2da75d9d89653fad4eafe24446c/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/AddressRegistrationTests.cs#L508
66+
//
67+
// This method is an attempt to safely get a free port from the OS. Most of the time,
68+
// when binding to dynamic port "0" the OS increments the assigned port, so it's safe
69+
// to re-use the assigned port in another process. However, occasionally the OS will reuse
70+
// a recently assigned port instead of incrementing, which causes flaky tests with AddressInUse
71+
// exceptions. This method should only be used when the application itself cannot use
72+
// dynamic port "0" (e.g. IISExpress). Most functional tests using raw Kestrel
73+
// (with status messages enabled) should directly bind to dynamic port "0" and scrape
74+
// the assigned port from the status message, which should be 100% reliable since the port
75+
// is bound once and never released.
3876
public static int GetNextPort()
3977
{
4078
using (var socket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp))
4179
{
42-
// Let the OS assign the next available port. Unless we cycle through all ports
43-
// on a test run, the OS will always increment the port number when making these calls.
44-
// This prevents races in parallel test runs where a test is already bound to
45-
// a given port, and a new test is able to bind to the same port due to port
46-
// reuse being enabled by default by the OS.
4780
socket.Bind(new IPEndPoint(IPAddress.Loopback, 0));
4881
return ((IPEndPoint)socket.LocalEndPoint).Port;
4982
}

src/Microsoft.AspNetCore.Server.IntegrationTesting/Deployers/SelfHostDeployer.cs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,10 @@ public override async Task<DeploymentResult> DeployAsync()
4141
DotnetPublish();
4242
}
4343

44-
var hintUrl = TestUriHelper.BuildTestUri(DeploymentParameters.ApplicationBaseUriHint);
44+
var hintUrl = TestUriHelper.BuildTestUri(
45+
DeploymentParameters.ApplicationBaseUriHint,
46+
DeploymentParameters.ServerType,
47+
DeploymentParameters.StatusMessagesEnabled);
4548

4649
// Launch the host process.
4750
var (actualUrl, hostExitToken) = await StartSelfHostAsync(hintUrl);

0 commit comments

Comments
 (0)