Skip to content

Fix Connect_DualMode_DnsConnect_RetrievedEndPoints_Success on Linux #54681

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
Merged
merged 4 commits into from
Jun 29, 2021
Merged

Fix Connect_DualMode_DnsConnect_RetrievedEndPoints_Success on Linux #54681

merged 4 commits into from
Jun 29, 2021

Conversation

@antonfirsov
Copy link
Contributor

@antonfirsov antonfirsov commented Jun 24, 2021 •
edited
Loading

50 failures in ~20 days. See #54677 (comment).

/cc @tmds

Fixes #54677

@antonfirsov antonfirsov requested a review from a team June 24, 2021 12:26
@ghost
Copy link

ghost commented Jun 24, 2021

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

Issue Details

50 failures in ~20 days. See #54677 (comment).

/cc @tmds

Author: antonfirsov
Assignees: -
Labels:

area-System.Net.Sockets
Milestone: -

tmds
tmds reviewed Jun 24, 2021
View reviewed changes
src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs Outdated
}

[Fact]
[ActiveIssue("https://github.com/dotnet/runtime/issues/54677", TestPlatforms.Linux)]
Copy link
Member

@tmds tmds Jun 24, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

@antonfirsov can you update the test so both IPAddress.Loopback.MapToIPv6() and IPAddress.IPv6Loopback are allowed for localEndPoint.Address?

I expect for remoteEndPoint.Address we should stick to IPAddress.Loopback.MapToIPv6() only because the test server is bound to IPAddress.Loopback.

Copy link
Contributor Author

@antonfirsov antonfirsov Jun 24, 2021

Choose a reason for hiding this comment

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

You are right. I overlooked the exception message, and for some reason thought LocalEndPoint was IPv4, not ::1.
Makes sense to fix the test instead of disabling.

@antonfirsov
Copy link
Contributor Author

antonfirsov commented Jun 24, 2021

I updated this to fix #54677, instead of disabling, let's wait for CI before merging.

@antonfirsov antonfirsov changed the title Disable Connect_DualMode_DnsConnect_RetrievedEndPoints_Success on Linux Fix Connect_DualMode_DnsConnect_RetrievedEndPoints_Success on Linux Jun 24, 2021
@antonfirsov
Copy link
Contributor Author

antonfirsov commented Jun 24, 2021

/azp run runtime

@azure-pipelines
Copy link

azure-pipelines bot commented Jun 24, 2021

Azure Pipelines successfully started running 1 pipeline(s).

@danmoseley
Copy link
Member

danmoseley commented Jun 25, 2021

@stephentoub this looks related to your change:

Assert.Throws() Failure
Expected: typeof(System.OperationCanceledException)
Actual:   (No exception was thrown)


Stack trace
   at System.Threading.Tests.PeriodicTimerTests.WaitForNextTickAsync_CanceledWaitThenWaitAgain_Succeeds() in /_/src/libraries/System.Runtime/tests/System/Threading/PeriodicTimerTests.cs:line 186
--- End of stack trace from previous location ---

https://dev.azure.com/dnceng/public/_build/results?buildId=1204988&view=ms.vss-test-web.build-test-results-tab&runId=36060014&resultId=173000&paneView=debug

@runfoapp runfoapp bot mentioned this pull request Jun 25, 2021
Closed
@stephentoub
Copy link
Member

stephentoub commented Jun 25, 2021

@danmoseley, that's #54713

@danmoseley
Copy link
Member

danmoseley commented Jun 25, 2021

@stephentoub thanks, I pasted in the wrong output.

tmds
tmds reviewed Jun 28, 2021
View reviewed changes
tmds
tmds approved these changes Jun 29, 2021
View reviewed changes
Copy link
Member

@tmds tmds left a comment

Choose a reason for hiding this comment

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

LGTM

@antonfirsov antonfirsov merged commit 1b61065 into main Jun 29, 2021
@jkotas jkotas deleted the antonfirsov/disable-Connect_DualMode_DnsConnect_RetrievedEndPoints_Success branch July 1, 2021 04:33
@karelz karelz added this to the 6.0.0 milestone Jul 15, 2021
This was referenced Jul 20, 2021
Closed
Closed
Closed
Closed
@antonfirsov antonfirsov mentioned this pull request Jul 23, 2021
Merged
@karelz karelz mentioned this pull request Jul 28, 2021
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Aug 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@karelz karelz karelz approved these changes

+1 more reviewer

@tmds tmds tmds approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area-System.Net.Sockets

Projects

None yet

Milestone

6.0.0

Development

Successfully merging this pull request may close these issues.

Connect_DualMode_DnsConnect_RetrievedEndPoints_Success - Assert ::1

6 participants

@antonfirsov @danmoseley @stephentoub @tmds @karelz