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 5 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)?

@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

@FloEdelmann FloEdelmann force-pushed the actions-instrumentation-tests branch from 7f2d063 to 1ce2ff6 Compare February 1, 2025 16:26
@FloEdelmann FloEdelmann force-pushed the actions-instrumentation-tests branch from 1ce2ff6 to 559982b Compare February 1, 2025 16:28
@FloEdelmann
Copy link
Member Author

@kmpoppe It's the tests from app/src/androidTest, not the ones from app/src/test (those already run in the existing test workflow).

Tests in androidTests are actually only a few database tests.

Yes, most of them are, but there is also the CountryInfosTest and the NameAndLocationLabelTest, both of which test quite user-facing features. It would of course be good to migrate them to unit tests instead, but that might be hard because they use the Android resource system.

I expect that whenever I change something there, I will also run these tests at least once

As noted in the PR description, while getting the tests to run I found a bug that was only occurring on API level 34 and below (see 6bbf0fb and 96f2758). That leads me to say that it still makes sense to automate the Android instrumentation tests, so that they are run in different API versions and also changes that seem unrelated are checked.

I would like to get rid of (most of) these tests by moving these to the normal tests. […] Then, all these database tests could run in Java without issue, without needing the Android instrumentation.

Agreed! But until then (or for the other tests), I guess this PR still improves the situation.

running Android instrumentation tests on the lowest and highest supported Android version sounds reasonable

Done that in 559982b. These are the resulting job runs:

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"

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