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

Run Android instrumentation tests on GitHub Actions #6083

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

FloEdelmann
Copy link
Member

@FloEdelmann FloEdelmann commented Jan 5, 2025

Just like the unit tests are run for every push onto the master branch (see https://github.com/streetcomplete/StreetComplete/actions/workflows/test.yml), this PR adds a new GitHub Actions workflow that runs the Android instrumentation tests using https://github.com/ReactiveCircus/android-emulator-runner.

Example workflow run: https://github.com/streetcomplete/StreetComplete/actions/runs/12621672783/job/35168756488

To be discussed:

  1. These tests are quite resource-heavy, so running them for every push might not be justified. Should I change it to a cron job that runs once every day or every week, maybe only if there are changes to the last run? That might need some more investigation though. Actually, I think this is not worth the effort; the Android instrumentation tests take around the same time as the unit tests, and pushes to master are not that often, so it should be fine.
  2. Currently, they run for API level 34 (Android 14). The app's target level was recently bumped to 35 (see #6074), however while getting the tests to run I found a bug that was only occurring on API level 34 and below (see 6bbf0fb and 96f2758). So should we maybe run the tests on the lowest and highest supported API level (using matrix strategies)?

push:
branches:
- "master"
- "actions-instrumentation-tests"
Copy link
Member Author

Choose a reason for hiding this comment

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

Should be removed before merging:

Suggested change
- "actions-instrumentation-tests"

@kmpoppe
Copy link
Collaborator

kmpoppe commented Jan 8, 2025

Q: I seem to be lacking some information, which tests exactly should run here? That aren't the ~2,200 tests from app/src/test/java/de/westnordost/streetcomplete/ but app/srv/AndroidTest/... right?

@westnordost
Copy link
Member

westnordost commented Jan 8, 2025

Tests in androidTests are actually only a few database tests. They are of course important, but changes on the DAO that access the database are rare and I expect that whenever I change something there, I will also run these tests at least once.

I would like to get rid of (most of) these tests by moving these to the normal tests. Currently, the tests use the Android API to access the SQLite database. To have these tests work independent of Android, we'd need to use some SQLite Java (or multiplatform??) library just in the tests. In the unit tests, we'd need to implement the Database interface, using that SQLite Java library. Then, all these database tests could run in Java without issue, without needing the Android instrumentation.

The Android instrumentation tests may still be necessary for testing a few things, for example if the AndroidDatabase is implemented correctly, or for testing any other pure-Android platform code that may remain.

In general, running Android instrumentation tests on the lowest and highest supported Android version sounds reasonable.

@westnordost
Copy link
Member

Hmm, this sounds good: https://developer.android.com/kotlin/multiplatform/sqlite

Would even solve #5417

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.

3 participants