-
Notifications
You must be signed in to change notification settings - Fork 10k
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
[Middleware] Move to GenericHost #23626
Conversation
{ | ||
var client = server.CreateClient(); | ||
client.DefaultRequestHeaders.AcceptLanguage.ParseAdd("jp,ar-SA,en-US"); | ||
var count = client.DefaultRequestHeaders.AcceptLanguage.Count; | ||
var response = await client.GetAsync(string.Empty); | ||
Assert.Equal(3, count); | ||
} | ||
|
||
await host.StopAsync(); |
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.
Remind me why you had to call stop? Why isn't Dispose good enough?
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 don't remember exactly, I think this was the issue where memory blocks were not being disposed which breaks in debug.
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's only an issue in Kestrel and SignalR (?), not TestServer.
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.
Ah true, only if you use Kestrel. Want me to remove the StopAsync
's then?
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.
Remove a few just to confirm it works without 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.
Seems to work fine. I'll remove them.
Part of #20964
Each Middleware has its own commit
Edit:
Disposing the
IHost
for all tests is in the last commit, sorry did it as an after the fact change so it's not separated by area.