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

[iOS][libraries] Skip failing iOS tests with ActiveIssues and proj level skips #51491

Merged
merged 78 commits into from
May 12, 2021

Conversation

mdh1418
Copy link
Member

@mdh1418 mdh1418 commented Apr 19, 2021

In effort to assess the state of the iOS library tests, a number of tests will be skipped

Test suites that crash, hang, or have a significant number of failures (mostly 20+ failing unique test methods) are skipped on at the tests.proj level. Otherwise, [ActiveIssue] attributes have been attached to either the individual test methods that fail or the entire class if the whole test class fails. Any pre-existing ActiveIssue attribute associated with iOS had been reassessed to either remain or be closed.

A few test suites have been skipped because they fail with some form of PlatformNotSupportedException. [SkipOnPlatform] attributes are attached to the failing test methods.

All of the Issues can be tracked at https://github.com/dotnet/runtime/projects/48#column-9236436

The ActiveIssues and tests.proj exclusions have been tested on #49917 as well.


This PR makes the following changes in the following files:
src/libraries/System.IO.Ports/tests/System.IO.Ports.Tests.csproj - Removed $(NetCoreAppCurrent)-iOS from target frameworks as it is stated to currently only supported on Windows.

src/libraries/tests.proj - Update the test suites that need to be skipped at the project level, reducing the number and categorizing as a skip due to PNSE, a skip due to numerous failures, and a skip due to the suite crashing.

The remaining files changed are either:
[ActiveIssue("<issue-url>", TestPlatforms.iOS | TestPlatforms.tvOS | TestPlatforms.MacCatalyst)]
[SkipOnPlatform(TestPlatforms.iOS | TestPlatforms.tvOS, "Not supported on iOS or tvOS.")]

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@mdh1418 mdh1418 changed the title [iOS][libraries] Skip failing Android tests with ActiveIssues and proj level skips [iOS][libraries] Skip failing iOS tests with ActiveIssues and proj level skips Apr 19, 2021
@mdh1418 mdh1418 requested a review from tqiu8 April 19, 2021 17:44
@mdh1418 mdh1418 force-pushed the skip_ios_library_tests branch 6 times, most recently from 521b003 to bfa8f8a Compare April 29, 2021 21:13
@marek-safar
Copy link
Contributor

A few test suites have been skipped because they fail with some form of PlatformNotSupportedException.

Which are they?

@mdh1418
Copy link
Member Author

mdh1418 commented Apr 30, 2021

@marek-safar So far the PNSE failure test suites being completely skipped are System.Console.Tests.csproj, System.IO.FileSystem.Tests.csproj, System.Security.Cryptography.OpenSsl.Tests.csproj, System.Threading.Thread.Tests.csproj. There is some more consolidation and checking to be done, especially since tests cannot run to completion related to https://github.com/dotnet/core-eng/issues/12969

There are some test facts that fail with PNSE that are skipped individually, I'll need to ensure that they're being tracked/marked with some note of PNSE.

@marek-safar
Copy link
Contributor

Out of these 4 I think only System.Security.Cryptography.OpenSsl.Tests.csproj should be skipped completely.

@mdh1418 mdh1418 force-pushed the skip_ios_library_tests branch from c144b51 to 05a9946 Compare April 30, 2021 19:15
@tqiu8
Copy link
Contributor

tqiu8 commented Apr 30, 2021

On my local machine System.Threading.Thread is passing.

@mdh1418
Copy link
Member Author

mdh1418 commented May 3, 2021

@tqiu8 when you ran System.Threading.Thread.Tests, was that including this commit? ca51de9 (Which skips the test facts)

@mdh1418
Copy link
Member Author

mdh1418 commented May 3, 2021

@marek-safar Of all the previous CI runs of iOS and tvOS, the following suites have the following number of methods that fail with PNSE. Should System.Security.Cryptography.X509Certificates.Tests also be skipped completely?


System.Console.Tests.dll - 46
System.Diagnostics.Process.Tests.dll - 12
System.IO.FileSystem.Tests.dll - 3 
System.Net.NetworkInformation.Functional.Tests.dll - 1 
System.Net.Ping.Functional.Tests.dll - 25 
System.Security.Cryptography.OpenSsl.Tests.dll - 726
System.Security.Cryptography.X509Certificates.Tests.dll - 163
System.Security.Cryptography.Xml.Tests.dll - 3 
System.Runtime.Extensions.Tests.dll - 1
System.Threading.Thread.Tests.dll - 1
System.Xml.RW.XmlReader.Tests.dll - 3 

all_ios_tvos_PNSE_us.txt

@tqiu8
Copy link
Contributor

tqiu8 commented May 3, 2021

@tqiu8 when you ran System.Threading.Thread.Tests, was that including this commit? ca51de9 (Which skips the test facts)

Yep, that's probably what happened.

@marek-safar
Copy link
Contributor

Should System.Security.Cryptography.X509Certificates.Tests also be skipped completely?

Preferably not

Mitchell Hwang added 23 commits May 12, 2021 09:22
…m.Net.NameResolution.Unit test suite skips
This reverts commit 163ff12da16e602e52830c79f267e14c420284c0.
@@ -80,6 +80,7 @@ public void KeySizeProp()
}

[Theory, MemberData(nameof(TestNewCurves))]
[ActiveIssue("https://github.com/dotnet/runtime/issues/51332", TestPlatforms.iOS | TestPlatforms.tvOS | TestPlatforms.MacCatalyst)]
Copy link
Member

Choose a reason for hiding this comment

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

Yup sounds good, but we'll still go ahead and merge this PR for now to get to a green state in runtime-staging :)

@akoeplinger akoeplinger merged commit 5297337 into dotnet:main May 12, 2021
@mdh1418 mdh1418 deleted the skip_ios_library_tests branch May 12, 2021 15:41
@karelz karelz added this to the 6.0.0 milestone May 20, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jun 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants