Skip to content

Disable more SendPacketsElement tests on Win11 #59359

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
antonfirsov merged 2 commits into from
Sep 23, 2021
Merged

Disable more SendPacketsElement tests on Win11 #59359

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
070d352
Disable more SendPacketsElement tests on Win11
antonfirsov Sep 20, 2021
d26494f
instead of SkipTestException, use ConditionalFact with an actual prop…
antonfirsov Sep 21, 2021
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
13 changes: 12 additions & 1 deletion src/libraries/System.Net.Sockets/tests/FunctionalTests/SendPacketsAsync.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,12 @@ public void SendPacketsElement_FileStreamMultiPart_Success()
[Fact]
public void SendPacketsElement_FileStreamLargeOffset_Throws()
{
if (PlatformDetection.IsWindows10Version22000OrGreater)
{
// [ActiveIssue("https://github.com/dotnet/runtime/issues/58898")]
Copy link
Contributor

@carlossanlop carlossanlop Sep 21, 2021

Choose a reason for hiding this comment

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

Isn't it better to have the [ActiveIssue] attribute and add the platform detection as a [ConditionalFact] attribute?

Copy link
Contributor Author

@antonfirsov antonfirsov Sep 21, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Addressed in d26494f. The SkipTestException trick somehow became a repo-wide practice for skipping on Windows 11, also not big fan of it.

Copy link
Member

@wfurt wfurt Sep 21, 2021

Choose a reason for hiding this comment

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

At least for the SslStream I see this is temporary. For me this seems to have less overhead comparing to build new conditional variables. The other part is that we would see it in database and it is visible during test run e.g. serves as reminder as well.

Copy link
Contributor Author

@antonfirsov antonfirsov Sep 23, 2021

Choose a reason for hiding this comment

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

The other part is that we would see it in database and it is visible during test run e.g. serves as reminder as well.

This makes sense, though I prefer to merge the PR as-is for now. We need to deal with #58898 with high priority anyways.

throw new SkipTestException("Unstable on Windows 11");
}

using (var stream = new FileStream(TestFileName, FileMode.Open, FileAccess.Read, FileShare.Read, 4096, useAsync: true))
{
stream.Seek(s_testFileSize / 2, SeekOrigin.Begin);
Expand All @@ -552,6 +558,12 @@ public void SendPacketsElement_FileStreamLargeCount_Throws()

[Fact]
public void SendPacketsElement_FileStreamWithOptions_Success() {
if (PlatformDetection.IsWindows10Version22000OrGreater)
{
// [ActiveIssue("https://github.com/dotnet/runtime/issues/58898")]
throw new SkipTestException("Unstable on Windows 11");
}

using (var stream = new FileStream(TestFileName, FileMode.Open, FileAccess.Read, FileShare.ReadWrite, 4096, FileOptions.Asynchronous | FileOptions.SequentialScan)) {
var element = new SendPacketsElement(stream, 0, s_testFileSize);
SendPackets(element, s_testFileSize, GetExpectedContent(element));
Expand Down Expand Up @@ -583,7 +595,6 @@ public void SendPacketsElement_FileStreamMultiPartMixed_Success() {

[ConditionalFact]
public void SendPacketsElement_FileStreamMultiPartMixed_MultipleFileStreams_Success() {

if (PlatformDetection.IsWindows10Version22000OrGreater)
{
// [ActiveIssue("https://github.com/dotnet/runtime/issues/58898")]
Expand Down