-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Refresh network tests server code to Kestrel #52642
Refresh network tests server code to Kestrel #52642
Conversation
I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label. |
Tagging subscribers to this area: @dotnet/ncl Issue DetailsPreparation for hosting this server also in xharness process in order to improve #42852
|
Tagging subscribers to this area: @dotnet/ncl Issue DetailsPreparation for hosting this server also in xharness process in order to improve #42852
|
src/libraries/Common/tests/System/Net/Prerequisites/NetCoreServer/Helpers/RequestInformation.cs
Outdated
Show resolved
Hide resolved
9365dea
to
0ddf37e
Compare
using Microsoft.AspNetCore.Http; | ||
using Microsoft.AspNetCore.Http.Features; | ||
|
||
namespace NetCoreServer |
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.
I'm not sure if this is best name since .NET Core become just .NET in 5.0.
Since this is purely related to testing perhaps the name can reflect that.
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.
Generally looks ok to me. The name space seems odd but it seems like it was inherited from the original code.
|
||
if (context.Request.QueryString.HasValue && context.Request.QueryString.Value.Contains("delay10sec")) | ||
{ | ||
Thread.Sleep(10000); |
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.
await Task.Delay()
? since this is async function? I assume this is copy of original, code right?
} | ||
catch (Exception) | ||
{ | ||
// We might want to log these exceptions. But for now we ignore them. |
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.
That may be hard for Azure service but perhaps simple print would be still beneficial. Unless there is easy way how to emit Kestrel error message.
This brings davidsh's
corefx-net-endpoint
into runtime repository.It's almost 1:1 copy, with minor changes
This new server doesn't cover full scope of the obsolete server. I created #52693 to describe the gaps I know about.
This makes it possible to hosts this server as Kestrel middleware also inside of xharness process in order to improve #42852