Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Unit Tests - Android + Kotlin#729

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
mvarendorff wants to merge9 commits intoappwrite:master
base:master
Choose a base branch
Loading
frommvarendorff:feat-680-add-unit-tests

Conversation

mvarendorff
Copy link
Contributor

@mvarendorffmvarendorff commentedOct 22, 2023
edited
Loading

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-11not higher!) and./gradlew test for the Kotlin SDK)

Related PRs and Issues

Have you read theContributing Guidelines on issues?

Yup


Discord username for swag as requested by Tessa: yestheory

@mvarendorffmvarendorffforce-pushed thefeat-680-add-unit-tests branch 3 times, most recently from17e9c9e to2ddb9cfCompareOctober 28, 2023 11:31
@mvarendorffmvarendorff changed the titleUnit TestsUnit Tests - KotlinOct 31, 2023
@mvarendorffmvarendorff marked this pull request as ready for reviewOctober 31, 2023 10:35
@mvarendorffmvarendorff changed the titleUnit Tests - KotlinUnit Tests - Android + KotlinOct 31, 2023
@stnguyen90stnguyen90 self-requested a reviewNovember 1, 2023 01:03
@loks0n
Copy link
Member

Awesome PR 😃

mvarendorff reacted with heart emoji

@mvarendorffmvarendorff marked this pull request as draftNovember 4, 2023 19:48
@mvarendorff
Copy link
ContributorAuthor

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
ContributorAuthor

mvarendorff commentedDec 22, 2023
edited
Loading

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, seehttps://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 at6b86cbfshould 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.

@mvarendorffmvarendorff marked this pull request as ready for reviewDecember 22, 2023 15:20
@loks0n
Copy link
Member

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
ContributorAuthor

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 :)

@stnguyen90stnguyen90 marked this pull request as draftDecember 28, 2023 00:46
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@stnguyen90stnguyen90Awaiting requested review from stnguyen90

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

3 participants
@mvarendorff@loks0n@Haimantika

[8]ページ先頭

©2009-2025 Movatter.jp