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

test: fix tests for lower API levels #145

Merged
merged 9 commits into from
Jan 12, 2025

Conversation

matthiasemde
Copy link
Owner

@matthiasemde matthiasemde commented Dec 15, 2024

Story

This PR fixes issues for instrumented tests with lower API versions or disables them if no fix is available.

Disabling test

For disabling tests, we use the @SdkSupress decorator.

Fixing test

Fixing test is a bit more difficult since many of the errors in the CI did not occur locally.
In an attempt to reproduce the CI environment on my machine and fix the tests, I migrated our test execution to gradle managed devices (god), introduced the AndroidX Test Orchestrator and built a screenshot on test failure feature which helps identify.

Gradle managed devices (gmd)

Using gradle managed devices, we can configure emulators inside our build.gradle.kts file which can then be used for testing. Both in the CI as well as locally. This improves reproducibility of CI test failures.

managedDevices {
  localDevices {
      create("api29") {
          device = "Pixel 6a"
          apiLevel = 29
          systemImageSource = "aosp"
      }
      ...
      create("api35") {
          device = "Pixel 6a"
          apiLevel = 35
          systemImageSource = "aosp-atd"
      }
  }
}

If you want to use these devices for testing, you can use the automatically generated gradle tasks:
$ ./gradlew app:api29DebugAndroidTest

If you want to run only a specific test(-class) you can parameterize the call like this:
$ ./gradlew app:api29DebugAndroidTest -Pandroid.testInstrumentationRunnerArguments.class=app.musikus.library.presentation.LibraryScreenTest#addFolderOrItem_hintDisappears

AndroidX Test Orchestrator

The Android Test Orchestrator improves test stability by running each test in an independent instance of the application.

image

Screenshot on test failure

Findings and learnings

While fixing the insertItems_checkSorting() test in LibraryFolderDetailsScreenTest I stumbled upon a nasty race condition involving FakeTimeProvider.advanceTimeBy(). When you i.e. create an object in the UI using the ComposeRule and advance the time right afterwards, the timestamp stored in the object can be either the old or the new timestamp depending on how fast the asynchronous execution of the object creation is running. This can result in multiple objects getting the same timestamp even though there is a call to FakeTimeProvider.advanceTimeBy() in-between them.

Helpful links

@matthiasemde matthiasemde requested a review from mipro98 December 15, 2024 16:43
@matthiasemde
Copy link
Owner Author

@matthiasemde matthiasemde force-pushed the test/fix-tests-for-lower-api-levels branch from 4f245ef to 4f95624 Compare December 30, 2024 14:57
@matthiasemde matthiasemde added skip-ci Skip the CI build in a PR and removed skip-ci Skip the CI build in a PR labels Dec 30, 2024
@matthiasemde matthiasemde force-pushed the test/fix-tests-for-lower-api-levels branch from 77b8509 to 975fda4 Compare January 1, 2025 18:20
@matthiasemde matthiasemde force-pushed the test/fix-tests-for-lower-api-levels branch from d94e3fa to 266403b Compare January 3, 2025 20:17
@matthiasemde matthiasemde force-pushed the test/fix-tests-for-lower-api-levels branch from 266403b to 2e8fe6c Compare January 3, 2025 20:52
@matthiasemde matthiasemde force-pushed the test/fix-tests-for-lower-api-levels branch 5 times, most recently from 8276a23 to 0c0d714 Compare January 6, 2025 07:52
mipro98
mipro98 previously approved these changes Jan 11, 2025
@matthiasemde matthiasemde force-pushed the test/fix-tests-for-lower-api-levels branch from 0d2325f to d4651e0 Compare January 12, 2025 13:47
Base automatically changed from test/active-session to main January 12, 2025 15:43
@matthiasemde matthiasemde dismissed mipro98’s stale review January 12, 2025 15:43

The base branch was changed.

@matthiasemde matthiasemde force-pushed the test/fix-tests-for-lower-api-levels branch from 73e9543 to 0db85e4 Compare January 12, 2025 16:13
@matthiasemde matthiasemde merged commit 0c094ea into main Jan 12, 2025
10 checks passed
@matthiasemde matthiasemde deleted the test/fix-tests-for-lower-api-levels branch January 12, 2025 17:50
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.

2 participants