Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Use Sync Send methods in Sync HttpClient tests #93139

Merged
merged 4 commits into from
Oct 10, 2023
Merged

Use Sync Send methods in Sync HttpClient tests #93139

merged 4 commits into from
Oct 10, 2023

Conversation

MihaZupan
Copy link
Member

Updated a few existing tests that weren't propagating the TestAsync flag through and would always test the async paths.

Also increased the timeout on GetAsync_ResponseHasNormalLineEndings_Success because AFAICT, the test isn't actually getting stuck, but can just take a long time to execute on busy machines.
In these logs, you can see how bytes were slowly being sent through, but sometimes with ~20-second delays (the test is sending the response one byte at a time).
Contributes to #76678 (I'll keep an eye on CI to see if this decreases the number of times we're retrying this test).

[19.0790] [Http/HandlerMessage] poolId: 30383028, workerId: 32561634, requestId: 50430218, memberName: FillAsync, message: Received 1 bytes.
[19.0960] [Sockets/DumpBuffer] thisOrContextObject: AwaitableSocketAsyncEventArgs#44062838, memberName: LogBuffer, buffer: 47
[19.0964] [Http/HandlerMessage] poolId: 30383028, workerId: 32561634, requestId: 50430218, memberName: FillAsync, message: Received 1 bytes.
[20.0123] [Sockets/DumpBuffer] thisOrContextObject: AwaitableSocketAsyncEventArgs#44062838, memberName: LogBuffer, buffer: 4D
[20.0128] [Http/HandlerMessage] poolId: 30383028, workerId: 32561634, requestId: 50430218, memberName: FillAsync, message: Received 1 bytes.
[24.0540] [Http/HandlerMessage] poolId: 30383028, workerId: 32561634, requestId: 50430218, memberName: FillAsync, message: Received 1 bytes.
[24.0541] [Sockets/DumpBuffer] thisOrContextObject: AwaitableSocketAsyncEventArgs#44062838, memberName: LogBuffer, buffer: 54
[24.0638] [Http/HandlerMessage] poolId: 30383028, workerId: 32561634, requestId: 50430218, memberName: FillAsync, message: Received 1 bytes.
[24.0639] [Sockets/DumpBuffer] thisOrContextObject: AwaitableSocketAsyncEventArgs#44062838, memberName: LogBuffer, buffer: 0D
[24.0671] [Sockets/DumpBuffer] thisOrContextObject: AwaitableSocketAsyncEventArgs#44062838, memberName: LogBuffer, buffer: 0A
[24.0671] [Http/HandlerMessage] poolId: 30383028, workerId: 32561634, requestId: 50430218, memberName: FillAsync, message: Received 1 bytes.
[47.0215] [Sockets/DumpBuffer] thisOrContextObject: AwaitableSocketAsyncEventArgs#44062838, memberName: LogBuffer, buffer: 53
[47.0216] [Http/HandlerMessage] poolId: 30383028, workerId: 32561634, requestId: 50430218, memberName: FillAsync, message: Received 1 bytes.
[47.0320] [Http/HandlerMessage] poolId: 30383028, workerId: 32561634, requestId: 50430218, memberName: FillAsync, message: Received 1 bytes.

@MihaZupan MihaZupan self-assigned this Oct 6, 2023
@ghost
Copy link

ghost commented Oct 6, 2023

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

Updated a few existing tests that weren't propagating the TestAsync flag through and would always test the async paths.

Also increased the timeout on GetAsync_ResponseHasNormalLineEndings_Success because AFAICT, the test isn't actually getting stuck, but can just take a long time to execute on busy machines.
In these logs, you can see how bytes were slowly being sent through, but sometimes with ~20-second delays (the test is sending the response one byte at a time).
Contributes to #76678 (I'll keep an eye on CI to see if this decreases the number of times we're retrying this test).

[19.0790] [Http/HandlerMessage] poolId: 30383028, workerId: 32561634, requestId: 50430218, memberName: FillAsync, message: Received 1 bytes.
[19.0960] [Sockets/DumpBuffer] thisOrContextObject: AwaitableSocketAsyncEventArgs#44062838, memberName: LogBuffer, buffer: 47
[19.0964] [Http/HandlerMessage] poolId: 30383028, workerId: 32561634, requestId: 50430218, memberName: FillAsync, message: Received 1 bytes.
[20.0123] [Sockets/DumpBuffer] thisOrContextObject: AwaitableSocketAsyncEventArgs#44062838, memberName: LogBuffer, buffer: 4D
[20.0128] [Http/HandlerMessage] poolId: 30383028, workerId: 32561634, requestId: 50430218, memberName: FillAsync, message: Received 1 bytes.
[24.0540] [Http/HandlerMessage] poolId: 30383028, workerId: 32561634, requestId: 50430218, memberName: FillAsync, message: Received 1 bytes.
[24.0541] [Sockets/DumpBuffer] thisOrContextObject: AwaitableSocketAsyncEventArgs#44062838, memberName: LogBuffer, buffer: 54
[24.0638] [Http/HandlerMessage] poolId: 30383028, workerId: 32561634, requestId: 50430218, memberName: FillAsync, message: Received 1 bytes.
[24.0639] [Sockets/DumpBuffer] thisOrContextObject: AwaitableSocketAsyncEventArgs#44062838, memberName: LogBuffer, buffer: 0D
[24.0671] [Sockets/DumpBuffer] thisOrContextObject: AwaitableSocketAsyncEventArgs#44062838, memberName: LogBuffer, buffer: 0A
[24.0671] [Http/HandlerMessage] poolId: 30383028, workerId: 32561634, requestId: 50430218, memberName: FillAsync, message: Received 1 bytes.
[47.0215] [Sockets/DumpBuffer] thisOrContextObject: AwaitableSocketAsyncEventArgs#44062838, memberName: LogBuffer, buffer: 53
[47.0216] [Http/HandlerMessage] poolId: 30383028, workerId: 32561634, requestId: 50430218, memberName: FillAsync, message: Received 1 bytes.
[47.0320] [Http/HandlerMessage] poolId: 30383028, workerId: 32561634, requestId: 50430218, memberName: FillAsync, message: Received 1 bytes.
Author: MihaZupan
Assignees: MihaZupan
Labels:

area-System.Net.Http
Milestone: -

@build-analysis build-analysis bot mentioned this pull request Oct 6, 2023
Open
@build-analysis build-analysis bot mentioned this pull request Oct 7, 2023
Closed
3 tasks
@MihaZupan MihaZupan requested a review from a team October 9, 2023 15:00
wfurt
wfurt approved these changes Oct 9, 2023
View reviewed changes
Copy link
Member

@wfurt wfurt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@MihaZupan MihaZupan merged commit 53830e2 into dotnet:main Oct 10, 2023
107 of 109 checks passed
@karelz karelz added this to the 9.0.0 milestone Oct 11, 2023
@MihaZupan MihaZupan mentioned this pull request Oct 16, 2023
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Nov 10, 2023
@antonfirsov antonfirsov added the test-enhancement Improvements of test source code label Dec 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@wfurt wfurt wfurt approved these changes

Assignees

@MihaZupan MihaZupan

Labels
area-System.Net.Http test-enhancement Improvements of test source code
Projects
None yet
Milestone
9.0.0
Development

Successfully merging this pull request may close these issues.
4 participants
@MihaZupan @wfurt @karelz @antonfirsov