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

[release/6.0] Skip test suites crashing on CI for iOS/tvOS/MacCatalyst and test fixes for Android #58841

Merged
merged 10 commits into from
Sep 13, 2021

Conversation

mdh1418
Copy link
Member

@mdh1418 mdh1418 commented Sep 8, 2021

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

/cc @mdh1418

Customer Impact

A majority of the changes are all dev-facing and are related to testing libraries. This include #57208, #58519, #58562, #58210, #57732, #58428, #58745

#58586 introduces a change that fixes when 32 bit arm Android apps getting an incorrect flag

#57687 adds mtriple values for MacCatalyst for proper mono AOT linking during the AppleAppBuilder task, it was primarily included because the error was hit in the previous CI Build run

Testing

This change is related to testing with a primary focus to re-enable test suites that are no longer crashing, and disable test suites and tests that still crash or are new crashes.

Risk

Low, the majority of changes are related to testing. #58586 fixes a flag issue on 32bit arm Android apps, and #57687 fixes a linking issue but is noted to be slow in the PR description.

@mdh1418 mdh1418 added this to the 6.0.0 milestone Sep 8, 2021
@ghost
Copy link

ghost commented Sep 8, 2021

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

Issue Details

Manual backport of #57208 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

/cc @mdh1418

Customer Impact

None, the changes are all dev-facing and are related to testing libraries.

Testing

This change is related to testing with a primary focus to re-enable test suites that are no longer crashing, and disable test suites and tests that still crash or are new crashes.

Risk

Low, the changes are related to testing.

Author: mdh1418
Assignees: -
Labels:

area-Infrastructure-libraries

Milestone: 6.0.0

@mdh1418
Copy link
Member Author

mdh1418 commented Sep 8, 2021

This is a WIP, there are more changes I plan to include in this backport.

mdh1418 and others added 4 commits September 8, 2021 19:45
…et#58519)

Co-authored-by: Mitchell Hwang <mitchell.hwang@microsoft.com>
…OS (dotnet#58562)

* [Libraries] Fix TimeZoneInfoTests IsIanaIdTest for iOS/MacCatalyst/tvOS

* [Libraries] Fix IsIanaIdTest to assert throw for iOS/MacCatalyst/tvOS

* Readd new line

* [Mobile][Libraries] System.Runtime.Tests TimeZoneInfoTests remove mobile from iana conversion support

* [libraries] System.Runtime.Tests revert IsIanaIdTest iOS tvOS special casing

* Remove active issue added in recently merged PR

Co-authored-by: Mitchell Hwang <mitchell.hwang@microsoft.com>
…alyst/tvOS (dotnet#57732)

* [libraries] Fix System.Runtime.Tests TimeZoneInfoTests for iOS/MacCatalyst/tvOS

* Remove redundant PlatformDetection.IsMacCatalyst as IsiOS covers it

Co-authored-by: Mitchell Hwang <mitchell.hwang@microsoft.com>
@mdh1418
Copy link
Member Author

mdh1418 commented Sep 9, 2021

@akoeplinger Were there more commits that you were aware of that I missed in regards to making runtime-staging more green?

@mdh1418 mdh1418 requested review from steveisok, akoeplinger and marek-safar and removed request for steveisok September 9, 2021 00:08
@akoeplinger akoeplinger changed the title [release/6.0] Skip test suites crashing on CI for iOS/tvOS/MacCatalyst [release/6.0] Skip test suites crashing on CI for iOS/tvOS/MacCatalyst and test fixes for Android Sep 9, 2021
- Use F_SETLK64 for fcntl to fix an issue where 32bit arm Android apps would get the wrong flag
- Add numerical magic numbers for missing file systems to Interop.UnixFileSystemTypes so they are correctly recognized
- Make ping test a bit more forgiving if the process returns a few milliseconds _before_ the timeout

(cherry picked from commit 462ab6e)
…le (dotnet#58745)

The TestSettings.UnreachableAddress is using 192.0.2.0 which is documented in RFC5735 as a network to be used in docs/example code.
On the Android devices in Helix the upstream network is configured to not route this address which results in a "Destination Net Unreachable" response
which causes the timeout argument to ping to be ignored and ping returns immediately.
Skip the test in these cases.

Example output from ping on the device:

```
1|sunfish:/ $ time ping -c 1 -W 2 -s 50 192.0.2.0
PING 192.0.2.0 (192.0.2.0) 50(78) bytes of data.
From 131.107.5.118: icmp_seq=1 Destination Net Unreachable

--- 192.0.2.0 ping statistics ---
1 packets transmitted, 0 received, +1 errors, 100% packet loss, time Oms

   0m00.03s real   0m00.00s user   0m00.00s system
```

(cherry picked from commit 84fb871)
@akoeplinger
Copy link
Member

akoeplinger commented Sep 9, 2021

@mdh1418 I pushed the commits

…otnet#57687)

Effectively, we are correctly saying "build a Mac Catalyst app from runtime
and these `-llvm.o` files, but the `-llvm.o` files are not correctly being
built in a Mac Catalyst compatible way, resulting in the error

```
  ld: building for Mac Catalyst, but linking in object file built for , file '/Users/filipnavara/Projects/llvmbug/obj/Debug/net6.0-maccatalyst/maccatalyst-arm64/nativelibraries/aot-output/arm64/llvmbug.dll.llvm.o'
```

The target platform parameter gets passed around a LOT -
`src/tasks/AotCompilerTask/MonoAOTCompiler.props` specifies
`<MonoAOTCompilerDefaultAotArguments Condition="'$(TargetArchitecture)' == 'arm64'" Include="mtriple=arm64-ios" />`
which passes through to MonoAOTCompiler.cs task which passes through to
the `mono --aot=llvmopts=mtriple=arm64-ios` flag, which is processed by
`src/mono/mono/mini/aot-compiler.c` and eventually regurgitated as
`llc -mtriple=arm64-ios -march=aarch64`, which results in .o files which
lack the required annotations to be considered Catalyst compatible.

Thankfully, it seems that `llc` accepts `clang`'s `-target` triplet
values as valid for `-mtriple`, so passing through Catalyst specific
values for `mtriple` in `MonoAOTCompiler.props` results in .o files
which are correctly linked later by `clang` during the AppleAppBuilder
task.

It's slow though! 🙀

Fixes dotnet#57589
@mdh1418
Copy link
Member Author

mdh1418 commented Sep 13, 2021

Browser wasm release Browser_wasm_Windows lane seems to be flakey @lewing, has failed for separate reasons the past few runs.

@steveisok steveisok merged commit 3e3fd87 into dotnet:release/6.0 Sep 13, 2021
@mdh1418 mdh1418 deleted the pr-57208-to-release/6.0 branch September 13, 2021 16:05
@ghost ghost locked as resolved and limited conversation to collaborators Nov 3, 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.

5 participants