Skip to content

Disabling two socket tests #93916

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

Merged
merged 3 commits into from
Oct 29, 2023
Merged

Disabling two socket tests #93916

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
43d93f6
Disable two socket tests
liveans Oct 24, 2023
8635e0f
Revert close to dispose
liveans Oct 24, 2023
cc5a753
Disable tests against one issue
liveans Oct 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,12 @@ public async Task Connect_AfterDisconnect_Fails()
[InlineData("[::ffff:1.1.1.1]", true, false)]
public async Task ConnectGetsCanceledByDispose(string addressString, bool useDns, bool owning)
{
if (UsesSync && !PlatformDetection.IsWindows)
Copy link
Member

Choose a reason for hiding this comment

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

Is there need to disable this also on macOS? It feels like !IsWindows should really be IsLinux????
Also this is regression in particular Linux Kernels. At that seems rare at this moment. It may be worth of adding version check instead of blindly disabling Linux runs -> it should still work on most supported distributions AFAIK.

Copy link
Member Author

@liveans liveans Oct 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Ah yes, I've missed it. It should work as expected on macOS, thanks for catching that. I'll change it with another PR.

For the version controlling, AFAIK, only the latest kernel versions have this problem. Is there any example of version checking on Linux in .NET?

Copy link
Member

Choose a reason for hiding this comment

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

It may be worth of adding version check instead of blindly disabling Linux runs

The change is being backported to some kernels, since it's considered to be security sensitive. I think it would be very difficult if not impossible to determine only by kernel version if a system is impacted.

{
// [ActiveIssue("https://github.com/dotnet/runtime/issues/94149", TestPlatforms.Linux)]
return;
}

// Aborting sync operations for non-owning handles is not supported on Unix.
if (!owning && UsesSync && !PlatformDetection.IsWindows)
{
Expand Down
1 change: 1 addition & 0 deletions src/libraries/System.Net.Sockets/tests/FunctionalTests/TelemetryTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ await listener.RunWithCallbackAsync(e => events.Enqueue((e, e.ActivityId)), asyn
}, connectMethod, useDnsEndPoint.ToString()).Dispose();
}

[ActiveIssue("https://github.com/dotnet/runtime/issues/94149", TestPlatforms.Linux)]
[OuterLoop]
[ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
[SkipOnPlatform(TestPlatforms.OSX | TestPlatforms.FreeBSD, "Same as Connect.ConnectGetsCanceledByDispose")]
Expand Down