Skip to content
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.

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

Organize SendReceive tests and isolate non-parallel test collection #44591

Merged
merged 7 commits into from
Nov 16, 2020

Conversation

antonfirsov
Copy link
Member

@antonfirsov antonfirsov commented Nov 12, 2020

Some SendReceive socket tests may be prone to timing issues on CI. This seems to be the root cause of #1712. We need a reliable way to run such tests to unblock the work on new UDP socket API-s in #33418.

This PR defines a new SendReceiveNonParallel test group, moving SendToRecvFrom_Datagram_UDP into that group. Since this is already a significant reorganization, it seemed reasonable to also:

  • Harmonize naming: all SendReceive test classses are now named either SendReceive_[SubVariant]
    or SendReceiveNonParallel_[SubVariant]
  • Split SendReceive.cs into multiple files:
    • SendReceive.cs for the parallel variants
    • SendReceiveNonParallel.cs for the new, non-parallel variants
    • Rename the non-generic class SendReceive to SendReceiveCommon (to avoid name collision and confusion with SendReceive<T>) and move it to SendReceiveCommon.cs
    • Move SendReceiveListener and SendReceiveUdpClient to separate files

Fixes #1712

/cc @geoffkizer

@ghost
Copy link

ghost commented Nov 12, 2020

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


Issue meta data
Issue content:
Some `SendReceive` socket tests may be prone to timing issues on CI. This seems to be the root cause of #1712. We need a reliable way to run such tests to unblock the work on new UDP socket API-s in #33418.

This PR defines a new SendReceiveNonParallel test group, moving SendToRecvFrom_Datagram_UDP into that group. Since this is already a significant reorganization, it seemed reasonable to also:

  • Harmonize naming: all SendReceive test classses are now named either SendReceive_[SubVariant]
    or SendReceiveNonParallel_[SubVariant]
  • Split SendReceive.cs into multiple files:
    • SendReceive.cs for the parallel variants
    • SendReceiveNonParallel.cs for the new, non-parallel variants
    • Renamed the non-generic class SendReceive to SendReceiveCommon (to avoid name collision and confusion with SendReceive<T>) and moved it SendReceiveCommon.cs
    • Moved SendReceiveListener and SendReceiveUdpClient to separate files

Fixes #1712

/cc @geoffkizer

</td>
Issue author: antonfirsov
Assignees: -
Labels:
`area-System.Net.Sockets`

</td>
Milestone: -

@antonfirsov

This comment has been minimized.

@azure-pipelines

This comment has been minimized.

Copy link
Contributor

@geoffkizer geoffkizer left a comment

Choose a reason for hiding this comment

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

Generally looks good, but I think we should change a couple class/file names... see above.

@antonfirsov

This comment has been minimized.

@antonfirsov
Copy link
Member Author

@geoffkizer all addressed.

@azure-pipelines

This comment has been minimized.

@antonfirsov
Copy link
Member Author

The OuterLoop test failures are unrelated (System.Xml, Http and #44015).

@antonfirsov antonfirsov merged commit 83d8561 into dotnet:master Nov 16, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 16, 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test: System.Net.Sockets.Tests.SendReceiveEap.SendToRecvFrom_Datagram_UDP(loopbackAddress: ::1) failed in CI
4 participants