Merge changes I39698d4d,I8c8b67a9 into tm-qpr-dev

* changes:
  [Sb refactor] Move active data repo to connections repo
  [Sb refactor] Address code comments

packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarIconControllerImpl.java

@@ -163,8 +163,7 @@ public class StatusBarIconControllerImpl implements Tunable,
         for (int i = currentSlots.size() - 1; i >= 0; i--) {
             Slot s = currentSlots.get(i);
             slotsToReAdd.put(s, s.getHolderList());
-            // Don't force here because the new pipeline properly handles the tuner settings
+            removeAllIconsForSlot(s.getName(), /* fromNewPipeline */ false);
-            removeAllIconsForSlot(s.getName(), /* force */ false);
         }
 
         // Add them all back
@@ -286,7 +285,7 @@ public class StatusBarIconControllerImpl implements Tunable,
         // Because of the way we cache the icon holders, we need to remove everything any time
         // we get a new set of subscriptions. This might change in the future, but is required
         // to support demo mode for now
-        removeAllIconsForSlot(slotName, /* force */ true);
+        removeAllIconsForSlot(slotName, /* fromNewPipeline */ true);
 
         Collections.reverse(subIds);
 
@@ -453,13 +452,13 @@ public class StatusBarIconControllerImpl implements Tunable,
     /** */
     @Override
     public void removeAllIconsForSlot(String slotName) {
-        removeAllIconsForSlot(slotName, /* force */ false);
+        removeAllIconsForSlot(slotName, /* fromNewPipeline */ false);
     }
 
-    private void removeAllIconsForSlot(String slotName, Boolean force) {
+    private void removeAllIconsForSlot(String slotName, boolean fromNewPipeline) {
         // If the new pipeline is on for this icon, don't allow removal, since the new pipeline
         // will never call this method
-        if (!force && mStatusBarPipelineFlags.isIconControlledByFlags(slotName)) {
+        if (!fromNewPipeline && mStatusBarPipelineFlags.isIconControlledByFlags(slotName)) {
             Log.i(TAG, "Ignoring removal of (" + slotName + "). "
                     + "It should be controlled elsewhere");
             return;

packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/MobileConnectionsRepository.kt

@@ -34,8 +34,14 @@ interface MobileConnectionsRepository {
     /** Observable list of current mobile subscriptions */
     val subscriptions: StateFlow<List<SubscriptionModel>>
 
-    /** Observable for the subscriptionId of the current mobile data connection */
+    /**
-    val activeMobileDataSubscriptionId: StateFlow<Int>
+     * Observable for the subscriptionId of the current mobile data connection. Null if we don't
+     * have a valid subscription id
+     */
+    val activeMobileDataSubscriptionId: StateFlow<Int?>
+
+    /** Repo that tracks the current [activeMobileDataSubscriptionId] */
+    val activeMobileDataRepository: StateFlow<MobileConnectionRepository?>
 
     /**
      * Observable event for when the active data sim switches but the group stays the same. E.g.,

packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/MobileRepositorySwitcher.kt

@@ -47,7 +47,6 @@ import kotlinx.coroutines.flow.stateIn
  * interface in its own repository, completely separate from the real version, while still using all
  * of the prod implementations for the rest of the pipeline (interactors and onward). Looks
  * something like this:
- *
  * ```
  * RealRepository
  *                 │
@@ -115,7 +114,7 @@ constructor(
             .flatMapLatest { it.subscriptions }
             .stateIn(scope, SharingStarted.WhileSubscribed(), realRepository.subscriptions.value)
 
-    override val activeMobileDataSubscriptionId: StateFlow<Int> =
+    override val activeMobileDataSubscriptionId: StateFlow<Int?> =
         activeRepo
             .flatMapLatest { it.activeMobileDataSubscriptionId }
             .stateIn(
@@ -124,6 +123,15 @@ constructor(
                 realRepository.activeMobileDataSubscriptionId.value
             )
 
+    override val activeMobileDataRepository: StateFlow<MobileConnectionRepository?> =
+        activeRepo
+            .flatMapLatest { it.activeMobileDataRepository }
+            .stateIn(
+                scope,
+                SharingStarted.WhileSubscribed(),
+                realRepository.activeMobileDataRepository.value
+            )
+
     override val activeSubChangedInGroupEvent: Flow<Unit> =
         activeRepo.flatMapLatest { it.activeSubChangedInGroupEvent }
 

packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/demo/DemoMobileConnectionsRepository.kt

@@ -55,6 +55,7 @@ import kotlinx.coroutines.flow.SharingStarted
 import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.filterNotNull
 import kotlinx.coroutines.flow.flowOf
+import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.flow.mapLatest
 import kotlinx.coroutines.flow.onEach
 import kotlinx.coroutines.flow.stateIn
@@ -121,6 +122,15 @@ constructor(
                 subscriptions.value.firstOrNull()?.subscriptionId ?: INVALID_SUBSCRIPTION_ID
             )
 
+    override val activeMobileDataRepository: StateFlow<MobileConnectionRepository?> =
+        activeMobileDataSubscriptionId
+            .map { getRepoForSubId(it) }
+            .stateIn(
+                scope,
+                SharingStarted.WhileSubscribed(),
+                getRepoForSubId(activeMobileDataSubscriptionId.value)
+            )
+
     // TODO(b/261029387): consider adding a demo command for this
     override val activeSubChangedInGroupEvent: Flow<Unit> = flowOf()
 

packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryImpl.kt

@@ -159,13 +159,16 @@ constructor(
             )
             .stateIn(scope, started = SharingStarted.WhileSubscribed(), listOf())
 
-    /** StateFlow that keeps track of the current active mobile data subscription */
+    override val activeMobileDataSubscriptionId: StateFlow<Int?> =
-    override val activeMobileDataSubscriptionId: StateFlow<Int> =
         conflatedCallbackFlow {
                 val callback =
                     object : TelephonyCallback(), ActiveDataSubscriptionIdListener {
                         override fun onActiveDataSubscriptionIdChanged(subId: Int) {
-                            trySend(subId)
+                            if (subId != INVALID_SUBSCRIPTION_ID) {
+                                trySend(subId)
+                            } else {
+                                trySend(null)
+                            }
                         }
                     }
 
@@ -179,7 +182,18 @@ constructor(
                 columnName = "activeSubId",
                 initialValue = INVALID_SUBSCRIPTION_ID,
             )
-            .stateIn(scope, started = SharingStarted.WhileSubscribed(), INVALID_SUBSCRIPTION_ID)
+            .stateIn(scope, started = SharingStarted.WhileSubscribed(), null)
+
+    override val activeMobileDataRepository =
+        activeMobileDataSubscriptionId
+            .map { activeSubId ->
+                if (activeSubId == null) {
+                    null
+                } else {
+                    getOrCreateRepoForSubId(activeSubId)
+                }
+            }
+            .stateIn(scope, SharingStarted.WhileSubscribed(), null)
 
     private val defaultDataSubIdChangeEvent: MutableSharedFlow<Unit> =
         MutableSharedFlow(extraBufferCapacity = 1)
@@ -240,10 +254,13 @@ constructor(
             )
         }
 
-        return subIdRepositoryCache[subId]
+        return getOrCreateRepoForSubId(subId)
-            ?: createRepositoryForSubId(subId).also { subIdRepositoryCache[subId] = it }
     }
 
+    private fun getOrCreateRepoForSubId(subId: Int) =
+        subIdRepositoryCache[subId]
+            ?: createRepositoryForSubId(subId).also { subIdRepositoryCache[subId] = it }
+
     @SuppressLint("MissingPermission")
     override val defaultMobileNetworkConnectivity: StateFlow<MobileConnectivityModel> =
         conflatedCallbackFlow {
@@ -292,7 +309,9 @@ constructor(
     override val activeSubChangedInGroupEvent =
         activeMobileDataSubscriptionId
             .pairwise()
-            .mapNotNull { (prevVal: Int, newVal: Int) ->
+            .mapNotNull { (prevVal: Int?, newVal: Int?) ->
+                if (prevVal == null || newVal == null) return@mapNotNull null
+
                 val prevSub = subscriptionManager.getActiveSubscriptionInfo(prevVal)?.groupUuid
                 val nextSub = subscriptionManager.getActiveSubscriptionInfo(newVal)?.groupUuid
 
@@ -300,15 +319,7 @@ constructor(
             }
             .flowOn(bgDispatcher)
 
-    private fun isValidSubId(subId: Int): Boolean {
+    private fun isValidSubId(subId: Int): Boolean = checkSub(subId, subscriptions.value)
-        subscriptions.value.forEach {
-            if (it.subscriptionId == subId) {
-                return true
-            }
-        }
-
-        return false
-    }
 
     @VisibleForTesting fun getSubIdRepoCache() = subIdRepositoryCache
 
@@ -335,12 +346,27 @@ constructor(
     private fun dropUnusedReposFromCache(newInfos: List<SubscriptionModel>) {
         // Remove any connection repository from the cache that isn't in the new set of IDs. They
         // will get garbage collected once their subscribers go away
-        val currentValidSubscriptionIds = newInfos.map { it.subscriptionId }
-
         subIdRepositoryCache =
-            subIdRepositoryCache
+            subIdRepositoryCache.filter { checkSub(it.key, newInfos) }.toMutableMap()
-                .filter { currentValidSubscriptionIds.contains(it.key) }
+    }
-                .toMutableMap()
+
+    /**
+     * True if the checked subId is in the list of current subs or the active mobile data subId
+     *
+     * @param checkedSubs the list to validate [subId] against. To invalidate the cache, pass in the
+     * new subscription list. Otherwise use [subscriptions.value] to validate a subId against the
+     * current known subscriptions
+     */
+    private fun checkSub(subId: Int, checkedSubs: List<SubscriptionModel>): Boolean {
+        if (activeMobileDataSubscriptionId.value == subId) return true
+
+        checkedSubs.forEach {
+            if (it.subscriptionId == subId) {
+                return true
+            }
+        }
+
+        return false
     }
 
     private suspend fun fetchSubscriptionsList(): List<SubscriptionInfo> =

packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractor.kt

@@ -18,7 +18,6 @@ package com.android.systemui.statusbar.pipeline.mobile.domain.interactor
 
 import android.telephony.CarrierConfigManager
 import android.telephony.SubscriptionManager
-import android.telephony.SubscriptionManager.INVALID_SUBSCRIPTION_ID
 import com.android.settingslib.SignalIcon.MobileIconGroup
 import com.android.settingslib.mobile.TelephonyIcons
 import com.android.systemui.dagger.SysUISingleton
@@ -118,22 +117,8 @@ constructor(
     userSetupRepo: UserSetupRepository,
     @Application private val scope: CoroutineScope,
 ) : MobileIconsInteractor {
-    private val activeMobileDataSubscriptionId =
-        mobileConnectionsRepo.activeMobileDataSubscriptionId
-
-    private val activeMobileDataConnectionRepo: StateFlow<MobileConnectionRepository?> =
-        activeMobileDataSubscriptionId
-            .mapLatest { activeId ->
-                if (activeId == INVALID_SUBSCRIPTION_ID) {
-                    null
-                } else {
-                    mobileConnectionsRepo.getRepoForSubId(activeId)
-                }
-            }
-            .stateIn(scope, SharingStarted.WhileSubscribed(), null)
-
     override val activeDataConnectionHasDataEnabled: StateFlow<Boolean> =
-        activeMobileDataConnectionRepo
+        mobileConnectionsRepo.activeMobileDataRepository
             .flatMapLatest { it?.dataEnabled ?: flowOf(false) }
             .stateIn(scope, SharingStarted.WhileSubscribed(), false)
 
@@ -154,35 +139,37 @@ constructor(
      * and by checking which subscription is opportunistic, or which one is active.
      */
     override val filteredSubscriptions: Flow<List<SubscriptionModel>> =
-        combine(unfilteredSubscriptions, activeMobileDataSubscriptionId) { unfilteredSubs, activeId
+        combine(
-            ->
+                unfilteredSubscriptions,
-            // Based on the old logic,
+                mobileConnectionsRepo.activeMobileDataSubscriptionId,
-            if (unfilteredSubs.size != 2) {
+            ) { unfilteredSubs, activeId ->
-                return@combine unfilteredSubs
+                // Based on the old logic,
-            }
+                if (unfilteredSubs.size != 2) {
+                    return@combine unfilteredSubs
+                }
 
-            val info1 = unfilteredSubs[0]
+                val info1 = unfilteredSubs[0]
-            val info2 = unfilteredSubs[1]
+                val info2 = unfilteredSubs[1]
-            // If both subscriptions are primary, show both
+                // If both subscriptions are primary, show both
-            if (!info1.isOpportunistic && !info2.isOpportunistic) {
+                if (!info1.isOpportunistic && !info2.isOpportunistic) {
-                return@combine unfilteredSubs
+                    return@combine unfilteredSubs
-            }
+                }
 
-            // NOTE: at this point, we are now returning a single SubscriptionInfo
+                // NOTE: at this point, we are now returning a single SubscriptionInfo
 
-            // If carrier required, always show the icon of the primary subscription.
+                // If carrier required, always show the icon of the primary subscription.
-            // Otherwise, show whichever subscription is currently active for internet.
+                // Otherwise, show whichever subscription is currently active for internet.
-            if (carrierConfigTracker.alwaysShowPrimarySignalBarInOpportunisticNetworkDefault) {
+                if (carrierConfigTracker.alwaysShowPrimarySignalBarInOpportunisticNetworkDefault) {
-                // return the non-opportunistic info
+                    // return the non-opportunistic info
-                return@combine if (info1.isOpportunistic) listOf(info2) else listOf(info1)
+                    return@combine if (info1.isOpportunistic) listOf(info2) else listOf(info1)
-            } else {
-                return@combine if (info1.subscriptionId == activeId) {
-                    listOf(info1)
                 } else {
-                    listOf(info2)
+                    return@combine if (info1.subscriptionId == activeId) {
+                        listOf(info1)
+                    } else {
+                        listOf(info2)
+                    }
                 }
             }
-        }
             .distinctUntilChanged()
             .logDiffsForTable(
                 tableLogger,

packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/FakeMobileConnectionsRepository.kt

@@ -55,8 +55,12 @@ class FakeMobileConnectionsRepository(
     private val _subscriptions = MutableStateFlow<List<SubscriptionModel>>(listOf())
     override val subscriptions = _subscriptions
 
-    private val _activeMobileDataSubscriptionId = MutableStateFlow(INVALID_SUBSCRIPTION_ID)
+    private val _activeMobileDataSubscriptionId = MutableStateFlow<Int?>(null)
     override val activeMobileDataSubscriptionId = _activeMobileDataSubscriptionId
+
+    private val _activeMobileRepository = MutableStateFlow<MobileConnectionRepository?>(null)
+    override val activeMobileDataRepository = _activeMobileRepository
+
     override val activeSubChangedInGroupEvent: MutableSharedFlow<Unit> = MutableSharedFlow()
 
     private val _defaultDataSubId = MutableStateFlow(INVALID_SUBSCRIPTION_ID)
@@ -66,6 +70,7 @@ class FakeMobileConnectionsRepository(
     override val defaultMobileNetworkConnectivity = _mobileConnectivity
 
     private val subIdRepos = mutableMapOf<Int, MobileConnectionRepository>()
+
     override fun getRepoForSubId(subId: Int): MobileConnectionRepository {
         return subIdRepos[subId]
             ?: FakeMobileConnectionRepository(subId, tableLogBuffer).also { subIdRepos[subId] = it }
@@ -92,7 +97,14 @@ class FakeMobileConnectionsRepository(
     }
 
     fun setActiveMobileDataSubscriptionId(subId: Int) {
-        _activeMobileDataSubscriptionId.value = subId
+        // Simulate the filtering that the repo does
+        if (subId == INVALID_SUBSCRIPTION_ID) {
+            _activeMobileDataSubscriptionId.value = null
+            _activeMobileRepository.value = null
+        } else {
+            _activeMobileDataSubscriptionId.value = subId
+            _activeMobileRepository.value = getRepoForSubId(subId)
+        }
     }
 
     fun setMobileConnectionRepositoryMap(connections: Map<Int, MobileConnectionRepository>) {

packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryTest.kt

@@ -26,6 +26,7 @@ import android.os.ParcelUuid
 import android.telephony.CarrierConfigManager
 import android.telephony.SubscriptionInfo
 import android.telephony.SubscriptionManager
+import android.telephony.SubscriptionManager.INVALID_SUBSCRIPTION_ID
 import android.telephony.TelephonyCallback
 import android.telephony.TelephonyCallback.ActiveDataSubscriptionIdListener
 import android.telephony.TelephonyManager
@@ -39,6 +40,7 @@ import com.android.systemui.log.table.TableLogBufferFactory
 import com.android.systemui.statusbar.pipeline.mobile.data.model.MobileConnectivityModel
 import com.android.systemui.statusbar.pipeline.mobile.data.model.SubscriptionModel
 import com.android.systemui.statusbar.pipeline.mobile.data.repository.CarrierConfigRepository
+import com.android.systemui.statusbar.pipeline.mobile.data.repository.MobileConnectionRepository
 import com.android.systemui.statusbar.pipeline.mobile.data.repository.prod.FullMobileConnectionRepository.Factory.Companion.tableBufferLogName
 import com.android.systemui.statusbar.pipeline.mobile.util.FakeMobileMappingsProxy
 import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger
@@ -55,6 +57,7 @@ import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.cancel
+import kotlinx.coroutines.flow.filterNotNull
 import kotlinx.coroutines.flow.launchIn
 import kotlinx.coroutines.flow.onEach
 import kotlinx.coroutines.runBlocking
@@ -256,10 +259,9 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() {
         }
 
     @Test
-    fun testActiveDataSubscriptionId_initialValueIsInvalidId() =
+    fun testActiveDataSubscriptionId_initialValueIsNull() =
         runBlocking(IMMEDIATE) {
-            assertThat(underTest.activeMobileDataSubscriptionId.value)
+            assertThat(underTest.activeMobileDataSubscriptionId.value).isEqualTo(null)
-                .isEqualTo(SubscriptionManager.INVALID_SUBSCRIPTION_ID)
         }
 
     @Test
@@ -277,6 +279,140 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() {
             job.cancel()
         }
 
+    @Test
+    fun activeSubId_nullIfInvalidSubIdIsReceived() =
+        runBlocking(IMMEDIATE) {
+            var latest: Int? = null
+
+            val job = underTest.activeMobileDataSubscriptionId.onEach { latest = it }.launchIn(this)
+
+            getTelephonyCallbackForType<ActiveDataSubscriptionIdListener>()
+                .onActiveDataSubscriptionIdChanged(SUB_2_ID)
+
+            assertThat(latest).isNotNull()
+
+            getTelephonyCallbackForType<ActiveDataSubscriptionIdListener>()
+                .onActiveDataSubscriptionIdChanged(INVALID_SUBSCRIPTION_ID)
+
+            assertThat(latest).isNull()
+
+            job.cancel()
+        }
+
+    @Test
+    fun activeRepo_initiallyNull() {
+        assertThat(underTest.activeMobileDataRepository.value).isNull()
+    }
+
+    @Test
+    fun activeRepo_updatesWithActiveDataId() =
+        runBlocking(IMMEDIATE) {
+            var latest: MobileConnectionRepository? = null
+            val job = underTest.activeMobileDataRepository.onEach { latest = it }.launchIn(this)
+
+            getTelephonyCallbackForType<ActiveDataSubscriptionIdListener>()
+                .onActiveDataSubscriptionIdChanged(SUB_2_ID)
+
+            assertThat(latest?.subId).isEqualTo(SUB_2_ID)
+
+            job.cancel()
+        }
+
+    @Test
+    fun activeRepo_nullIfActiveDataSubIdBecomesInvalid() =
+        runBlocking(IMMEDIATE) {
+            var latest: MobileConnectionRepository? = null
+            val job = underTest.activeMobileDataRepository.onEach { latest = it }.launchIn(this)
+
+            getTelephonyCallbackForType<ActiveDataSubscriptionIdListener>()
+                .onActiveDataSubscriptionIdChanged(SUB_2_ID)
+
+            assertThat(latest).isNotNull()
+
+            getTelephonyCallbackForType<ActiveDataSubscriptionIdListener>()
+                .onActiveDataSubscriptionIdChanged(INVALID_SUBSCRIPTION_ID)
+
+            assertThat(latest).isNull()
+
+            job.cancel()
+        }
+
+    @Test
+    /** Regression test for b/268146648. */
+    fun activeSubIdIsSetBeforeSubscriptionsAreUpdated_doesNotThrow() =
+        runBlocking(IMMEDIATE) {
+            var activeRepo: MobileConnectionRepository? = null
+            var subscriptions: List<SubscriptionModel>? = null
+
+            val activeRepoJob =
+                underTest.activeMobileDataRepository.onEach { activeRepo = it }.launchIn(this)
+            val subscriptionsJob =
+                underTest.subscriptions.onEach { subscriptions = it }.launchIn(this)
+
+            getTelephonyCallbackForType<ActiveDataSubscriptionIdListener>()
+                .onActiveDataSubscriptionIdChanged(SUB_2_ID)
+
+            assertThat(subscriptions).isEmpty()
+            assertThat(activeRepo).isNotNull()
+
+            activeRepoJob.cancel()
+            subscriptionsJob.cancel()
+        }
+
+    @Test
+    fun getRepoForSubId_activeDataSubIdIsRequestedBeforeSubscriptionsUpdate() =
+        runBlocking(IMMEDIATE) {
+            var latest: MobileConnectionRepository? = null
+            var subscriptions: List<SubscriptionModel>? = null
+            val activeSubIdJob =
+                underTest.activeMobileDataSubscriptionId
+                    .filterNotNull()
+                    .onEach { latest = underTest.getRepoForSubId(it) }
+                    .launchIn(this)
+            val subscriptionsJob =
+                underTest.subscriptions.onEach { subscriptions = it }.launchIn(this)
+
+            // Active data subscription id is sent, but no subscription change has been posted yet
+            getTelephonyCallbackForType<ActiveDataSubscriptionIdListener>()
+                .onActiveDataSubscriptionIdChanged(SUB_2_ID)
+
+            // Subscriptions list is empty
+            assertThat(subscriptions).isEmpty()
+            // getRepoForSubId does not throw
+            assertThat(latest).isNotNull()
+
+            activeSubIdJob.cancel()
+            subscriptionsJob.cancel()
+        }
+
+    @Test
+    fun activeDataSentBeforeSubscriptionList_subscriptionReusesActiveDataRepo() =
+        runBlocking(IMMEDIATE) {
+            var activeRepo: MobileConnectionRepository? = null
+            val job = underTest.activeMobileDataRepository.onEach { activeRepo = it }.launchIn(this)
+            val subscriptionsJob = underTest.subscriptions.launchIn(this)
+
+            // GIVEN active repo is updated before the subscription list updates
+            getTelephonyCallbackForType<ActiveDataSubscriptionIdListener>()
+                .onActiveDataSubscriptionIdChanged(SUB_2_ID)
+
+            assertThat(activeRepo).isNotNull()
+
+            // GIVEN the subscription list is then updated which includes the active data sub id
+            whenever(subscriptionManager.completeActiveSubscriptionInfoList)
+                .thenReturn(listOf(SUB_2))
+            getSubscriptionCallback().onSubscriptionsChanged()
+
+            // WHEN requesting a connection repository for the subscription
+            val newRepo = underTest.getRepoForSubId(SUB_2_ID)
+
+            // THEN the newly request repo has been cached and reused
+            assertThat(activeRepo).isSameInstanceAs(newRepo)
+
+            job.cancel()
+            subscriptionsJob.cancel()
+        }
+
     @Test
     fun testConnectionRepository_validSubId_isCached() =
         runBlocking(IMMEDIATE) {
@@ -477,7 +613,7 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() {
         }
 
     @Test
-    fun `connection repository - log buffer contains sub id in its name`() =
+    fun connectionRepository_logBufferContainsSubIdInItsName() =
         runBlocking(IMMEDIATE) {
             val job = underTest.subscriptions.launchIn(this)
 
@@ -693,7 +829,7 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() {
         }
 
     @Test
-    fun `active data change - in same group - emits unit`() =
+    fun activeDataChange_inSameGroup_emitsUnit() =
         runBlocking(IMMEDIATE) {
             var latest: Unit? = null
             val job = underTest.activeSubChangedInGroupEvent.onEach { latest = it }.launchIn(this)
@@ -709,7 +845,7 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() {
         }
 
     @Test
-    fun `active data change - not in same group - does not emit`() =
+    fun activeDataChange_notInSameGroup_doesNotEmit() =
         runBlocking(IMMEDIATE) {
             var latest: Unit? = null
             val job = underTest.activeSubChangedInGroupEvent.onEach { latest = it }.launchIn(this)