-
Notifications
You must be signed in to change notification settings - Fork 149
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
Fix and enable Android tests #557
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Older versions of Gradle toolchain are falling out of support and stop working. Update Gradle binaries to 4.6 since current 4.4.1 starts screaming about being unsupported. Current minimum Android API requires at least Gradle 4.6. New Gradle requires Google and JCenter repositories to be available during compilation because AAPT2 tool has been moved to Google's repo. Add these repositories to the search path. However, we're still using Gradle 3.2.1 for the actual build. Just make sure we're using the same version for both Android Themis and BoringSSL. Version mismatches can cause build failures (at least now they do). That said, as of now Google requires all submissions to Google Play store to target Android API 28. Bump the version of build tools, SDK used for compilation, and target SDK to 28. This means that we can use all the new features, but we can still maintain compatibility with earlier SDKs as necessary. Our minimum supported version is actually Android API 21 (not 16) because that's required by BoringSSL build. Finally, Android SDK 24 deprecates the old instrumentation testing harness and introduces AndroidX test runner. Add those libraries and appropriate repositories to the build.
New Android instrumentation testing framework deprecates AndroidTestCase in favor of more common JUnit4 framework. Convert the tests to use that. It's a quick-and-dirty conversion to just make it run. We'll improve the quality of tests later. As a side effect, this allows us to reuse those tests as unit tests, not only for Android, but for desktop Java as well.
Calling superclass in SecureSocket constructor leads to unexpected call to overloaded connect() implementation in SecureSocket. It crashes with the following stack trace: Caused by: java.lang.NullPointerException: Attempt to invoke virtual method 'byte[] com.cossacklabs.themis.AsymmetricKey.toByteArray()' on a null object reference at com.cossacklabs.themis.SecureSession.<init>(SecureSession.java:113) at com.cossacklabs.themis.SecureSocket.runClientProtocol(SecureSocket.java:70) at com.cossacklabs.themis.SecureSocket.connect(SecureSocket.java:168) at com.cossacklabs.themis.SecureSocket.connect(SecureSocket.java:173) at java.net.Socket.<init>(Socket.java:427) at java.net.Socket.<init>(Socket.java:243) at com.cossacklabs.themis.SecureSocket.<init>(SecureSocket.java:54) at com.cossacklabs.themis.test.SecureSocketTest$2.run(SecureSocketTest.java:176) because "signPrivateKey" is not initialized yet when connect() calls runClientProtocol(). The correct approach is to use the simple Socket constructor, then initialize all the fields, and finally call the connect() method ourselves. (No idea how this slipped though the cracks. I have reproduced this issue right when I managed to get integration tests running. It looks as if Android tests either were not running at all, or their failures have been ignored.)
Use a new Docker image for running tests. It contains updated Android SDK and NDK, platform tools, system images, etc. Remove the concurrency hacks since they seem to be no longer necessary on CirceCI. We can avoid tweaking the build files and explicitly specifying "--no-parallel --max-workers=2" in Gradle invocations. Update the system image used for testing. Unfortunately, we still need to use ARM emulation because x86 and x86_64 requires KVM which is not available on CircleCI runners. More unfortunately, we need at least Android API 24 in order to run new tests, and Android 24 is the last version that has ARM images. It looks like we'll get into a pinch when Google eventually decides to deprecate and remove the image. Tweak the command line for running the emulator. Previously we had disabled GPU, but that seems to break the current emulator and it gets stuck in the boot process without going anywhere. Therefore, run with GPU enabled. (We still don't need and cannot open a window though.) All in all, this allows us to finally reenable Android build job!
Lagovas
approved these changes
Nov 13, 2019
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.
looks great
vixentael
approved these changes
Nov 13, 2019
This was referenced May 1, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
infrastructure
Automated building and packaging
O-Android 🤖
Operating system: Android
W-JavaThemis ☕
Wrapper: Java, Java and Kotlin API
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Rejoice, Android, because your name is written in config.yml!
Ahem... I mean, let’s bring Android build on CircleCI back to life and deliver on that promise that I made in July. And we’re not moving to Bitrise.
There are quite a few changes in here. More details can be found in commit messages. A short summary is:
SecureSocket
implementation that was breaking the tests* I have absolutely no idea how it survived given that the tests were working in the past. I strongly suspect now that they were not actually running, or the failure was ignored.
Checklist
Public API has proper documentation(no changes)Example projects and code samples are updated(not affected)