Skip to content

[android][test] Fix several tests on the Android CI #59279

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

Merged
merged 1 commit into from
Jun 9, 2022

Conversation

finagolfin
Copy link
Member

@finagolfin finagolfin commented Jun 6, 2022

Also, these five tests were all disabled on linux before #58975 but pass on the Android CI now, so I left them enabled for Android. Maybe they should be enabled for linux-gnu too:

AutoDiff/compiler_crashers_fixed/rdar71191415-nested-differentiation-of-extension-method-optimized
SILOptimizer/addr_escape_info
SILOptimizer/escape_info
SILOptimizer/ranges
Sema/type_checker_perf/fast/rdar54580427

Another 5-10 tests are now enabled natively on Android, I will kick off a native build and test run of the toolchain with the next trunk snapshot tag and update this pull to disable them again if any fail.

Once this pull gets these tests fixed, all that remains is the 11 C++ Interop foreign reference tests that have been failing on the Android CI since they were added late last year. I can simply disable them all for Android, or if @zoecarver has any suggestions, we could try to fix them.

@bnbarham, you modified most of these tests, let me know what you think.

@compnerd and @drodriguez, trying to get the Android CI passing again, feedback welcome.

@@ -1,7 +1,6 @@
// REQUIRES: OS=macosx
// REQUIRES: CPU=x86_64
// FIXME: rdar://problem/19648117 Needs splitting objc parts out
// XFAIL: OS=linux-gnu, OS=freebsd
Copy link
Member Author

Choose a reason for hiding this comment

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

Not needed because of the REQUIRES: OS=macosx line above?

@@ -7,8 +7,6 @@
// REQUIRES: concurrency
// REQUIRES: objc_interop
// REQUIRES: tsan_runtime
// UNSUPPORTED: OS=linux-gnu
// UNSUPPORTED: OS=windows-msvc
Copy link
Member Author

Choose a reason for hiding this comment

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

Since this test requires objc_interop, which is only available on Apple platforms, these lines are not needed.

@@ -1,7 +1,7 @@
// RUN: %target-typecheck-verify-swift

// FIXME: No simd module on linux rdar://problem/20795411
// XFAIL: OS=linux-gnu, OS=windows-msvc, OS=openbsd
// XFAIL: OS=linux-gnu, OS=windows-msvc, OS=openbsd, OS=linux-android, OS=linux-androideabi
Copy link
Member Author

Choose a reason for hiding this comment

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

Strangely, this test passes when the toolchain is built and run natively on Android but not when cross-compiling, not sure what's going on.

Copy link
Member Author

Choose a reason for hiding this comment

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

And now it fails natively too. 🤷

@bnbarham
Copy link
Contributor

bnbarham commented Jun 6, 2022

@bnbarham, you modified most of these tests, let me know what you think.

Those updates LGTM. Thanks and sorry about the churn - I was just trying to get the tests to consistently use the OS= version rather than a substring of the triple.

Copy link
Contributor

@drodriguez drodriguez left a comment

Choose a reason for hiding this comment

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

@swift-ci please smoke test

- swiftlang#58975 switched many tests from XFAIL on linux to linux-gnu, so seven
  fail on the Android CI and two natively. They are now explicitly excluded.
- swiftlang#39605 added several C++ Interop tests, 11 of which fail on the Android CI,
  so disable them for now.
- swiftlang#42478 removed the @NoEscape attribute for the non-Android
  SIL/clang-function-types tests, so I remove it for Android too.
- My pull swiftlang#40779 moved the Swift pointer tags to the second byte, so
  SILOptimizer/concat_string_literals.64 will need to be updated for that,
  disabled it for now.
- Compiler-rt moved the directory in which it places those libraries for
  Android, llvm/llvm-project@a68ccba, so lit.cfg is updated for that.
@finagolfin
Copy link
Member Author

Finally ran the compiler validation suite natively on the latest Jun. 7 trunk snapshot and updated this pull with the results. I simply disabled the 11 C++ foreign reference Interop tests that are failing on the Android CI, though I left alone the additional four Interop tests that fail natively.

That means this pull should get the Android CI green again, ready for review, @drodriguez.

Running the testsuite natively on Android AArch64 also turned up the following:

  • SILOptimizer/{addr_escape_info,escape_info,ranges} all fail natively but pass on the Android CI, so left them enabled.
  • Prototypes/CollectionTransformers and stdlib/SIMDParameterPassing failed natively, so disabled them for linux-android only in this pull, since nobody runs the armv7 linux-androideabi tests natively.
  • SILOptimizer/large_string_array passes if the right CPU=aarch64 is listed, so I added that to this pull.
  • Concurrency/Runtime/async_taskgroup_is_asyncsequence, Interpreter/shebang-direct, and execution/dsohandle-multi-module pass natively after [Tests] Make OS features consistent #58975 happened to enable them for Android, so left them enabled.
  • Concurrency/Runtime/async_taskgroup_asynciterator_semantics and Concurrency/Runtime/executor_deinit1 would pass natively but they're disabled because of separate rdar issues, so left them disabled because of those issues alone.
  • Sanitizers/tsan/actor_counters is disabled for a rdar issue but fails natively anyway because tsan doesn't work on Android, so left it disabled.
  • Sema/type_checker_perf/fast/rdar23620262 is disabled for a rdar issue and fails natively with the type checker timing out, so left it disabled.

@drodriguez
Copy link
Contributor

@swift-ci please smoke test and merge

@swift-ci swift-ci merged commit 48ee0a6 into swiftlang:main Jun 9, 2022
@finagolfin finagolfin deleted the droid branch June 9, 2022 18:56
@drodriguez
Copy link
Contributor

Congrats. The AArch64 test seems to be green: https://ci-external.swift.org/job/oss-swift-RA-linux-ubuntu-16.04-android-arm64/902/ . Sadly there's a static_assert for the ARMv7 build: https://ci-external.swift.org/job/oss-swift-RA-linux-ubuntu-16.04-android/901/ (is i386 a thing in Android anymore? it might fail in the same way)

@finagolfin
Copy link
Member Author

Yeah, I saw that the armv7 build of the stdlib started failing yesterday morning, right before you merged this pull. Nobody has ever tried Swift on Android i386 AFAIK, which makes sense as there was never really any such hardware.

@finagolfin
Copy link
Member Author

Maybe related to #59287?

@al45tair
Copy link
Contributor

The ARMv7 thing is my fault. I hadn't been thinking about 32-bit Linux when I wrote the new swift::once() implementation. Hopefully #59371 will fix that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants