Use generic test handling for remote server failures. #25910

Priya91 · 2017-12-14T00:09:54Z

fixes #25878
fixes #25892

When any test with remote server dependency fails, it will output something like this:

System.Net.Http.Functional.Tests.HttpClientHandler_SslProtocols_Test.GetAsync_UnsupportedSSLVersion_Throws(name: \"SSLv2\", url: \"https://www.ssllabs.com:10200/\") [FAIL]
System.Net.Test.Common.RemoteServerException : Likely external issue with remote server : https://www.ssllabs.com:10200/
---- System.Net.Http.HttpRequestException : An error occurred while sending the request.
-------- System.Net.Http.WinHttpException : A connection with the server could not be established
Stack Trace:
   E:\corefx\src\Common\tests\System\Net\RemoteServerQuery.cs(65,0): at System.Net.Test.Common.RemoteServerQuery.<ThrowsAsync>d__1`1.MoveNext()
   --- End of stack trace from previous location where exception was thrown ---
   E:\corefx\src\System.Net.Http\tests\FunctionalTests\HttpClientHandlerTest.SslProtocols.cs(220,0): at System.Net.Http.Functional.Tests.HttpClientHandler_SslProtocols_Test.<GetAsync_UnsupportedSSLVersion_Throws>d__9.MoveNext()
   --- End of stack trace from previous location where exception was thrown ---
   --- End of stack trace from previous location where exception was thrown ---
   --- End of stack trace from previous location where exception was thrown ---
   ----- Inner Stack Trace -----
   E:\corefx\src\System.Net.Http\src\System\Net\Http\HttpClient.cs(464,0): at System.Net.Http.HttpClient.<FinishSendAsyncBuffered>d__58.MoveNext()
   --- End of stack trace from previous location where exception was thrown ---
   E:\corefx\src\Common\tests\System\Net\RemoteServerQuery.cs(56,0): at System.Net.Test.Common.RemoteServerQuery.<ThrowsAsync>d__1`1.MoveNext()
   ----- Inner Stack Trace -----
   E:\corefx\src\System.Runtime.Extensions\src\System\Environment.cs(176,0): at System.Environment.get_StackTrace()
   E:\corefx\src\Common\src\System\Runtime\ExceptionServices\ExceptionStackTrace.cs(23,0): at System.Runtime.ExceptionServices.ExceptionStackTrace.AddCurrentStack(Exception exception)
   E:\corefx\src\Common\src\System\Net\Http\WinHttpException.cs(56,0): at System.Net.Http.WinHttpException.CreateExceptionUsingError(Int32 error)
   E:\corefx\src\System.Net.Http.WinHttpHandler\src\System\Net\Http\WinHttpRequestCallback.cs(324,0): at System.Net.Http.WinHttpRequestCallback.OnRequestError(WinHttpRequestState state, WINHTTP_ASYNC_RESULT asyncResult)
   E:\corefx\src\System.Net.Http.WinHttpHandler\src\System\Net\Http\WinHttpRequestCallback.cs(104,0): at System.Net.Http.WinHttpRequestCallback.RequestCallback(IntPtr handle, WinHttpRequestState state, UInt32 internetStatus, IntPtr statusInformation, UInt32 statusInformationLength)
   E:\corefx\src\System.Net.Http.WinHttpHandler\src\System\Net\Http\WinHttpRequestCallback.cs(47,0): at System.Net.Http.WinHttpRequestCallback.WinHttpCallback(IntPtr handle, IntPtr context, UInt32 internetStatus, IntPtr statusInformation, UInt32 statusInformationLength)
   --- End of stack trace from AddCurrentStack ---
   E:\corefx\src\Common\src\System\Threading\Tasks\RendezvousAwaitable.cs(62,0): at System.Threading.Tasks.RendezvousAwaitable`1.GetResult()
   E:\corefx\src\System.Net.Http.WinHttpHandler\src\System\Net\Http\WinHttpHandler.cs(856,0): at System.Net.Http.WinHttpHandler.<StartRequest>d__105.MoveNext()

Priya91 · 2017-12-14T01:01:59Z

@dotnet-bot test outerloop linux x64 debug build please
@dotnet-bot test outerloop windows x64 debug build please
@dotnet-bot test outerloop osx x64 debug build please

Priya91 · 2017-12-14T18:43:01Z

The test failures are not related to this PR.

wfurt · 2017-12-14T18:59:20Z

It looks ok to me. With "Likely external issue with remote server", would it make sense to claim it only on specific underlying exceptions e.g. timeout and reset?

Priya91 · 2017-12-14T19:13:47Z

would it make sense to claim it only on specific underlying exceptions e.g. timeout and reset?

Yes, that's what the lambda you pass in to the API is for, only when the lambda returns true, the remoteserverexception is thrown.

karelz · 2017-12-14T22:33:05Z

src/Common/tests/System/Net/RemoteServerQuery.cs

+        public RemoteServerException()
+            : this(null, null) { }
+
+        public RemoteServerException(string server)


Can we remove this overload? - to force everyone to use the original exception as well?

karelz · 2017-12-14T22:33:59Z

src/Common/tests/System/Net/RemoteServerQuery.cs

+        public RemoteServerException(string server, Exception inner)
+            : base(GetMessage(server), inner) { }
+
+        public static string GetMessage(string server)


Looks like helper method, do we need it public?

This is not exposed in any api or anything, the whole class in internal. But yeah it can be private.

karelz · 2017-12-14T22:44:08Z

src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.SslProtocols.cs

            using (HttpClient client = CreateHttpClient())
            {
-                await Assert.ThrowsAsync<HttpRequestException>(() => client.GetAsync(url));
+                await RemoteServerQuery.ThrowsAsync<HttpRequestException>(() => client.GetAsync(url), remoteServerExceptionWrapper, url);


What if we just wrap the exception around the call?

await Assert.ThrowsAsync<HttpRequestException>(() => RemoteServerQuery.Run(() => client.GetAsync(url), remoteServerExceptionWrapper, url));

karelz · 2017-12-14T22:45:21Z

src/Common/tests/System/Net/RemoteServerQuery.cs

+
+        internal static async Task ThrowsAsync<T>(Func<Task> testCode, Func<Exception, bool> remoteExceptionWrapper, string serverName)
+            where T : Exception
+        {


We're now playing the role of XUnit a bit by comparing the exception type and not its subtypes (it may be xunit default behavior). Do we really need it? What if we stick to just wrapping the exception? (see next comment)

Priya91 · 2017-12-15T00:02:05Z

the desktop run failed with

15:27:30 D:\j\workspace\windows-TGrou---2a8f9c29\Tools\tests.targets(484,5): warning MSB3073: The command "D:\j\workspace\windows-TGrou---2a8f9c29\bin/tests/System.IO.Compression.Tests/netfx-Windows_NT-Release-x86//RunTests.cmd D:\j\workspace\windows-TGrou---2a8f9c29\bin/testhost/netfx-Windows_NT-Release-x86/" exited with code 1. [D:\j\workspace\windows-TGrou---2a8f9c29\src\System.IO.Compression\tests\System.IO.Compression.Tests.csproj]
15:27:30 D:\j\workspace\windows-TGrou---2a8f9c29\Tools\tests.targets(492,5): error : One or more tests failed while running tests from 'System.IO.Compression.Tests' please check D:\j\workspace\windows-TGrou---2a8f9c29\bin/tests/System.IO.Compression.Tests/netfx-Windows_NT-Release-x86/testResults.xml for details! [D:\j\workspace\windows-TGrou---2a8f9c29\src\System.IO.Compression\tests\System.IO.Compression.Tests.csproj]

Use generic test handling for remote server failures. Commit migrated from dotnet/corefx@2fe2cd5

Use generic test handling for remote server failures.

d37b86e

Priya91 force-pushed the remoteserver branch from 3237719 to d37b86e Compare December 14, 2017 01:01

davidsh added area-System.Net test enhancement Improvements of test source code labels Dec 14, 2017

karelz assigned Priya91 Dec 14, 2017

karelz suggested changes Dec 14, 2017

View reviewed changes

Respond to pr feedback.

c9a8b39

karelz approved these changes Dec 14, 2017

View reviewed changes

karelz removed the test enhancement Improvements of test source code label Dec 14, 2017

Priya91 merged commit 2fe2cd5 into dotnet:master Dec 15, 2017

Priya91 deleted the remoteserver branch December 15, 2017 01:03

karelz added this to the 2.1.0 milestone Dec 28, 2017

picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022

Merge pull request dotnet/corefx#25910 from Priya91/remoteserver

9909f77

Use generic test handling for remote server failures. Commit migrated from dotnet/corefx@2fe2cd5

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Use generic test handling for remote server failures. #25910

Use generic test handling for remote server failures. #25910

Uh oh!

Priya91 commented Dec 14, 2017

Uh oh!

Priya91 commented Dec 14, 2017

Uh oh!

Priya91 commented Dec 14, 2017

Uh oh!

wfurt commented Dec 14, 2017

Uh oh!

Priya91 commented Dec 14, 2017

Uh oh!

karelz Dec 14, 2017

Uh oh!

Priya91 Dec 14, 2017

Uh oh!

karelz Dec 14, 2017

Uh oh!

Priya91 Dec 14, 2017

Uh oh!

karelz Dec 14, 2017

Uh oh!

karelz Dec 14, 2017

Uh oh!

Priya91 commented Dec 15, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Use generic test handling for remote server failures. #25910

Use generic test handling for remote server failures. #25910

Uh oh!

Conversation

Priya91 commented Dec 14, 2017

Uh oh!

Priya91 commented Dec 14, 2017

Uh oh!

Priya91 commented Dec 14, 2017

Uh oh!

wfurt commented Dec 14, 2017

Uh oh!

Priya91 commented Dec 14, 2017

Uh oh!

karelz Dec 14, 2017

Choose a reason for hiding this comment

Uh oh!

Priya91 Dec 14, 2017

Choose a reason for hiding this comment

Uh oh!

karelz Dec 14, 2017

Choose a reason for hiding this comment

Uh oh!

Priya91 Dec 14, 2017

Choose a reason for hiding this comment

Uh oh!

karelz Dec 14, 2017

Choose a reason for hiding this comment

Uh oh!

karelz Dec 14, 2017

Choose a reason for hiding this comment

Uh oh!

Priya91 commented Dec 15, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants