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

[Libraries][iOS/MacCatalyst/tvOS] IsIanaIdTest fails from mismatched expectation #58440

Closed
Tracked by #64603
mdh1418 opened this issue Aug 31, 2021 · 10 comments · Fixed by #58562
Closed
Tracked by #64603

[Libraries][iOS/MacCatalyst/tvOS] IsIanaIdTest fails from mismatched expectation #58440

mdh1418 opened this issue Aug 31, 2021 · 10 comments · Fixed by #58562
Labels
area-System.Runtime in-pr There is an active PR which will close this issue when it is merged os-ios Apple iOS os-maccatalyst MacCatalyst OS os-tvos Apple tvOS
Milestone

Comments

@mdh1418
Copy link
Member

mdh1418 commented Aug 31, 2021

In #57732, several TimeZoneInfoTests were tweaked to pass on iOS/MacCatalyst/tvOS. At the time, when running tests, America/Buenos_Aires was no recognized as an IanaID on iOS/MacCatalyst/tvOS, so that value was used instead of Pacific Standard Time which throws TimeZoneNotFound ahead of returning false for IsIanaId.

I'm not certain what is supposed to be a TimeZoneId and not IanaId on iOS/MacCatalyst/tvOS yet, and I'm not sure exactly what had changed between that change and the latest run on #57208 to cause it to no longer not be an IanaId.

The fix should simply be finding a known TimeZoneId on iOS/MacCatalyst/tvOS that is and will not be recognized as an IanaId.


From another glance with #58440 (comment), it appears that the failure is directly from this addition to the test

Assert.Equal((expected || TimeZoneInfo.Local.Id.Equals("Utc", StringComparison.OrdinalIgnoreCase)), TimeZoneInfo.Local.HasIanaId);

The newest check fails on iOS/MacCatalyst/tvOS as the TimeZoneInfo.Local.Id is America/New_York. The check expects for the TimeZoneInfo.Local.Id to be an Iana Id if the platform is NOT windows or if the Id is NOT UTC. It seems as though America/New_York is evaluated as not being an Iana Id.

@mdh1418 mdh1418 added this to the 7.0.0 milestone Aug 31, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Aug 31, 2021
@dotnet-issue-labeler
Copy link

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

@ghost
Copy link

ghost commented Sep 1, 2021

Tagging subscribers to this area: @tarekgh, @safern
See info in area-owners.md if you want to be subscribed.

Issue Details

In #57732, several TimeZoneInfoTests were tweaked to pass on iOS/MacCatalyst/tvOS. At the time, when running tests, America/Buenos_Aires was no recognized as an IanaID on iOS/MacCatalyst/tvOS, so that value was used instead of Pacific Standard Time which throws TimeZoneNotFound ahead of returning false for IsIanaId.

I'm not certain what is supposed to be a TimeZoneId and not IanaId on iOS/MacCatalyst/tvOS yet, and I'm not sure exactly what had changed between that change and the latest run on #57208 to cause it to no longer not be an IanaId.

The fix should simply be finding a known TimeZoneId on iOS/MacCatalyst/tvOS that is and will not be recognized as an IanaId.

Author: mdh1418
Assignees: -
Labels:

area-System.Globalization

Milestone: 7.0.0

@ghost
Copy link

ghost commented Sep 1, 2021

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

Issue Details

In #57732, several TimeZoneInfoTests were tweaked to pass on iOS/MacCatalyst/tvOS. At the time, when running tests, America/Buenos_Aires was no recognized as an IanaID on iOS/MacCatalyst/tvOS, so that value was used instead of Pacific Standard Time which throws TimeZoneNotFound ahead of returning false for IsIanaId.

I'm not certain what is supposed to be a TimeZoneId and not IanaId on iOS/MacCatalyst/tvOS yet, and I'm not sure exactly what had changed between that change and the latest run on #57208 to cause it to no longer not be an IanaId.

The fix should simply be finding a known TimeZoneId on iOS/MacCatalyst/tvOS that is and will not be recognized as an IanaId.

Author: mdh1418
Assignees: -
Labels:

area-System.Runtime

Milestone: 7.0.0

@tarekgh tarekgh added os-ios Apple iOS os-maccatalyst MacCatalyst OS os-tvos Apple tvOS labels Sep 1, 2021
@tarekgh
Copy link
Member

tarekgh commented Sep 1, 2021

@mdh1418 we had a bug which we have recently fixed it #58326. could be this is the reason?

@ghost
Copy link

ghost commented Sep 1, 2021

Tagging subscribers to 'arch-ios': @steveisok, @akoeplinger
See info in area-owners.md if you want to be subscribed.

Issue Details

In #57732, several TimeZoneInfoTests were tweaked to pass on iOS/MacCatalyst/tvOS. At the time, when running tests, America/Buenos_Aires was no recognized as an IanaID on iOS/MacCatalyst/tvOS, so that value was used instead of Pacific Standard Time which throws TimeZoneNotFound ahead of returning false for IsIanaId.

I'm not certain what is supposed to be a TimeZoneId and not IanaId on iOS/MacCatalyst/tvOS yet, and I'm not sure exactly what had changed between that change and the latest run on #57208 to cause it to no longer not be an IanaId.

The fix should simply be finding a known TimeZoneId on iOS/MacCatalyst/tvOS that is and will not be recognized as an IanaId.

Author: mdh1418
Assignees: -
Labels:

area-System.Runtime, os-ios, os-tvos, os-maccatalyst

Milestone: 7.0.0

@mdh1418
Copy link
Member Author

mdh1418 commented Sep 1, 2021

@tarekgh Ah it does look like the culprit instead of my original suspicion. It looks like on iOS/tvOS/MacCatalyst the TimeZoneInfo.Local.Id is America/New_York (atleast on my local machine), and so that is evaluated as having an Iana Id

@tarekgh
Copy link
Member

tarekgh commented Sep 1, 2021

Ah it does look like the culprit instead of my original suspicion. It looks like on iOS/tvOS/MacCatalyst the TimeZoneInfo.Local.Id is America/New_York (atleast on my local machine), and so that is evaluated as having an Iana Id

America/New_York is IANA Id. So, what you are seeing there is correct. So, still look to me the problem is in the test. right?

@mdh1418
Copy link
Member Author

mdh1418 commented Sep 2, 2021

Right, the problem is the test itself, I'm thinking the right action would be to extend
expected || TimeZoneInfo.Local.Id.Equals("Utc", StringComparison.OrdinalIgnoreCase)
to
expected || PlatformDetection.IsiOS || PlatformDetection.IstvOS || TimeZoneInfo.Local.Id.Equals("Utc", StringComparison.OrdinalIgnoreCase)

I'll put up a PR

@tarekgh
Copy link
Member

tarekgh commented Sep 2, 2021

I am not sure why you need to add these extra conditions? Could you tell in detail what are the TZ Ids which fail on iOS or tvOS?

@mdh1418 mdh1418 changed the title [Libraries][iOS/MacCatalyst/tvOS] IsIanaIdTest misidentifies America/Buenos_Aires as not an IanaId [Libraries][iOS/MacCatalyst/tvOS] IsIanaIdTest fails from mismatched expectation Sep 2, 2021
@mdh1418
Copy link
Member Author

mdh1418 commented Sep 2, 2021

@tarekgh I updated the description since my first glance/assumption was incorrect.

Could you tell in detail what are the TZ Ids which fail on iOS or tvOS?

The only failure seems to be that iOS/MacCatalyst/tvOS has a TimeZoneInfo.Local.Id that is an Iana Id that is not UTC.
To me it seems like its a problem with the check specifically for iOS/MacCatalyst/tvOS.

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Sep 2, 2021
steveisok pushed a commit that referenced this issue Sep 13, 2021
…t and test fixes for Android (#58841)

Manual backport of #57208, #58519, #58562, #58210, #57732, #58428, #58586, #58745, #57687 to release/6.0

Numerous test suites have been failing for iOS/tvOS/MacCatalyst consistently on CI without useful logs as to why. Moreover, some of these suites pass locally.

This PR looks to reduce the failures on CI by skipping the problematic suites
Skips test suites logged in #53624

ActiveIssues
#58440
#58418
#58367
#58584

Co-authored-by: Mitchell Hwang <[email protected]>
Co-authored-by: Alexander Köplinger <[email protected]>
Co-authored-by: Jo Shields <[email protected]>
@ghost ghost locked as resolved and limited conversation to collaborators Oct 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Runtime in-pr There is an active PR which will close this issue when it is merged os-ios Apple iOS os-maccatalyst MacCatalyst OS os-tvos Apple tvOS
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants