Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Fix proxy connect tunnel test #30007

Merged
merged 3 commits into from
May 31, 2018
Merged

Fix proxy connect tunnel test #30007

merged 3 commits into from
May 31, 2018

Conversation

davidsh
Copy link
Contributor

@davidsh davidsh commented May 31, 2018

The test was hanging on most handlers except for SocketsHttpHandler. It was hanging because
the await'ed task of connecting to the proxy never happened. Except for SocketsHttpHandler,
the other handlers optimize loopback so that if the destination server is on loopback, then it
ignores any proxy setting. The test was using both a loopback destination server as well as a
loopback proxy server.

This test is one of the few tests that do an end-to-end request/response thru a proxy. So, it's
important that we keep the test and have it work on all handlers. Modified the test to use a real
external server destination. The test still uses a loopback proxy server.

I also moved some of the Outerloop tests to Innerloop since those tests were using loopback.

Fixes #27746

The test was hanging on most handlers except for SocketsHttpHandler. It was hanging because
the await'ed task of connecting to the proxy never happened. Except for SocketsHttpHandler,
the other handlers optimize loopback so that if the destination server is on loopback, then it
ignores any proxy setting. The test was using both a loopback destination server as well as a
loopback proxy server.

This test is one of the few tests that do an end-to-end request/response thru a proxy. So, it's
important that we keep the test and have it work on all handlers. Modified the test to use a real
external server destination. The test still uses a loopback proxy server.

I also moved some of the Outerloop tests to Innerloop since those tests were using loopback.

Fixes #27746
@davidsh davidsh added test bug Problem in test source code (most likely) area-System.Net.Http labels May 31, 2018
@davidsh davidsh added this to the 3.0 milestone May 31, 2018
@davidsh davidsh self-assigned this May 31, 2018
@davidsh davidsh requested review from stephentoub and a team May 31, 2018 03:26
@@ -2230,7 +2221,7 @@ public void GetAsync_ServerNeedsAuthAndNoCredential_StatusCodeUnauthorized()
server => server.AcceptConnectionSendResponseAndCloseAsync());
}

[OuterLoop] // TODO: Issue #11345
[OuterLoop("Multiple connections")]
Copy link
Member

Choose a reason for hiding this comment

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

Does this one need to be outerloop?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was concerned at first that it might take a longer time since it used multiple connections. But we can probably get rid of the Outerloop.

await serverTask;
}, options);
await proxy;
await TaskTimeoutExtensions.WhenAllOrAnyFailed(new Task[] { proxyTask }, TestHelper.PassingTestTimeoutMilliseconds);
Copy link
Member

Choose a reason for hiding this comment

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

This isn't waiting on multiple tasks anymore, so the WhenAllOrAnyFailed isn't needed. It can instead be:

await proxyTask.TimeoutAfter(TestHelper.PassingTestTimeoutMilliseconds);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thx. I'll change that.

@davidsh
Copy link
Contributor Author

davidsh commented May 31, 2018

@dotnet-bot test Outerloop Windows x64 Debug Build
@dotnet-bot test Outerloop Linux x64 Debug Build
@dotnet-bot test Outerloop NETFX x86 Debug Build
@dotnet-bot test Outerloop UWP CoreCLR x64 Debug Build
@dotnet-bot test Outerloop OSX x64 Debug Build

@davidsh
Copy link
Contributor Author

davidsh commented May 31, 2018

@davidsh davidsh merged commit 2edbc59 into dotnet:master May 31, 2018
@davidsh davidsh deleted the proxy_test branch May 31, 2018 20:17
caesar-chen pushed a commit to caesar-chen/corefx that referenced this pull request Jun 1, 2018
The test was hanging on most handlers except for SocketsHttpHandler. It was hanging because
the await'ed task of connecting to the proxy never happened. Except for SocketsHttpHandler,
the other handlers optimize loopback so that if the destination server is on loopback, then it
ignores any proxy setting. The test was using both a loopback destination server as well as a
loopback proxy server.

This test is one of the few tests that do an end-to-end request/response thru a proxy. So, it's
important that we keep the test and have it work on all handlers. Modified the test to use a real
external server destination. The test still uses a loopback proxy server.

I also moved some of the Outerloop tests to Innerloop since those tests were using loopback.

Fixes #27746
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
The test was hanging on most handlers except for SocketsHttpHandler. It was hanging because
the await'ed task of connecting to the proxy never happened. Except for SocketsHttpHandler,
the other handlers optimize loopback so that if the destination server is on loopback, then it
ignores any proxy setting. The test was using both a loopback destination server as well as a
loopback proxy server.

This test is one of the few tests that do an end-to-end request/response thru a proxy. So, it's
important that we keep the test and have it work on all handlers. Modified the test to use a real
external server destination. The test still uses a loopback proxy server.

I also moved some of the Outerloop tests to Innerloop since those tests were using loopback.

Fixes dotnet/corefx#27746


Commit migrated from dotnet/corefx@2edbc59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Net.Http test bug Problem in test source code (most likely)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants