diff --git a/.github/workflows/android.yml b/.github/workflows/android.yml index 735c7e5..b2a7d65 100644 --- a/.github/workflows/android.yml +++ b/.github/workflows/android.yml @@ -27,3 +27,52 @@ jobs: run: ./gradlew :app:testDebugUnitTest - name: Build release run: ./gradlew :app:assembleRelease + + instrumented: + runs-on: ubuntu-latest + env: + API_LEVEL: 36 + TARGET: default + ARCH: x86_64 + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-java@v4 + with: + distribution: temurin + java-version: '17' + - uses: gradle/actions/setup-gradle@v4 + - name: Enable KVM + run: | + echo 'KERNEL=="kvm", GROUP="kvm", MODE="0666", OPTIONS+="static_node=kvm"' | sudo tee /etc/udev/rules.d/99-kvm4all.rules + sudo udevadm control --reload-rules + sudo udevadm trigger --name-match=kvm + - name: AVD cache + uses: actions/cache@v5 + id: avd-cache + with: + path: | + ~/.android/avd/* + ~/.android/adb* + key: avd-${{ env.API_LEVEL }}-${{ env.TARGET }}-${{ env.ARCH }} + - name: Create AVD and snapshot for caching + if: steps.avd-cache.outputs.cache-hit != 'true' + uses: reactivecircus/android-emulator-runner@v2 + with: + api-level: ${{ env.API_LEVEL }} + target: ${{ env.TARGET }} + arch: ${{ env.ARCH }} + force-avd-creation: false + emulator-options: -no-window -gpu swiftshader_indirect -noaudio -no-boot-anim -camera-back none + disable-animations: false + script: echo "Generated AVD snapshot for caching." + - name: Instrumented tests + uses: reactivecircus/android-emulator-runner@v2 + with: + api-level: ${{ env.API_LEVEL }} + target: ${{ env.TARGET }} + arch: ${{ env.ARCH }} + force-avd-creation: false + emulator-options: -no-snapshot-save -no-window -gpu swiftshader_indirect -noaudio -no-boot-anim -camera-back none + disable-animations: true + working-directory: android + script: ./gradlew :app:connectedDebugAndroidTest diff --git a/android/app/src/androidTest/kotlin/org/onebusaway/vehicletracker/data/KeystoreCryptorTest.kt b/android/app/src/androidTest/kotlin/org/onebusaway/vehicletracker/data/KeystoreCryptorTest.kt index 4c8d8db..94d1137 100644 --- a/android/app/src/androidTest/kotlin/org/onebusaway/vehicletracker/data/KeystoreCryptorTest.kt +++ b/android/app/src/androidTest/kotlin/org/onebusaway/vehicletracker/data/KeystoreCryptorTest.kt @@ -18,8 +18,7 @@ import java.io.File /** * Covers the one piece [SessionStoreTest] cannot reach on the JVM: the real Android Keystore - * binding. CI runs no instrumented tests (`.github/workflows/android.yml` runs `assembleDebug`, - * `testDebugUnitTest`, `assembleRelease` and starts no emulator), so run these with + * binding. CI runs these on an emulator in the `instrumented` job; locally, run * `./gradlew :app:connectedDebugAndroidTest` on a device or emulator. */ @RunWith(AndroidJUnit4::class) diff --git a/android/app/src/androidTest/kotlin/org/onebusaway/vehicletracker/ui/ScreenFlowTest.kt b/android/app/src/androidTest/kotlin/org/onebusaway/vehicletracker/ui/ScreenFlowTest.kt index 5a6ba22..1d07be1 100644 --- a/android/app/src/androidTest/kotlin/org/onebusaway/vehicletracker/ui/ScreenFlowTest.kt +++ b/android/app/src/androidTest/kotlin/org/onebusaway/vehicletracker/ui/ScreenFlowTest.kt @@ -1,10 +1,19 @@ package org.onebusaway.vehicletracker.ui +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.height +import androidx.compose.runtime.CompositionLocalProvider +import androidx.compose.ui.Modifier +import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.test.assertIsDisplayed +import androidx.compose.ui.test.assertIsNotDisplayed import androidx.compose.ui.test.assertIsNotEnabled import androidx.compose.ui.test.junit4.createComposeRule import androidx.compose.ui.test.onNodeWithText import androidx.compose.ui.test.performClick +import androidx.compose.ui.test.performScrollTo +import androidx.compose.ui.unit.Density +import androidx.compose.ui.unit.dp import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.platform.app.InstrumentationRegistry import org.junit.Assert.assertTrue @@ -121,6 +130,44 @@ class ScreenFlowTest { assertTrue(ended) } + @Test + fun resumeScreen_disablesBothActionsUntilTheTripLoads() { + compose.setContent { + ResumeShiftScreenContent( + state = ResumeShiftUiState(trip = null), + onResumeClick = {}, + onEndShiftClick = {}, + onEndShiftLocallyClick = {}, + onDismissError = {}, + ) + } + + compose.onNodeWithText(getString(R.string.resume_button)).assertIsNotEnabled() + compose.onNodeWithText(getString(R.string.resume_end_shift_button)).assertIsNotEnabled() + } + + @Test + fun resumeScreen_scrollsToEndShiftAtALargeFontScale() { + compose.setContent { + CompositionLocalProvider(LocalDensity provides Density(LocalDensity.current.density, fontScale = 2f)) { + // A short screen, so End Shift starts below the fold whatever the test device. + Box(Modifier.height(360.dp)) { + ResumeShiftScreenContent( + state = ResumeShiftUiState(trip = ActiveTrip(7L, "T1", "bus-1", "R1", "20260902", 100L)), + onResumeClick = {}, + onEndShiftClick = {}, + onEndShiftLocallyClick = {}, + onDismissError = {}, + ) + } + } + } + + val endShift = compose.onNodeWithText(getString(R.string.resume_end_shift_button)) + endShift.assertIsNotDisplayed() + endShift.performScrollTo().assertIsDisplayed() + } + // --- The adherence panel, against the server fixture's trip T1 --- private val resources get() = InstrumentationRegistry.getInstrumentation().targetContext.resources diff --git a/android/app/src/main/kotlin/org/onebusaway/vehicletracker/MainActivity.kt b/android/app/src/main/kotlin/org/onebusaway/vehicletracker/MainActivity.kt index 53e0161..6ef3c26 100644 --- a/android/app/src/main/kotlin/org/onebusaway/vehicletracker/MainActivity.kt +++ b/android/app/src/main/kotlin/org/onebusaway/vehicletracker/MainActivity.kt @@ -8,9 +8,14 @@ import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.safeDrawingPadding import androidx.compose.material3.Surface import androidx.compose.ui.Modifier +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.lifecycleScope +import androidx.lifecycle.repeatOnLifecycle import dagger.hilt.android.AndroidEntryPoint +import kotlinx.coroutines.launch import org.onebusaway.vehicletracker.data.TrackingRepository import org.onebusaway.vehicletracker.service.ServiceController +import org.onebusaway.vehicletracker.service.rearmWhenActive import org.onebusaway.vehicletracker.ui.AppNav import org.onebusaway.vehicletracker.ui.theme.AppTheme import javax.inject.Inject @@ -34,15 +39,15 @@ class MainActivity : ComponentActivity() { } } } - } - - override fun onResume() { - super.onResume() // Re-arms a service that is running without location updates: the degraded restart - // path (SecurityException in the service). A stored trip with no service running is not - // started here: that shift was interrupted, and the resume prompt asks the driver. - if (trackingRepository.state.value.active) { - serviceController.startTracking() + // path (SecurityException in the service). Watched while resumed rather than checked once, + // because a START_STICKY restart can turn active after the activity has resumed. A stored + // trip with no service running is not started here: that shift was interrupted, and the + // resume prompt asks the driver. + lifecycleScope.launch { + repeatOnLifecycle(Lifecycle.State.RESUMED) { + rearmWhenActive(trackingRepository.state, serviceController::startTracking) + } } } } diff --git a/android/app/src/main/kotlin/org/onebusaway/vehicletracker/data/Cryptor.kt b/android/app/src/main/kotlin/org/onebusaway/vehicletracker/data/Cryptor.kt index 99cabc3..355528e 100644 --- a/android/app/src/main/kotlin/org/onebusaway/vehicletracker/data/Cryptor.kt +++ b/android/app/src/main/kotlin/org/onebusaway/vehicletracker/data/Cryptor.kt @@ -19,8 +19,7 @@ import javax.crypto.spec.GCMParameterSpec * This is a seam rather than an abstraction for its own sake: [EncryptedSessionStore] holds all * of the session, migration and recovery logic and is exercised on the JVM against a fake, while * [KeystoreCryptor] — the only part that needs a real device — is covered by instrumented tests. - * CI runs `testDebugUnitTest` and no emulator, so the logic that has to gate a change sits on - * this side of the seam. + * The logic sits on this side of the seam so the JVM tests reach it without an emulator. */ interface Cryptor { /** @throws GeneralSecurityException if the key cannot be created or used. */ diff --git a/android/app/src/main/kotlin/org/onebusaway/vehicletracker/service/LocationTrackingService.kt b/android/app/src/main/kotlin/org/onebusaway/vehicletracker/service/LocationTrackingService.kt index 1dcbd53..e829e10 100644 --- a/android/app/src/main/kotlin/org/onebusaway/vehicletracker/service/LocationTrackingService.kt +++ b/android/app/src/main/kotlin/org/onebusaway/vehicletracker/service/LocationTrackingService.kt @@ -159,7 +159,7 @@ class LocationTrackingService : Service() { } catch (e: SecurityException) { // Background restart without ACCESS_BACKGROUND_LOCATION granted — the spec's degraded // path. Drop out of foreground state but keep the service (and its notification) alive; - // MainActivity.onResume calls ServiceController.startTracking() again on next foreground. + // MainActivity calls ServiceController.startTracking() again while it is in the foreground. Log.w(TAG, "Missing location permission for background restart; entering degraded mode", e) locationUpdatesActive = false ServiceCompat.stopForeground(this, ServiceCompat.STOP_FOREGROUND_DETACH) diff --git a/android/app/src/main/kotlin/org/onebusaway/vehicletracker/service/RearmWhenActive.kt b/android/app/src/main/kotlin/org/onebusaway/vehicletracker/service/RearmWhenActive.kt new file mode 100644 index 0000000..a104eff --- /dev/null +++ b/android/app/src/main/kotlin/org/onebusaway/vehicletracker/service/RearmWhenActive.kt @@ -0,0 +1,17 @@ +package org.onebusaway.vehicletracker.service + +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.filter +import kotlinx.coroutines.flow.map +import org.onebusaway.vehicletracker.data.TrackingState + +/** + * Calls [startTracking] when collection starts with tracking active, and again each time it turns + * active. Going inactive never calls it, so a trip the driver ended is not restarted. Calling it on + * a healthy service is harmless: [LocationTrackingService] only requests location updates when it + * has none running. + */ +internal suspend fun rearmWhenActive(state: Flow, startTracking: () -> Unit) { + state.map { it.active }.distinctUntilChanged().filter { it }.collect { startTracking() } +} diff --git a/android/app/src/main/kotlin/org/onebusaway/vehicletracker/ui/resume/ResumeShiftScreen.kt b/android/app/src/main/kotlin/org/onebusaway/vehicletracker/ui/resume/ResumeShiftScreen.kt index 25da615..48a9bf4 100644 --- a/android/app/src/main/kotlin/org/onebusaway/vehicletracker/ui/resume/ResumeShiftScreen.kt +++ b/android/app/src/main/kotlin/org/onebusaway/vehicletracker/ui/resume/ResumeShiftScreen.kt @@ -1,6 +1,6 @@ package org.onebusaway.vehicletracker.ui.resume -import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.fillMaxSize @@ -8,12 +8,15 @@ import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.heightIn import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.rememberScrollState +import androidx.compose.foundation.verticalScroll import androidx.compose.material3.Button import androidx.compose.material3.ButtonDefaults import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue +import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.res.pluralStringResource import androidx.compose.ui.res.stringResource @@ -48,47 +51,53 @@ fun ResumeShiftScreenContent( onEndShiftLocallyClick: () -> Unit, onDismissError: () -> Unit, ) { - Column( - modifier = Modifier.fillMaxSize().padding(24.dp), - verticalArrangement = Arrangement.Center, - ) { - Text( - text = stringResource(R.string.resume_title), - style = MaterialTheme.typography.headlineMedium, - ) - state.trip?.let { trip -> - Spacer(Modifier.height(16.dp)) + // Until the stored trip loads, the prompt shows no shift to decide on and End Shift has + // nothing to end. + val actionsEnabled = state.trip != null && !state.ending + // The Box centres the prompt: Arrangement.Center cannot, as verticalScroll measures the Column + // with unbounded height. It scrolls once a large font scale pushes the buttons off the screen. + Box(modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { + Column( + modifier = Modifier.fillMaxWidth().verticalScroll(rememberScrollState()).padding(24.dp), + ) { Text( - text = stringResource(R.string.resume_message, trip.vehicleId), - style = MaterialTheme.typography.bodyLarge, + text = stringResource(R.string.resume_title), + style = MaterialTheme.typography.headlineMedium, ) + state.trip?.let { trip -> + Spacer(Modifier.height(16.dp)) + Text( + text = stringResource(R.string.resume_message, trip.vehicleId), + style = MaterialTheme.typography.bodyLarge, + ) + Spacer(Modifier.height(16.dp)) + Text( + text = stringResource(R.string.resume_route, trip.routeId), + style = MaterialTheme.typography.titleLarge, + ) + Spacer(Modifier.height(8.dp)) + Text( + text = startedAgoText(state.startedAgoSec), + style = MaterialTheme.typography.bodyLarge, + ) + } + Spacer(Modifier.height(32.dp)) + Button( + onClick = onResumeClick, + enabled = actionsEnabled, + modifier = Modifier.fillMaxWidth().heightIn(min = 64.dp), + ) { + Text(stringResource(R.string.resume_button)) + } Spacer(Modifier.height(16.dp)) - Text( - text = stringResource(R.string.resume_route, trip.routeId), - style = MaterialTheme.typography.titleLarge, - ) - Spacer(Modifier.height(8.dp)) - Text( - text = startedAgoText(state.startedAgoSec), - style = MaterialTheme.typography.bodyLarge, - ) - } - Spacer(Modifier.height(32.dp)) - Button( - onClick = onResumeClick, - enabled = !state.ending, - modifier = Modifier.fillMaxWidth().heightIn(min = 64.dp), - ) { - Text(stringResource(R.string.resume_button)) - } - Spacer(Modifier.height(16.dp)) - Button( - onClick = onEndShiftClick, - enabled = !state.ending, - colors = ButtonDefaults.buttonColors(containerColor = StatusRed), - modifier = Modifier.fillMaxWidth().heightIn(min = 64.dp), - ) { - Text(stringResource(R.string.resume_end_shift_button)) + Button( + onClick = onEndShiftClick, + enabled = actionsEnabled, + colors = ButtonDefaults.buttonColors(containerColor = StatusRed), + modifier = Modifier.fillMaxWidth().heightIn(min = 64.dp), + ) { + Text(stringResource(R.string.resume_end_shift_button)) + } } } diff --git a/android/app/src/test/kotlin/org/onebusaway/vehicletracker/data/SessionStoreTest.kt b/android/app/src/test/kotlin/org/onebusaway/vehicletracker/data/SessionStoreTest.kt index 14b519a..192d1c6 100644 --- a/android/app/src/test/kotlin/org/onebusaway/vehicletracker/data/SessionStoreTest.kt +++ b/android/app/src/test/kotlin/org/onebusaway/vehicletracker/data/SessionStoreTest.kt @@ -33,8 +33,7 @@ import java.security.ProviderException /** * Exercises [EncryptedSessionStore] against a real file-backed DataStore and a [FakeCryptor], * covering the upgrade from the build that stored the driver's JWT in the clear. These run on the - * JVM, which is the only thing CI executes — see `KeystoreCryptorTest` for the parts that need a - * device. + * JVM; `KeystoreCryptorTest` covers the parts that need a device. */ class SessionStoreTest { @get:Rule val tempFolder = TemporaryFolder() diff --git a/android/app/src/test/kotlin/org/onebusaway/vehicletracker/data/VehiclePrefsStoreTest.kt b/android/app/src/test/kotlin/org/onebusaway/vehicletracker/data/VehiclePrefsStoreTest.kt index 55c4982..a29e16e 100644 --- a/android/app/src/test/kotlin/org/onebusaway/vehicletracker/data/VehiclePrefsStoreTest.kt +++ b/android/app/src/test/kotlin/org/onebusaway/vehicletracker/data/VehiclePrefsStoreTest.kt @@ -17,8 +17,8 @@ import java.io.File /** * Exercises [DataStoreVehiclePrefsStore] against a real file-backed DataStore. That is why the - * store takes a `DataStore` rather than a `Context` — Android CI runs - * `testDebugUnitTest` with no emulator, so a Context-bound store could not be covered here. + * store takes a `DataStore` rather than a `Context`: this runs on the JVM, where a + * Context-bound store could not be covered. */ class VehiclePrefsStoreTest { @get:Rule val tempFolder = TemporaryFolder() diff --git a/android/app/src/test/kotlin/org/onebusaway/vehicletracker/service/RearmWhenActiveTest.kt b/android/app/src/test/kotlin/org/onebusaway/vehicletracker/service/RearmWhenActiveTest.kt new file mode 100644 index 0000000..6067a5c --- /dev/null +++ b/android/app/src/test/kotlin/org/onebusaway/vehicletracker/service/RearmWhenActiveTest.kt @@ -0,0 +1,79 @@ +package org.onebusaway.vehicletracker.service + +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.launch +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.runCurrent +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Test +import org.onebusaway.vehicletracker.data.TrackingProblem +import org.onebusaway.vehicletracker.data.TrackingState + +class RearmWhenActiveTest { + private val state = MutableStateFlow(TrackingState()) + private var starts = 0 + + private fun TestScope.startCollecting() { + backgroundScope.launch { rearmWhenActive(state) { starts++ } } + runCurrent() + } + + /** Lets the collector see each value: a StateFlow keeps only the latest one. */ + private fun TestScope.emit(next: TrackingState) { + state.value = next + runCurrent() + } + + @Test fun `already active when collection starts re-arms once`() = runTest { + state.value = TrackingState(active = true) + + startCollecting() + + assertEquals(1, starts) + } + + @Test fun `turning active while resumed re-arms`() = runTest { + startCollecting() + assertEquals(0, starts) + + emit(TrackingState(active = true)) + + assertEquals(1, starts) + } + + @Test fun `turning inactive does not restart the trip the driver ended`() = runTest { + state.value = TrackingState(active = true) + startCollecting() + + emit(TrackingState()) + + assertEquals(1, starts) + } + + @Test fun `a problem changing while active does not re-arm again`() = runTest { + state.value = TrackingState(active = true) + startCollecting() + + emit(state.value.copy(problem = TrackingProblem.NO_NETWORK)) + emit(state.value.copy(problem = TrackingProblem.NONE, fixesSent = 1)) + + assertEquals(1, starts) + } + + @Test fun `inactive throughout never starts the service`() = runTest { + startCollecting() + + assertEquals(0, starts) + } + + @Test fun `a trip started after the last one ended re-arms again`() = runTest { + state.value = TrackingState(active = true) + startCollecting() + + emit(TrackingState()) + emit(TrackingState(active = true)) + + assertEquals(2, starts) + } +}