Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Fix Loop in System.Private.Uri #37733

Closed
wtgodbe wants to merge 2 commits into from
Closed

Fix Loop in System.Private.Uri #37733

wtgodbe wants to merge 2 commits into from

Conversation

@wtgodbe
Copy link
Member

@wtgodbe wtgodbe commented May 16, 2019

Ports 6da61ff to master

CC @karelz @davidsh

@wtgodbe
Copy link
Member Author

wtgodbe commented May 17, 2019

Test failure in System.Net.NetworkInformation.Tests.PingTest/SendPingAsyncWithHostAndTtlAndFragmentPingOptions on OSX10.12:

Assert.Equal() Failure
Expected: Success
Actual: TimedOut

at System.Net.NetworkInformation.Tests.PingTest.PingResultValidator(PingReply pingReply, IPAddress[] localIpAddresses) in /Users/vsts/agent/2.150.3/work/1/s/src/System.Net.Ping/tests/FunctionalTests/PingTest.cs:line 61
at System.Net.NetworkInformation.Tests.PingTest.<>c__DisplayClass46_0.b__1(PingReply pingReply) in /Users/vsts/agent/2.150.3/work/1/s/src/System.Net.Ping/tests/FunctionalTests/PingTest.cs:line 806
at System.Net.NetworkInformation.Tests.PingTest.SendPingAsync(Func2 sendPing, Action1 pingResultValidator) in /Users/vsts/agent/2.150.3/work/1/s/src/System.Net.Ping/tests/FunctionalTests/PingTest.cs:line 776
at System.Net.NetworkInformation.Tests.PingTest.SendPingAsyncWithHostAndTtlAndFragmentPingOptions(Boolean fragment) in /Users/vsts/agent/2.150.3/work/1/s/src/System.Net.Ping/tests/FunctionalTests/PingTest.cs:line 802
--- End of stack trace from previous location where exception was thrown ---

@wfurt is this a known issue on Mac? It seems like it should be unrelated, but I haven't dug further yet.

@wtgodbe
Copy link
Member Author

wtgodbe commented May 17, 2019

System.Private.Uri.Functional.Tests: [Long Running Test] 'System.PrivateUri.Tests.IriTest.Iri_ExpandingContents_ThrowsIfTooLong', Elapsed: 00:14:11

This test timed out after 15 minutes on Windows.10.Amd64.ClientRS5.Open-x86:Release. On Debug, the test took about 45 seconds. Looking more now, as this is the test case that was added with this PR

stephentoub
stephentoub reviewed May 17, 2019
View reviewed changes
src/System.Private.Uri/tests/FunctionalTests/IriTest.cs Outdated
// The second, 65535 (ushort.MaxValue) is only reachable via expansion as a result of
// percent encoding. Exceeding this value used to result in a hang, but now results in
// an exception.
static int ExpandedLengthLimit = 65535;
Copy link
Member

@stephentoub stephentoub May 17, 2019

Choose a reason for hiding this comment

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

Nit: private for both of these

@wtgodbe wtgodbe mentioned this pull request May 20, 2019
Merged
@wtgodbe
Copy link
Member Author

wtgodbe commented May 20, 2019

The added test case once again timed out on Windows.10.Amd64.ClientRS5.Open-x86:Release, so it's definitely consistent.

@davidsh davidsh added this to the 3.0 milestone May 25, 2019
@davidsh davidsh mentioned this pull request May 27, 2019
Merged
@wtgodbe wtgodbe closed this May 30, 2019
@davidsh davidsh mentioned this pull request May 30, 2019
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@stephentoub stephentoub stephentoub approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area-System.Net

Projects

None yet

Milestone

3.0

Development

Successfully merging this pull request may close these issues.

3 participants

@wtgodbe @stephentoub @davidsh