Skip to content

Conversation

@antonfirsov
Copy link
Contributor

@antonfirsov antonfirsov commented Nov 3, 2020

dotnet/corefx#15697 changed this value to 1, and no-one touched it since then.

As the comment in that PR points out, the root cause of the UDP test failures wasn't packet loss, but IPv4 / IPv6 port collision on Unix in dual-mode cases.

The value and the for loops are complicating Socket test code unnecessarily, simplification seems reasonable.

/cc @geoffkizer @stephentoub

@ghost
Copy link

ghost commented Nov 3, 2020

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

@antonfirsov
Copy link
Contributor Author

/cc @wfurt

@antonfirsov antonfirsov added the test-enhancement Improvements of test source code label Nov 3, 2020
@antonfirsov antonfirsov requested a review from a team November 3, 2020 16:31
Copy link
Member

@stephentoub stephentoub left a comment

Choose a reason for hiding this comment

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

LGTM if @wfurt is happy with it (he's been looking a lot at test failures).

@wfurt
Copy link
Member

wfurt commented Nov 3, 2020

I think this may be ok - I certainly like the simplification. We can watch the results and react if we see new failures.

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

@wfurt
Copy link
Member

wfurt commented Nov 3, 2020

/azp run runtime-libraries-coreclr outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@antonfirsov
Copy link
Contributor Author

The System.Diagnostics.Tests failures are unrelated.

The Read_PartiallySatisfied_RemainderOfBufferUntouched failure seems to be related to #44214.

@antonfirsov
Copy link
Contributor Author

antonfirsov commented Nov 3, 2020

The outerloop test hang #44219 seems to be unrelated, since NetworkStreamTest is untouched by this PR.
Same for telemetry failure #44221.

@antonfirsov
Copy link
Contributor Author

/azp run runtime-libraries-coreclr outerloop

@antonfirsov
Copy link
Contributor Author

/azp run runtime

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

1 similar comment
@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@antonfirsov antonfirsov merged commit bd6c905 into dotnet:master Nov 4, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 6, 2020
@karelz karelz added this to the 6.0.0 milestone Jan 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.Net.Sockets test-enhancement Improvements of test source code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants