Complete refactoring of GpxImport backend code

Refactored to use Flows and moving tests that need XmlPullParser to androidTest to get rid of robolectric
streetcomplete · Jul 5, 2024 · 12b4787 · 12b4787
1 parent 86d3703
commit 12b4787
Show file tree

Hide file tree

Showing 15 changed files with 541 additions and 453 deletions.
diff --git a/app/build.gradle.kts b/app/build.gradle.kts
@@ -45,6 +45,7 @@ android {
             isShrinkResources = false
             // don't use proguard-android-optimize.txt, it is too aggressive, it is more trouble than it is worth
             proguardFiles(getDefaultProguardFile("proguard-android.txt"), "proguard-rules.pro")
+            testProguardFile("test-proguard-rules.pro")
         }
         getByName("release") {
             signingConfig = signingConfigs.getByName("release")
@@ -116,12 +117,13 @@ dependencies {
     testImplementation("org.mockito:mockito-inline:$mockitoVersion")
     testImplementation("org.assertj:assertj-core:3.23.1")
     testImplementation(kotlin("test"))
-    testImplementation("org.robolectric:robolectric:4.12.2")
+    testImplementation("org.jetbrains.kotlinx:kotlinx-coroutines-test:1.7.1")
 
     androidTestImplementation("androidx.test:runner:1.5.2")
     androidTestImplementation("androidx.test:rules:1.5.0")
     androidTestImplementation("org.mockito:mockito-android:$mockitoVersion")
     androidTestImplementation("org.assertj:assertj-core:3.23.1")
+    androidTestImplementation("org.jetbrains.kotlinx:kotlinx-coroutines-test:1.7.1")
     androidTestImplementation(kotlin("test"))
 
     // dependency injection

diff --git a/...omplete/data/import/GpxImportParseTest.kt → ...omplete/data/import/GpxImportParseTest.kt b/...omplete/data/import/GpxImportParseTest.kt → ...omplete/data/import/GpxImportParseTest.kt
@@ -1,16 +1,15 @@
 package de.westnordost.streetcomplete.data.import
 
 import de.westnordost.streetcomplete.data.osm.mapdata.LatLon
-import org.junit.runner.RunWith
-import org.robolectric.RobolectricTestRunner
+import kotlinx.coroutines.flow.toList
+import kotlinx.coroutines.runBlocking
 import kotlin.test.Test
 import kotlin.test.assertEquals
 import kotlin.test.assertFails
 
-@RunWith(RobolectricTestRunner::class)
 class GpxImportParseTest {
     @Test
-    fun `successfully parses minimal sample track`() {
+    fun successfullyParsesMinimalSampleTrack() = runBlocking {
         val originalTrackPoints = arrayListOf(
             TrackPoint("22.22", "172.3"),
             TrackPoint("39.11111", "-179.999"),
@@ -20,13 +19,17 @@ class GpxImportParseTest {
             TrackPoint("-72.0", "0.3"),
         )
 
-        val inputGpx = minimalGpxBuilder(originalTrackPoints)
+        val inputGpx =
+            minimalGpxBuilder(originalTrackPoints)
 
-        assertSuccess(originalTrackPoints, parseGpx(inputGpx))
+        assertSuccess(
+            originalTrackPoints,
+            parseGpx(inputGpx)
+        )
     }
 
     @Test
-    fun `concatenates multiple track segments`() {
+    fun concatenatesMultipleTrackSegments() = runBlocking {
         val trackPointsSegment1 = arrayListOf(
             TrackPoint("-56.0", "0.0"),
             TrackPoint("57.57", "172.3")
@@ -53,11 +56,14 @@ class GpxImportParseTest {
             append("</gpx>")
         }
 
-        assertSuccess(trackPointsSegment1 + trackPointsSegment2, parseGpx(inputGpx))
+        assertSuccess(
+            trackPointsSegment1 + trackPointsSegment2,
+            parseGpx(inputGpx)
+        )
     }
 
     @Test
-    fun `process multiple tracks and segments`() {
+    fun processesMultipleTracksAndSegments() = runBlocking {
         val trackPoints1 = arrayListOf(
             TrackPoint("-12.33", "0.0"),
             TrackPoint("74.1", "-122.34")
@@ -95,21 +101,32 @@ class GpxImportParseTest {
             append("</gpx>")
         }
 
-        assertSuccess(trackPoints1 + trackPoints2 + trackPoints3, parseGpx(inputGpx))
+        assertSuccess(
+            trackPoints1 + trackPoints2 + trackPoints3,
+            parseGpx(inputGpx)
+        )
     }
 
     @Test
-    fun `throws on invalid trackPoints`() {
+    fun throwsOnInvalidTrackPoints(): Unit = runBlocking {
         assertFails {
-            parseGpx(minimalGpxBuilder(listOf(TrackPoint("99.0", "-12.1"))))
+            parseGpx(
+                minimalGpxBuilder(
+                    listOf(TrackPoint("99.0", "-12.1"))
+                )
+            )
         }
         assertFails {
-            parseGpx(minimalGpxBuilder(listOf(TrackPoint("-11.5", "-181.0"))))
+            parseGpx(
+                minimalGpxBuilder(
+                    listOf(TrackPoint("-11.5", "-181.0"))
+                )
+            )
         }
     }
 
     @Test
-    fun `throws on non-gpx files`() {
+    fun throwsOnNonGpxFiles(): Unit = runBlocking {
         val nonGpxXml = """
             <xml>
             </xml>
@@ -120,7 +137,7 @@ class GpxImportParseTest {
     }
 
     @Test
-    fun `Exhausting outer before inner sequence yields no elements`() {
+    fun exhaustingOuterBeforeInnerFlowYieldsNoElements() = runBlocking {
         val inputGpx = minimalGpxBuilder(
             arrayListOf(
                 TrackPoint("-39.654", "180"),
@@ -129,28 +146,28 @@ class GpxImportParseTest {
         )
 
         // exhausting outer first
-        val incorrectlyRetrievedSegments = parseGpxFile(inputGpx.byteInputStream()).toList();
+        val incorrectlyRetrievedSegments = parseGpxFile(inputGpx.byteInputStream()).toList()
         assertEquals(
             1, incorrectlyRetrievedSegments.size,
             "Exhausting outer first fails to retrieve the track segment"
         )
         assertEquals(
-            0, incorrectlyRetrievedSegments.first().count(),
+            emptyList(), incorrectlyRetrievedSegments.first().toList(),
             "Exhausting outer first unexpectedly yields track points"
         )
 
         // exhausting inner first
-        val correctlyRetrievedSegments =
-            parseGpxFile(inputGpx.byteInputStream()).flatMap { it.toList() }
+        val correctlyRetrievedSegments = parseGpx(inputGpx)
         assertEquals(
-            2, correctlyRetrievedSegments.count(),
+            2, correctlyRetrievedSegments.size,
             "Exhausting inner first fails to retrieve track points"
         )
     }
 
     @Test
-    fun `handles additional data gracefully`() {
-        val originalTrackPoints = arrayListOf(TrackPoint("88", "-19"))
+    fun handlesAdditionalDataGracefully() = runBlocking {
+        val originalTrackPoints =
+            arrayListOf(TrackPoint("88", "-19"))
 
         val inputGpx = buildString {
             append("<gpx version='1.1' xmlns='http://www.topografix.com/GPX/1/1'>")
@@ -167,28 +184,31 @@ class GpxImportParseTest {
             append("</gpx>")
         }
 
-        assertSuccess(originalTrackPoints, parseGpx(inputGpx))
+        assertSuccess(
+            originalTrackPoints,
+            parseGpx(inputGpx)
+        )
     }
-}
 
-private fun assertSuccess(
-    originalTrackPoints: List<TrackPoint>,
-    parseResult: List<LatLon>,
-) {
-    assertEquals(
-        originalTrackPoints.size, parseResult.size,
-        "Not all trackPoints are retrieved"
-    )
-    originalTrackPoints.map{it.toLatLon()}.zip(parseResult).forEach { pair ->
-        assertEquals(
-            expected = pair.component1().latitude,
-            actual = pair.component2().latitude,
-            "Latitudes don't match"
-        )
+    private fun assertSuccess(
+        originalTrackPoints: List<TrackPoint>,
+        parseResult: List<LatLon>,
+    ) {
         assertEquals(
-            expected = pair.component1().longitude,
-            actual = pair.component2().longitude,
-            "Longitudes don't match"
+            originalTrackPoints.size, parseResult.size,
+            "Not all trackPoints are retrieved"
         )
+        originalTrackPoints.map { it.toLatLon() }.zip(parseResult).forEach { pair ->
+            assertEquals(
+                expected = pair.component1().latitude,
+                actual = pair.component2().latitude,
+                "Latitudes don't match"
+            )
+            assertEquals(
+                expected = pair.component1().longitude,
+                actual = pair.component2().longitude,
+                "Longitudes don't match"
+            )
+        }
     }
 }