aspnet / StaticFiles

[Archived] Middleware for handling requests for file system resources including files and directories. Project moved to https://github.com/aspnet/AspNetCore
Apache License 2.0
114 stars 72 forks source link

Use dynamic ports in StaticFileMiddlewareTests #236

Closed mikeharder closed 6 years ago

mikeharder commented 6 years ago
mikeharder commented 6 years ago

Alternatively, we could add a new public method BuildTestUri(ServerType serverType) to https://github.com/aspnet/Hosting/blob/dev/src/Microsoft.AspNetCore.Server.IntegrationTesting/Common/TestUriHelper.cs, then use this method instead of the private GetTestUrl(). It will be a little more work since it's a new cross-repo API, but it avoids some duplicate code.

mikeharder commented 6 years ago

Even though the methods are public, it turns out no other repos currently use either BuildTestUri() or GetNextPort() from IntegrationTesting (Kestrel has its own copy of GetNextPort():

https://github.com/search?l=C%23&q=org%3Aaspnet+buildtesturi&type=Code https://github.com/search?l=C%23&q=org%3Aaspnet+getnextport&type=Code

So I'm not sure if we want to start the precedent of directly calling either BuildTestUri() or GetNextPort(). We may instead want to make them private in IntegrationTesting.

mikeharder commented 6 years ago

@anurse: Any thoughts regarding whether it's better to share common code like BuildTestUri() and GetNextPort() in integration testing, or just copying the code to repos which need it? It's fairly short and simple code, so I'm not sure if the cross-repo dependency is worth it.

Tratcher commented 6 years ago

Having a dependency for BuildTestUri seems fine, the logic keeps getting more complicated and we only want to maintain it in one place.

analogrelay commented 6 years ago

For repos that already have the dependency sure, I'm not sure it's worth adding new cross-repo dependencies.

mikeharder commented 6 years ago

Ok, I will move the logic to IntegrationTesting and update this PR tomorrow.

mikeharder commented 6 years ago

Updated to use new helper methods added in https://github.com/aspnet/Hosting/pull/1387.