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

Unit Tests - Android + Kotlin #729

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

mvarendorff
Copy link
Contributor

@mvarendorff mvarendorff commented Oct 22, 2023

What does this PR do?

This PR adds generated unit tests to Android and Kotlin SDKs. It also contains some refactoring to generate similar classes from the same template to both the Android and Kotlin SDK, similar to how Dart and Flutter share templates.

Test Plan

I set up the generated project locally and ran the respective test command (./gradlew library:test for the Android SDK (requires Java 8-11 not higher!) and ./gradlew test for the Kotlin SDK)

Related PRs and Issues

Have you read the Contributing Guidelines on issues?

Yup


Discord username for swag as requested by Tessa: yestheory

@mvarendorff mvarendorff force-pushed the feat-680-add-unit-tests branch 3 times, most recently from 17e9c9e to 2ddb9cf Compare October 28, 2023 11:31
@mvarendorff mvarendorff changed the title Unit Tests Unit Tests - Kotlin Oct 31, 2023
@mvarendorff mvarendorff marked this pull request as ready for review October 31, 2023 10:35
@mvarendorff mvarendorff changed the title Unit Tests - Kotlin Unit Tests - Android + Kotlin Oct 31, 2023
@stnguyen90 stnguyen90 self-requested a review November 1, 2023 01:03
@loks0n
Copy link
Member

loks0n commented Nov 1, 2023

Awesome PR 😃

@mvarendorff mvarendorff marked this pull request as draft November 4, 2023 19:48
@mvarendorff
Copy link
Contributor Author

Converting this to a draft for the time being until I figure out what gradle version still works with Java 8 to get the pipeline green. Not sure when I will get to it since I am uncertain how long setting up the build locally for proper testing will take.

@mvarendorff
Copy link
Contributor Author

mvarendorff commented Dec 22, 2023

After lots of battling, I found 2 culprits that were causing issues with tests:

  1. The latest Android Gradle build plugin supporting Java 8 is 4.2.2; anything beyond that needs at least Java 11. For some reason tests fail for me locally on Java 17+ but run fine with Java 11, see https://stackoverflow.com/a/76170361/6707985. I downgraded back to 4.2.2 to get the pipeline to work and added a note to the original PR post noting the restriction to Java 11.

  2. Robolectric runs a compatibility check on startup; for Android 29+ this will fail with the error that creating a Robolectric Sandbox for the respective Android API version requires at least Java 9. I removed Robolectric and mocked out the parts of the SDK that were needed using different snippets I found around the internet. When Java 8 is dropped longterm, the implementation using robolectric that was present at 6b86cbf should work properly and be a bit more sensible. I have no idea what part of this PR caused issues with the existing robolectric test, to be honest...


I have no idea why node is failing now; this is cruel but I am relatively confident that's not my fault because I didn't touch node.

@mvarendorff mvarendorff marked this pull request as ready for review December 22, 2023 15:20
@loks0n
Copy link
Member

loks0n commented Dec 24, 2023

I have no idea why node is failing now; this is cruel but I am relatively confident that's not my fault because I didn't touch node.

This is fixed on the 1.5.x branch now, you may rebase to this if you want to see everything pass

@mvarendorff
Copy link
Contributor Author

Ah, I see! I will hold off on that I until that's merged I think to avoid more chaos than necessary. As long as I know it was not my fault indeed and it's fixed somewhere else I got peace of mind! Thanks :)

@stnguyen90 stnguyen90 marked this pull request as draft December 28, 2023 00:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants