-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[android][test] Fix two C++ Interop tests that were split off for Android and enable two more new ones #61873
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
Conversation
// REQUIRES: CODEGENERATOR=X86 | ||
// REQUIRES: CODEGENERATOR=ARM | ||
|
||
// REQUIRES: CPU=x86_64 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This simply follows suit with #61801.
@@ -133,7 +133,7 @@ $ adb push build/Ninja-ReleaseAssert/swift-linux-x86_64/lib/swift/android/libBlo | |||
In addition, you'll also need to copy the Android NDK's libc++: | |||
|
|||
``` | |||
$ adb push /path/to/android-ndk-r25b/sources/cxx-stl/llvm-libc++/libs/arm64-v8a/libc++_shared.so /data/local/tmp | |||
$ adb push /path/to/android-ndk-r25b/toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/lib/aarch64-linux-android/libc++_shared.so /data/local/tmp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previous NDKs put this library in both these directories, but the recent NDK 25 update removed the first path, which I missed in #60938.
…roid, enable two more new ones, and set an executable_test Also, make some related changes, like updating a path in the Android doc, making sure the `unknown` vendor is always used, and using `CPU` instead of `CODEGENERATOR`.
@@ -1,5 +1,7 @@ | |||
// RUN: %target-run-simple-swift(-enable-experimental-feature SwiftParser -enable-experimental-feature ParserASTGen) | |||
|
|||
// REQUIRES: executable_test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most target-run-simple-swift
tests are marked executable_test
because they must be run as native executables. That's why this new test fails on the community Android CI, which only runs cross-compiled tests.
@swift-ci please test and merge |
@buttaface you should ask for test/commit access. It will make it easier to get these small changes thru faster. |
@drodriguez, I am in maintenance mode on the Android port at this point, so I think it's better I don't have a commit bit for the 5-10 Swift pulls I expect to submit in the coming year. Good news is all these tests are working now, bad news is the Android CI is still not green because of new Interop test failures. |
Also, make some related changes, like updating a path in the Android doc, making sure the
unknown
vendor is always used, and usingCPU
instead ofCODEGENERATOR
.I fixed the two constructor tests by restricting them to only be built on Android AArch64 instead, where I natively ran the tests, as the stdlib Cxx.swiftmodule has to be built for these two tests to run now. Since the Android CI only builds each Android ARM arch separately, these two tests will now only run on the AArch64 CI, otherwise you'll see errors about not being able to find the Cxx module, as can be seen with the second one on the Android armv7 CI now
(though that CI currently appears to be down).@egorzhdan, you split the constructor tests off in #60603 a couple weeks ago, please review.
@drodriguez, this should get the Android CI green again.
The two new tests were added in #61535 and assumed they wouldn't work on Android, but the CI reports that they do.