From 884f426fe2ff1211bf3c6043f794679b3376da77 Mon Sep 17 00:00:00 2001 From: Caitlin Shkuratov Date: Wed, 3 May 2023 16:19:26 +0000 Subject: [PATCH 1/3] [SB Refactor] Convert MobileConnectionsRepositoryTest to testScope etc. Bug: 238425913 Test: atest MobileConnectionsRepositoryTest Change-Id: I8576cc36b034a6ccaf087e5f7a111b3a3d30c1fd --- .../prod/MobileConnectionsRepositoryTest.kt | 389 ++++++------------ 1 file changed, 134 insertions(+), 255 deletions(-) diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryTest.kt index 7cc59b67414b9..4eabecac92392 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryTest.kt @@ -34,11 +34,13 @@ import android.telephony.SubscriptionManager.INVALID_SUBSCRIPTION_ID import android.telephony.TelephonyCallback import android.telephony.TelephonyCallback.ActiveDataSubscriptionIdListener import android.telephony.TelephonyManager +import android.testing.TestableLooper import androidx.test.filters.SmallTest import com.android.internal.telephony.PhoneConstants import com.android.settingslib.R import com.android.settingslib.mobile.MobileMappings import com.android.systemui.SysuiTestCase +import com.android.systemui.coroutines.collectLastValue import com.android.systemui.log.table.TableLogBuffer import com.android.systemui.log.table.TableLogBufferFactory import com.android.systemui.statusbar.pipeline.mobile.data.MobileInputLogger @@ -60,16 +62,15 @@ import com.android.systemui.util.mockito.mock import com.android.systemui.util.mockito.whenever import com.google.common.truth.Truth.assertThat import java.util.UUID -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 +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.runCurrent +import kotlinx.coroutines.test.runTest import kotlinx.coroutines.yield -import org.junit.After import org.junit.Assert.assertThrows import org.junit.Assert.assertTrue import org.junit.Before @@ -83,6 +84,9 @@ import org.mockito.MockitoAnnotations @Suppress("EXPERIMENTAL_IS_NOT_ENABLED") @OptIn(ExperimentalCoroutinesApi::class) @SmallTest +// This is required because our [SubscriptionManager.OnSubscriptionsChangedListener] uses a looper +// to run the callback and this makes the looper place nicely with TestScope etc. +@TestableLooper.RunWithLooper class MobileConnectionsRepositoryTest : SysuiTestCase() { private lateinit var underTest: MobileConnectionsRepositoryImpl @@ -102,7 +106,8 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { private val mobileMappings = FakeMobileMappingsProxy() private val subscriptionManagerProxy = FakeSubscriptionManagerProxy() - private val scope = CoroutineScope(IMMEDIATE) + private val dispatcher = StandardTestDispatcher() + private val testScope = TestScope(dispatcher) @Before fun setUp() { @@ -138,7 +143,7 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { context, mock(), mock(), - scope, + testScope.backgroundScope, mock(), ) @@ -150,28 +155,28 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { mock(), mock(), logger, - scope, + testScope.backgroundScope, ) connectionFactory = MobileConnectionRepositoryImpl.Factory( fakeBroadcastDispatcher, telephonyManager = telephonyManager, - bgDispatcher = IMMEDIATE, + bgDispatcher = dispatcher, logger = logger, mobileMappingsProxy = mobileMappings, - scope = scope, + scope = testScope.backgroundScope, carrierConfigRepository = carrierConfigRepository, ) carrierMergedFactory = CarrierMergedConnectionRepository.Factory( telephonyManager, - scope, + testScope.backgroundScope, wifiRepository, ) fullConnectionFactory = FullMobileConnectionRepository.Factory( - scope = scope, + scope = testScope.backgroundScope, logFactory = logBufferFactory, mobileRepoFactory = connectionFactory, carrierMergedRepoFactory = carrierMergedFactory, @@ -188,46 +193,37 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { mobileMappings, fakeBroadcastDispatcher, context, - IMMEDIATE, - scope, + dispatcher, + testScope.backgroundScope, wifiRepository, fullConnectionFactory, ) - } - @After - fun tearDown() { - scope.cancel() + testScope.runCurrent() } @Test fun testSubscriptions_initiallyEmpty() = - runBlocking(IMMEDIATE) { + testScope.runTest { assertThat(underTest.subscriptions.value).isEqualTo(listOf()) } @Test fun testSubscriptions_listUpdates() = - runBlocking(IMMEDIATE) { - var latest: List? = null - - val job = underTest.subscriptions.onEach { latest = it }.launchIn(this) + testScope.runTest { + val latest by collectLastValue(underTest.subscriptions) whenever(subscriptionManager.completeActiveSubscriptionInfoList) .thenReturn(listOf(SUB_1, SUB_2)) getSubscriptionCallback().onSubscriptionsChanged() assertThat(latest).isEqualTo(listOf(MODEL_1, MODEL_2)) - - job.cancel() } @Test fun testSubscriptions_removingSub_updatesList() = - runBlocking(IMMEDIATE) { - var latest: List? = null - - val job = underTest.subscriptions.onEach { latest = it }.launchIn(this) + testScope.runTest { + val latest by collectLastValue(underTest.subscriptions) // WHEN 2 networks show up whenever(subscriptionManager.completeActiveSubscriptionInfoList) @@ -241,16 +237,12 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { // THEN the subscriptions list represents the newest change assertThat(latest).isEqualTo(listOf(MODEL_2)) - - job.cancel() } @Test fun testSubscriptions_carrierMergedOnly_listHasCarrierMerged() = - runBlocking(IMMEDIATE) { - var latest: List? = null - - val job = underTest.subscriptions.onEach { latest = it }.launchIn(this) + testScope.runTest { + val latest by collectLastValue(underTest.subscriptions) wifiRepository.setWifiNetwork(WIFI_NETWORK_CM) whenever(subscriptionManager.completeActiveSubscriptionInfoList) @@ -258,16 +250,12 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { getSubscriptionCallback().onSubscriptionsChanged() assertThat(latest).isEqualTo(listOf(MODEL_CM)) - - job.cancel() } @Test fun testSubscriptions_carrierMergedAndOther_listHasBothWithCarrierMergedLast() = - runBlocking(IMMEDIATE) { - var latest: List? = null - - val job = underTest.subscriptions.onEach { latest = it }.launchIn(this) + testScope.runTest { + val latest by collectLastValue(underTest.subscriptions) wifiRepository.setWifiNetwork(WIFI_NETWORK_CM) whenever(subscriptionManager.completeActiveSubscriptionInfoList) @@ -275,37 +263,29 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { getSubscriptionCallback().onSubscriptionsChanged() assertThat(latest).isEqualTo(listOf(MODEL_1, MODEL_2, MODEL_CM)) - - job.cancel() } @Test fun testActiveDataSubscriptionId_initialValueIsNull() = - runBlocking(IMMEDIATE) { + testScope.runTest { assertThat(underTest.activeMobileDataSubscriptionId.value).isEqualTo(null) } @Test fun testActiveDataSubscriptionId_updates() = - runBlocking(IMMEDIATE) { - var active: Int? = null - - val job = underTest.activeMobileDataSubscriptionId.onEach { active = it }.launchIn(this) + testScope.runTest { + val active by collectLastValue(underTest.activeMobileDataSubscriptionId) getTelephonyCallbackForType() .onActiveDataSubscriptionIdChanged(SUB_2_ID) assertThat(active).isEqualTo(SUB_2_ID) - - job.cancel() } @Test fun activeSubId_nullIfInvalidSubIdIsReceived() = - runBlocking(IMMEDIATE) { - var latest: Int? = null - - val job = underTest.activeMobileDataSubscriptionId.onEach { latest = it }.launchIn(this) + testScope.runTest { + val latest by collectLastValue(underTest.activeMobileDataSubscriptionId) getTelephonyCallbackForType() .onActiveDataSubscriptionIdChanged(SUB_2_ID) @@ -316,8 +296,6 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { .onActiveDataSubscriptionIdChanged(INVALID_SUBSCRIPTION_ID) assertThat(latest).isNull() - - job.cancel() } @Test @@ -327,23 +305,19 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { @Test fun activeRepo_updatesWithActiveDataId() = - runBlocking(IMMEDIATE) { - var latest: MobileConnectionRepository? = null - val job = underTest.activeMobileDataRepository.onEach { latest = it }.launchIn(this) + testScope.runTest { + val latest by collectLastValue(underTest.activeMobileDataRepository) getTelephonyCallbackForType() .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) + testScope.runTest { + val latest by collectLastValue(underTest.activeMobileDataRepository) getTelephonyCallbackForType() .onActiveDataSubscriptionIdChanged(SUB_2_ID) @@ -354,64 +328,49 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { .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? = null - - val activeRepoJob = - underTest.activeMobileDataRepository.onEach { activeRepo = it }.launchIn(this) - val subscriptionsJob = - underTest.subscriptions.onEach { subscriptions = it }.launchIn(this) + testScope.runTest { + val activeRepo by collectLastValue(underTest.activeMobileDataRepository) + val subscriptions by collectLastValue(underTest.subscriptions) getTelephonyCallbackForType() .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? = null - val activeSubIdJob = - underTest.activeMobileDataSubscriptionId - .filterNotNull() - .onEach { latest = underTest.getRepoForSubId(it) } - .launchIn(this) - val subscriptionsJob = - underTest.subscriptions.onEach { subscriptions = it }.launchIn(this) + testScope.runTest { + var latestActiveRepo: MobileConnectionRepository? = null + collectLastValue( + underTest.activeMobileDataSubscriptionId.filterNotNull().onEach { + latestActiveRepo = underTest.getRepoForSubId(it) + } + ) + + val latestSubscriptions by collectLastValue(underTest.subscriptions) // Active data subscription id is sent, but no subscription change has been posted yet getTelephonyCallbackForType() .onActiveDataSubscriptionIdChanged(SUB_2_ID) // Subscriptions list is empty - assertThat(subscriptions).isEmpty() + assertThat(latestSubscriptions).isEmpty() // getRepoForSubId does not throw - assertThat(latest).isNotNull() - - activeSubIdJob.cancel() - subscriptionsJob.cancel() + assertThat(latestActiveRepo).isNotNull() } @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) + testScope.runTest { + val activeRepo by collectLastValue(underTest.activeMobileDataRepository) + collectLastValue(underTest.subscriptions) // GIVEN active repo is updated before the subscription list updates getTelephonyCallbackForType() @@ -429,15 +388,12 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { // 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) { - val job = underTest.subscriptions.launchIn(this) + testScope.runTest { + collectLastValue(underTest.subscriptions) whenever(subscriptionManager.completeActiveSubscriptionInfoList) .thenReturn(listOf(SUB_1)) @@ -447,14 +403,12 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { val repo2 = underTest.getRepoForSubId(SUB_1_ID) assertThat(repo1).isSameInstanceAs(repo2) - - job.cancel() } @Test fun testConnectionRepository_carrierMergedSubId_isCached() = - runBlocking(IMMEDIATE) { - val job = underTest.subscriptions.launchIn(this) + testScope.runTest { + collectLastValue(underTest.subscriptions) wifiRepository.setWifiNetwork(WIFI_NETWORK_CM) whenever(subscriptionManager.completeActiveSubscriptionInfoList) @@ -465,14 +419,12 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { val repo2 = underTest.getRepoForSubId(SUB_CM_ID) assertThat(repo1).isSameInstanceAs(repo2) - - job.cancel() } @Test fun testConnectionRepository_carrierMergedAndMobileSubs_usesCorrectRepos() = - runBlocking(IMMEDIATE) { - val job = underTest.subscriptions.launchIn(this) + testScope.runTest { + collectLastValue(underTest.subscriptions) wifiRepository.setWifiNetwork(WIFI_NETWORK_CM) whenever(subscriptionManager.completeActiveSubscriptionInfoList) @@ -483,14 +435,12 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { val mobileRepo = underTest.getRepoForSubId(SUB_1_ID) assertThat(carrierMergedRepo.getIsCarrierMerged()).isTrue() assertThat(mobileRepo.getIsCarrierMerged()).isFalse() - - job.cancel() } @Test fun testSubscriptions_subNoLongerCarrierMerged_repoUpdates() = - runBlocking(IMMEDIATE) { - val job = underTest.subscriptions.launchIn(this) + testScope.runTest { + collectLastValue(underTest.subscriptions) wifiRepository.setWifiNetwork(WIFI_NETWORK_CM) whenever(subscriptionManager.completeActiveSubscriptionInfoList) @@ -504,20 +454,19 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { // WHEN the wifi network updates to be not carrier merged wifiRepository.setWifiNetwork(WifiNetworkModel.Active(networkId = 4, level = 1)) + runCurrent() // THEN the repos update val noLongerCarrierMergedRepo = underTest.getRepoForSubId(SUB_CM_ID) mobileRepo = underTest.getRepoForSubId(SUB_1_ID) assertThat(noLongerCarrierMergedRepo.getIsCarrierMerged()).isFalse() assertThat(mobileRepo.getIsCarrierMerged()).isFalse() - - job.cancel() } @Test fun testSubscriptions_subBecomesCarrierMerged_repoUpdates() = - runBlocking(IMMEDIATE) { - val job = underTest.subscriptions.launchIn(this) + testScope.runTest { + collectLastValue(underTest.subscriptions) wifiRepository.setWifiNetwork(WifiNetworkModel.Inactive) whenever(subscriptionManager.completeActiveSubscriptionInfoList) @@ -531,20 +480,19 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { // WHEN the wifi network updates to be carrier merged wifiRepository.setWifiNetwork(WIFI_NETWORK_CM) + runCurrent() // THEN the repos update val carrierMergedRepo = underTest.getRepoForSubId(SUB_CM_ID) mobileRepo = underTest.getRepoForSubId(SUB_1_ID) assertThat(carrierMergedRepo.getIsCarrierMerged()).isTrue() assertThat(mobileRepo.getIsCarrierMerged()).isFalse() - - job.cancel() } @Test fun testConnectionCache_clearsInvalidSubscriptions() = - runBlocking(IMMEDIATE) { - val job = underTest.subscriptions.launchIn(this) + testScope.runTest { + collectLastValue(underTest.subscriptions) whenever(subscriptionManager.completeActiveSubscriptionInfoList) .thenReturn(listOf(SUB_1, SUB_2)) @@ -563,14 +511,12 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { getSubscriptionCallback().onSubscriptionsChanged() assertThat(underTest.getSubIdRepoCache()).containsExactly(SUB_1_ID, repo1) - - job.cancel() } @Test fun testConnectionCache_clearsInvalidSubscriptions_includingCarrierMerged() = - runBlocking(IMMEDIATE) { - val job = underTest.subscriptions.launchIn(this) + testScope.runTest { + collectLastValue(underTest.subscriptions) wifiRepository.setWifiNetwork(WIFI_NETWORK_CM) whenever(subscriptionManager.completeActiveSubscriptionInfoList) @@ -591,15 +537,13 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { getSubscriptionCallback().onSubscriptionsChanged() assertThat(underTest.getSubIdRepoCache()).containsExactly(SUB_1_ID, repo1) - - job.cancel() } /** Regression test for b/261706421 */ @Test fun testConnectionsCache_clearMultipleSubscriptionsAtOnce_doesNotThrow() = - runBlocking(IMMEDIATE) { - val job = underTest.subscriptions.launchIn(this) + testScope.runTest { + collectLastValue(underTest.subscriptions) whenever(subscriptionManager.completeActiveSubscriptionInfoList) .thenReturn(listOf(SUB_1, SUB_2)) @@ -617,26 +561,20 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { getSubscriptionCallback().onSubscriptionsChanged() assertThat(underTest.getSubIdRepoCache()).isEmpty() - - job.cancel() } @Test fun testConnectionRepository_invalidSubId_throws() = - runBlocking(IMMEDIATE) { - val job = underTest.subscriptions.launchIn(this) - + testScope.runTest { assertThrows(IllegalArgumentException::class.java) { underTest.getRepoForSubId(SUB_1_ID) } - - job.cancel() } @Test fun connectionRepository_logBufferContainsSubIdInItsName() = - runBlocking(IMMEDIATE) { - val job = underTest.subscriptions.launchIn(this) + testScope.runTest { + collectLastValue(underTest.subscriptions) whenever(subscriptionManager.completeActiveSubscriptionInfoList) .thenReturn(listOf(SUB_1, SUB_2)) @@ -655,15 +593,12 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { eq(tableBufferLogName(SUB_2_ID)), anyInt(), ) - - job.cancel() } @Test fun testDefaultDataSubId_updatesOnBroadcast() = - runBlocking(IMMEDIATE) { - var latest: Int? = null - val job = underTest.defaultDataSubId.onEach { latest = it }.launchIn(this) + testScope.runTest { + val latest by collectLastValue(underTest.defaultDataSubId) assertThat(latest).isEqualTo(INVALID_SUBSCRIPTION_ID) @@ -686,28 +621,24 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { } assertThat(latest).isEqualTo(SUB_1_ID) - - job.cancel() } @Test fun defaultDataSubId_fetchesInitialValueOnStart() = - runBlocking(IMMEDIATE) { + testScope.runTest { subscriptionManagerProxy.defaultDataSubId = 2 - var latest: Int? = null - val job = underTest.defaultDataSubId.onEach { latest = it }.launchIn(this) + val latest by collectLastValue(underTest.defaultDataSubId) assertThat(latest).isEqualTo(2) - - job.cancel() } @Test fun defaultDataSubId_fetchesCurrentOnRestart() = - runBlocking(IMMEDIATE) { + testScope.runTest { subscriptionManagerProxy.defaultDataSubId = 2 var latest: Int? = null var job = underTest.defaultDataSubId.onEach { latest = it }.launchIn(this) + runCurrent() assertThat(latest).isEqualTo(2) @@ -720,6 +651,7 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { subscriptionManagerProxy.defaultDataSubId = 1 job = underTest.defaultDataSubId.onEach { latest = it }.launchIn(this) + runCurrent() assertThat(latest).isEqualTo(1) @@ -733,43 +665,37 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { @Test fun mobileIsDefault_capsHaveCellular_isDefault() = - runBlocking(IMMEDIATE) { + testScope.runTest { val caps = mock().also { whenever(it.hasTransport(TRANSPORT_CELLULAR)).thenReturn(true) } - var latest: Boolean? = null - val job = underTest.mobileIsDefault.onEach { latest = it }.launchIn(this) + val latest by collectLastValue(underTest.mobileIsDefault) getDefaultNetworkCallback().onCapabilitiesChanged(NETWORK, caps) assertThat(latest).isTrue() - - job.cancel() } @Test fun mobileIsDefault_capsDoNotHaveCellular_isNotDefault() = - runBlocking(IMMEDIATE) { + testScope.runTest { val caps = mock().also { whenever(it.hasTransport(TRANSPORT_CELLULAR)).thenReturn(false) } - var latest: Boolean? = null - val job = underTest.mobileIsDefault.onEach { latest = it }.launchIn(this) + val latest by collectLastValue(underTest.mobileIsDefault) getDefaultNetworkCallback().onCapabilitiesChanged(NETWORK, caps) assertThat(latest).isFalse() - - job.cancel() } @Test fun mobileIsDefault_carrierMergedViaMobile_isDefault() = - runBlocking(IMMEDIATE) { + testScope.runTest { val carrierMergedInfo = mock().apply { whenever(this.isCarrierMerged).thenReturn(true) } val caps = @@ -778,56 +704,47 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { whenever(it.transportInfo).thenReturn(carrierMergedInfo) } - var latest: Boolean? = null - val job = underTest.mobileIsDefault.onEach { latest = it }.launchIn(this) + val latest by collectLastValue(underTest.mobileIsDefault) getDefaultNetworkCallback().onCapabilitiesChanged(NETWORK, caps) assertThat(latest).isTrue() - - job.cancel() } @Test fun mobileIsDefault_wifiDefault_mobileNotDefault() = - runBlocking(IMMEDIATE) { + testScope.runTest { val caps = mock().also { whenever(it.hasTransport(TRANSPORT_WIFI)).thenReturn(true) } - var latest: Boolean? = null - val job = underTest.mobileIsDefault.onEach { latest = it }.launchIn(this) + val latest by collectLastValue(underTest.mobileIsDefault) getDefaultNetworkCallback().onCapabilitiesChanged(NETWORK, caps) assertThat(latest).isFalse() - - job.cancel() } @Test fun mobileIsDefault_ethernetDefault_mobileNotDefault() = - runBlocking(IMMEDIATE) { + testScope.runTest { val caps = mock().also { whenever(it.hasTransport(TRANSPORT_ETHERNET)).thenReturn(true) } - var latest: Boolean? = null - val job = underTest.mobileIsDefault.onEach { latest = it }.launchIn(this) + val latest by collectLastValue(underTest.mobileIsDefault) getDefaultNetworkCallback().onCapabilitiesChanged(NETWORK, caps) assertThat(latest).isFalse() - - job.cancel() } /** Regression test for b/272586234. */ @Test fun hasCarrierMergedConnection_carrierMergedViaWifi_isTrue() = - runBlocking(IMMEDIATE) { + testScope.runTest { val carrierMergedInfo = mock().apply { whenever(this.isCarrierMerged).thenReturn(true) } val caps = @@ -836,20 +753,17 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { whenever(it.transportInfo).thenReturn(carrierMergedInfo) } - var latest: Boolean? = null - val job = underTest.hasCarrierMergedConnection.onEach { latest = it }.launchIn(this) + val latest by collectLastValue(underTest.hasCarrierMergedConnection) getDefaultNetworkCallback().onCapabilitiesChanged(NETWORK, caps) yield() assertThat(latest).isTrue() - - job.cancel() } @Test fun hasCarrierMergedConnection_carrierMergedViaMobile_isTrue() = - runBlocking(IMMEDIATE) { + testScope.runTest { val carrierMergedInfo = mock().apply { whenever(this.isCarrierMerged).thenReturn(true) } val caps = @@ -858,21 +772,18 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { whenever(it.transportInfo).thenReturn(carrierMergedInfo) } - var latest: Boolean? = null - val job = underTest.hasCarrierMergedConnection.onEach { latest = it }.launchIn(this) + val latest by collectLastValue(underTest.hasCarrierMergedConnection) getDefaultNetworkCallback().onCapabilitiesChanged(NETWORK, caps) yield() assertThat(latest).isTrue() - - job.cancel() } /** Regression test for b/272586234. */ @Test fun hasCarrierMergedConnection_carrierMergedViaWifiWithVcnTransport_isTrue() = - runBlocking(IMMEDIATE) { + testScope.runTest { val carrierMergedInfo = mock().apply { whenever(this.isCarrierMerged).thenReturn(true) } val caps = @@ -881,20 +792,17 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { whenever(it.transportInfo).thenReturn(VcnTransportInfo(carrierMergedInfo)) } - var latest: Boolean? = null - val job = underTest.hasCarrierMergedConnection.onEach { latest = it }.launchIn(this) + val latest by collectLastValue(underTest.hasCarrierMergedConnection) getDefaultNetworkCallback().onCapabilitiesChanged(NETWORK, caps) yield() assertThat(latest).isTrue() - - job.cancel() } @Test fun hasCarrierMergedConnection_carrierMergedViaMobileWithVcnTransport_isTrue() = - runBlocking(IMMEDIATE) { + testScope.runTest { val carrierMergedInfo = mock().apply { whenever(this.isCarrierMerged).thenReturn(true) } val caps = @@ -903,22 +811,18 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { whenever(it.transportInfo).thenReturn(VcnTransportInfo(carrierMergedInfo)) } - var latest: Boolean? = null - val job = underTest.hasCarrierMergedConnection.onEach { latest = it }.launchIn(this) + val latest by collectLastValue(underTest.hasCarrierMergedConnection) getDefaultNetworkCallback().onCapabilitiesChanged(NETWORK, caps) yield() assertThat(latest).isTrue() - - job.cancel() } @Test fun hasCarrierMergedConnection_isCarrierMergedViaUnderlyingWifi_isTrue() = - runBlocking(IMMEDIATE) { - var latest: Boolean? = null - val job = underTest.hasCarrierMergedConnection.onEach { latest = it }.launchIn(this) + testScope.runTest { + val latest by collectLastValue(underTest.hasCarrierMergedConnection) val underlyingNetwork = mock() val carrierMergedInfo = @@ -945,15 +849,12 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { // THEN there's a carrier merged connection assertThat(latest).isTrue() - - job.cancel() } @Test fun hasCarrierMergedConnection_isCarrierMergedViaUnderlyingCellular_isTrue() = - runBlocking(IMMEDIATE) { - var latest: Boolean? = null - val job = underTest.hasCarrierMergedConnection.onEach { latest = it }.launchIn(this) + testScope.runTest { + val latest by collectLastValue(underTest.hasCarrierMergedConnection) val underlyingCarrierMergedNetwork = mock() val carrierMergedInfo = @@ -981,16 +882,13 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { // THEN there's a carrier merged connection assertThat(latest).isTrue() - - job.cancel() } /** Regression test for b/272586234. */ @Test fun hasCarrierMergedConnection_defaultNotCarrierMerged_butWifiRepoHasCarrierMerged_isTrue() = - runBlocking(IMMEDIATE) { - var latest: Boolean? = null - val job = underTest.hasCarrierMergedConnection.onEach { latest = it }.launchIn(this) + testScope.runTest { + val latest by collectLastValue(underTest.hasCarrierMergedConnection) // WHEN the default callback isn't carrier merged val carrierMergedInfo = @@ -1009,8 +907,6 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { // THEN hasCarrierMergedConnection is true assertThat(latest).isTrue() - - job.cancel() } @Test @@ -1020,43 +916,37 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { @Test fun defaultConnectionIsValidated_capsHaveValidated_isValidated() = - runBlocking(IMMEDIATE) { + testScope.runTest { val caps = mock().also { whenever(it.hasCapability(NET_CAPABILITY_VALIDATED)).thenReturn(true) } - var latest: Boolean? = null - val job = underTest.defaultConnectionIsValidated.onEach { latest = it }.launchIn(this) + val latest by collectLastValue(underTest.defaultConnectionIsValidated) getDefaultNetworkCallback().onCapabilitiesChanged(NETWORK, caps) assertThat(latest).isTrue() - - job.cancel() } @Test fun defaultConnectionIsValidated_capsHaveNotValidated_isNotValidated() = - runBlocking(IMMEDIATE) { + testScope.runTest { val caps = mock().also { whenever(it.hasCapability(NET_CAPABILITY_VALIDATED)).thenReturn(false) } - var latest: Boolean? = null - val job = underTest.defaultConnectionIsValidated.onEach { latest = it }.launchIn(this) + val latest by collectLastValue(underTest.defaultConnectionIsValidated) getDefaultNetworkCallback().onCapabilitiesChanged(NETWORK, caps) assertThat(latest).isFalse() - - job.cancel() } @Test fun config_initiallyFromContext() = - runBlocking(IMMEDIATE) { + testScope.runTest { overrideResource(R.bool.config_showMin3G, true) val configFromContext = MobileMappings.Config.readConfig(context) assertThat(configFromContext.showAtLeast3G).isTrue() @@ -1074,26 +964,23 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { mobileMappings, fakeBroadcastDispatcher, context, - IMMEDIATE, - scope, + dispatcher, + testScope.backgroundScope, wifiRepository, fullConnectionFactory, ) - var latest: MobileMappings.Config? = null - val job = underTest.defaultDataSubRatConfig.onEach { latest = it }.launchIn(this) + val latest by collectLastValue(underTest.defaultDataSubRatConfig) assertTrue(latest!!.areEqual(configFromContext)) assertTrue(latest!!.showAtLeast3G) - - job.cancel() } @Test fun config_subIdChangeEvent_updated() = - runBlocking(IMMEDIATE) { - var latest: MobileMappings.Config? = null - val job = underTest.defaultDataSubRatConfig.onEach { latest = it }.launchIn(this) + testScope.runTest { + val latest by collectLastValue(underTest.defaultDataSubRatConfig) + assertThat(latest!!.showAtLeast3G).isFalse() overrideResource(R.bool.config_showMin3G, true) @@ -1112,15 +999,13 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { // THEN the config is updated assertTrue(latest!!.areEqual(configFromContext)) assertTrue(latest!!.showAtLeast3G) - - job.cancel() } @Test fun config_carrierConfigChangeEvent_updated() = - runBlocking(IMMEDIATE) { - var latest: MobileMappings.Config? = null - val job = underTest.defaultDataSubRatConfig.onEach { latest = it }.launchIn(this) + testScope.runTest { + val latest by collectLastValue(underTest.defaultDataSubRatConfig) + assertThat(latest!!.showAtLeast3G).isFalse() overrideResource(R.bool.config_showMin3G, true) @@ -1138,15 +1023,12 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { // THEN the config is updated assertThat(latest!!.areEqual(configFromContext)).isTrue() assertThat(latest!!.showAtLeast3G).isTrue() - - job.cancel() } @Test fun activeDataChange_inSameGroup_emitsUnit() = - runBlocking(IMMEDIATE) { - var latest: Unit? = null - val job = underTest.activeSubChangedInGroupEvent.onEach { latest = it }.launchIn(this) + testScope.runTest { + val latest by collectLastValue(underTest.activeSubChangedInGroupEvent) getTelephonyCallbackForType() .onActiveDataSubscriptionIdChanged(SUB_3_ID_GROUPED) @@ -1154,15 +1036,12 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { .onActiveDataSubscriptionIdChanged(SUB_4_ID_GROUPED) assertThat(latest).isEqualTo(Unit) - - job.cancel() } @Test fun activeDataChange_notInSameGroup_doesNotEmit() = - runBlocking(IMMEDIATE) { - var latest: Unit? = null - val job = underTest.activeSubChangedInGroupEvent.onEach { latest = it }.launchIn(this) + testScope.runTest { + val latest by collectLastValue(underTest.activeSubChangedInGroupEvent) getTelephonyCallbackForType() .onActiveDataSubscriptionIdChanged(SUB_3_ID_GROUPED) @@ -1170,38 +1049,38 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { .onActiveDataSubscriptionIdChanged(SUB_1_ID) assertThat(latest).isEqualTo(null) - - job.cancel() } - private fun getDefaultNetworkCallback(): ConnectivityManager.NetworkCallback { + private fun TestScope.getDefaultNetworkCallback(): ConnectivityManager.NetworkCallback { + runCurrent() val callbackCaptor = argumentCaptor() verify(connectivityManager).registerDefaultNetworkCallback(callbackCaptor.capture()) return callbackCaptor.value!! } - private fun getSubscriptionCallback(): SubscriptionManager.OnSubscriptionsChangedListener { + private fun TestScope.getSubscriptionCallback(): + SubscriptionManager.OnSubscriptionsChangedListener { + runCurrent() val callbackCaptor = argumentCaptor() verify(subscriptionManager) .addOnSubscriptionsChangedListener(any(), callbackCaptor.capture()) return callbackCaptor.value!! } - private fun getTelephonyCallbacks(): List { + private fun TestScope.getTelephonyCallbacks(): List { + runCurrent() val callbackCaptor = argumentCaptor() verify(telephonyManager).registerTelephonyCallback(any(), callbackCaptor.capture()) return callbackCaptor.allValues } - private inline fun getTelephonyCallbackForType(): T { - val cbs = getTelephonyCallbacks().filterIsInstance() + private inline fun TestScope.getTelephonyCallbackForType(): T { + val cbs = this.getTelephonyCallbacks().filterIsInstance() assertThat(cbs.size).isEqualTo(1) return cbs[0] } companion object { - private val IMMEDIATE = Dispatchers.Main.immediate - // Subscription 1 private const val SUB_1_ID = 1 private val GROUP_1 = ParcelUuid(UUID.randomUUID()) From f31398af73db5ba1eebbafc722de2cbce7efcba9 Mon Sep 17 00:00:00 2001 From: Caitlin Shkuratov Date: Wed, 3 May 2023 18:44:46 +0000 Subject: [PATCH 2/3] [SB Refactor] Use a real WifiRepo in MobileConnectionsRepositoryTest. Using the real wifi repo will let us send the same network capabilities objects to both the default network callback and the normal network callback in tests. This makes the tests more reflective of production scenarios. This is in preparation for the CL that fixes b/278618530, since that CL will rely more heavily on updating both the default and non-default network callbacks with information. Bug: 278618530 Test: atest MobileConnectionsRepositoryTest Change-Id: I917506fc3c04cd63a0236cdf356cd26719c20337 --- .../prod/MobileConnectionsRepositoryTest.kt | 81 ++++++++++++++----- 1 file changed, 60 insertions(+), 21 deletions(-) diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryTest.kt index 4eabecac92392..dccbe915792a9 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryTest.kt @@ -53,13 +53,15 @@ import com.android.systemui.statusbar.pipeline.mobile.util.FakeSubscriptionManag import com.android.systemui.statusbar.pipeline.shared.data.model.ConnectivitySlots import com.android.systemui.statusbar.pipeline.shared.data.repository.ConnectivityRepository import com.android.systemui.statusbar.pipeline.shared.data.repository.ConnectivityRepositoryImpl -import com.android.systemui.statusbar.pipeline.wifi.data.repository.FakeWifiRepository -import com.android.systemui.statusbar.pipeline.wifi.shared.model.WifiNetworkModel +import com.android.systemui.statusbar.pipeline.wifi.data.repository.WifiRepository +import com.android.systemui.statusbar.pipeline.wifi.data.repository.prod.WifiRepositoryImpl +import com.android.systemui.util.concurrency.FakeExecutor import com.android.systemui.util.mockito.any import com.android.systemui.util.mockito.argumentCaptor import com.android.systemui.util.mockito.eq import com.android.systemui.util.mockito.mock import com.android.systemui.util.mockito.whenever +import com.android.systemui.util.time.FakeSystemClock import com.google.common.truth.Truth.assertThat import java.util.UUID import kotlinx.coroutines.ExperimentalCoroutinesApi @@ -94,7 +96,7 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { private lateinit var carrierMergedFactory: CarrierMergedConnectionRepository.Factory private lateinit var fullConnectionFactory: FullMobileConnectionRepository.Factory private lateinit var connectivityRepository: ConnectivityRepository - private lateinit var wifiRepository: FakeWifiRepository + private lateinit var wifiRepository: WifiRepository private lateinit var carrierConfigRepository: CarrierConfigRepository @Mock private lateinit var connectivityManager: ConnectivityManager @Mock private lateinit var subscriptionManager: SubscriptionManager @@ -147,7 +149,17 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { mock(), ) - wifiRepository = FakeWifiRepository() + wifiRepository = + WifiRepositoryImpl( + fakeBroadcastDispatcher, + connectivityManager, + connectivityRepository, + mock(), + mock(), + FakeExecutor(FakeSystemClock()), + testScope.backgroundScope, + mock(), + ) carrierConfigRepository = CarrierConfigRepository( @@ -244,7 +256,7 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { testScope.runTest { val latest by collectLastValue(underTest.subscriptions) - wifiRepository.setWifiNetwork(WIFI_NETWORK_CM) + getNormalNetworkCallback().onCapabilitiesChanged(NETWORK, WIFI_NETWORK_CAPS_CM) whenever(subscriptionManager.completeActiveSubscriptionInfoList) .thenReturn(listOf(SUB_CM)) getSubscriptionCallback().onSubscriptionsChanged() @@ -257,7 +269,7 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { testScope.runTest { val latest by collectLastValue(underTest.subscriptions) - wifiRepository.setWifiNetwork(WIFI_NETWORK_CM) + getNormalNetworkCallback().onCapabilitiesChanged(NETWORK, WIFI_NETWORK_CAPS_CM) whenever(subscriptionManager.completeActiveSubscriptionInfoList) .thenReturn(listOf(SUB_1, SUB_2, SUB_CM)) getSubscriptionCallback().onSubscriptionsChanged() @@ -410,7 +422,7 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { testScope.runTest { collectLastValue(underTest.subscriptions) - wifiRepository.setWifiNetwork(WIFI_NETWORK_CM) + getNormalNetworkCallback().onCapabilitiesChanged(NETWORK, WIFI_NETWORK_CAPS_CM) whenever(subscriptionManager.completeActiveSubscriptionInfoList) .thenReturn(listOf(SUB_CM)) getSubscriptionCallback().onSubscriptionsChanged() @@ -426,7 +438,7 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { testScope.runTest { collectLastValue(underTest.subscriptions) - wifiRepository.setWifiNetwork(WIFI_NETWORK_CM) + getNormalNetworkCallback().onCapabilitiesChanged(NETWORK, WIFI_NETWORK_CAPS_CM) whenever(subscriptionManager.completeActiveSubscriptionInfoList) .thenReturn(listOf(SUB_1, SUB_CM)) getSubscriptionCallback().onSubscriptionsChanged() @@ -442,7 +454,7 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { testScope.runTest { collectLastValue(underTest.subscriptions) - wifiRepository.setWifiNetwork(WIFI_NETWORK_CM) + getNormalNetworkCallback().onCapabilitiesChanged(NETWORK, WIFI_NETWORK_CAPS_CM) whenever(subscriptionManager.completeActiveSubscriptionInfoList) .thenReturn(listOf(SUB_1, SUB_CM)) getSubscriptionCallback().onSubscriptionsChanged() @@ -453,7 +465,7 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { assertThat(mobileRepo.getIsCarrierMerged()).isFalse() // WHEN the wifi network updates to be not carrier merged - wifiRepository.setWifiNetwork(WifiNetworkModel.Active(networkId = 4, level = 1)) + getNormalNetworkCallback().onCapabilitiesChanged(NETWORK, WIFI_NETWORK_CAPS_ACTIVE) runCurrent() // THEN the repos update @@ -468,10 +480,11 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { testScope.runTest { collectLastValue(underTest.subscriptions) - wifiRepository.setWifiNetwork(WifiNetworkModel.Inactive) + getNormalNetworkCallback().onCapabilitiesChanged(NETWORK, WIFI_NETWORK_CAPS_ACTIVE) whenever(subscriptionManager.completeActiveSubscriptionInfoList) .thenReturn(listOf(SUB_1, SUB_CM)) getSubscriptionCallback().onSubscriptionsChanged() + runCurrent() val notYetCarrierMergedRepo = underTest.getRepoForSubId(SUB_CM_ID) var mobileRepo = underTest.getRepoForSubId(SUB_1_ID) @@ -479,7 +492,7 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { assertThat(mobileRepo.getIsCarrierMerged()).isFalse() // WHEN the wifi network updates to be carrier merged - wifiRepository.setWifiNetwork(WIFI_NETWORK_CM) + getNormalNetworkCallback().onCapabilitiesChanged(NETWORK, WIFI_NETWORK_CAPS_CM) runCurrent() // THEN the repos update @@ -518,7 +531,7 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { testScope.runTest { collectLastValue(underTest.subscriptions) - wifiRepository.setWifiNetwork(WIFI_NETWORK_CM) + getNormalNetworkCallback().onCapabilitiesChanged(NETWORK, WIFI_NETWORK_CAPS_CM) whenever(subscriptionManager.completeActiveSubscriptionInfoList) .thenReturn(listOf(SUB_1, SUB_2, SUB_CM)) getSubscriptionCallback().onSubscriptionsChanged() @@ -902,8 +915,7 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { yield() // BUT the wifi repo has gotten updates that it *is* carrier merged - wifiRepository.setWifiNetwork(WIFI_NETWORK_CM) - yield() + getNormalNetworkCallback().onCapabilitiesChanged(NETWORK, WIFI_NETWORK_CAPS_CM) // THEN hasCarrierMergedConnection is true assertThat(latest).isTrue() @@ -1058,6 +1070,14 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { return callbackCaptor.value!! } + // Note: This is used to update the [WifiRepository]. + private fun TestScope.getNormalNetworkCallback(): ConnectivityManager.NetworkCallback { + runCurrent() + val callbackCaptor = argumentCaptor() + verify(connectivityManager).registerNetworkCallback(any(), callbackCaptor.capture()) + return callbackCaptor.value!! + } + private fun TestScope.getSubscriptionCallback(): SubscriptionManager.OnSubscriptionsChangedListener { runCurrent() @@ -1138,11 +1158,30 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { private val SUB_CM = mock().also { whenever(it.subscriptionId).thenReturn(SUB_CM_ID) } private val MODEL_CM = SubscriptionModel(subscriptionId = SUB_CM_ID) - private val WIFI_NETWORK_CM = - WifiNetworkModel.CarrierMerged( - networkId = 3, - subscriptionId = SUB_CM_ID, - level = 1, - ) + + private val WIFI_INFO_CM = + mock().apply { + whenever(this.isPrimary).thenReturn(true) + whenever(this.isCarrierMerged).thenReturn(true) + whenever(this.subscriptionId).thenReturn(SUB_CM_ID) + } + private val WIFI_NETWORK_CAPS_CM = + mock().also { + whenever(it.hasTransport(TRANSPORT_WIFI)).thenReturn(true) + whenever(it.transportInfo).thenReturn(WIFI_INFO_CM) + whenever(it.hasCapability(NET_CAPABILITY_VALIDATED)).thenReturn(true) + } + + private val WIFI_INFO_ACTIVE = + mock().apply { + whenever(this.isPrimary).thenReturn(true) + whenever(this.isCarrierMerged).thenReturn(false) + } + private val WIFI_NETWORK_CAPS_ACTIVE = + mock().also { + whenever(it.hasTransport(TRANSPORT_WIFI)).thenReturn(true) + whenever(it.transportInfo).thenReturn(WIFI_INFO_ACTIVE) + whenever(it.hasCapability(NET_CAPABILITY_VALIDATED)).thenReturn(true) + } } } From 476bf5a5665b4f5c7ea4e0ad15d0d89bb4669767 Mon Sep 17 00:00:00 2001 From: Caitlin Shkuratov Date: Wed, 3 May 2023 19:29:00 +0000 Subject: [PATCH 3/3] [SB Refactor] Only use carrier merged sub ID if wifi or CM are default. (or the device is in airplane mode) The old pipeline's WifiSignalController only sent updates to MobileSignalController if the wifi info was carrier merged *and* wifi or carrier merged was the default connection or the device was in airplane mode. The new pipeline previously didn't check this default status, which caused the new pipeline to keep polling the carrier merged info instead of the normal cellular info. Also increases the size of the connectivity buffer, since investigating the BRs attached to this bug revealed that the buffer cut off too early. Bug: 278618530 Test: atest MobileConnectionsRepositoryTest Change-Id: Ib9dde0234c38921bb67179a138f9cc8c60e7d669 --- .../dagger/StatusBarPipelineModule.kt | 2 +- .../prod/MobileConnectionsRepositoryImpl.kt | 28 +++-- .../MobileRepositorySwitcherTest.kt | 2 + .../prod/MobileConnectionsRepositoryTest.kt | 106 +++++++++++++++--- 4 files changed, 111 insertions(+), 27 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/dagger/StatusBarPipelineModule.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/dagger/StatusBarPipelineModule.kt index 7aa90336e2bfa..49de5a232f300 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/dagger/StatusBarPipelineModule.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/dagger/StatusBarPipelineModule.kt @@ -160,7 +160,7 @@ abstract class StatusBarPipelineModule { @SysUISingleton @SharedConnectivityInputLog fun provideSharedConnectivityTableLogBuffer(factory: LogBufferFactory): LogBuffer { - return factory.create("SharedConnectivityInputLog", 30) + return factory.create("SharedConnectivityInputLog", 60) } @Provides diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryImpl.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryImpl.kt index 0e9b6c56437ed..81a068d10a145 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryImpl.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryImpl.kt @@ -38,6 +38,7 @@ import com.android.systemui.dagger.qualifiers.Application import com.android.systemui.dagger.qualifiers.Background import com.android.systemui.log.table.TableLogBuffer import com.android.systemui.log.table.logDiffsForTable +import com.android.systemui.statusbar.pipeline.airplane.data.repository.AirplaneModeRepository import com.android.systemui.statusbar.pipeline.dagger.MobileSummaryLog import com.android.systemui.statusbar.pipeline.mobile.data.MobileInputLogger import com.android.systemui.statusbar.pipeline.mobile.data.model.NetworkNameModel @@ -88,6 +89,7 @@ constructor( private val context: Context, @Background private val bgDispatcher: CoroutineDispatcher, @Application private val scope: CoroutineScope, + airplaneModeRepository: AirplaneModeRepository, // Some "wifi networks" should be rendered as a mobile connection, which is why the wifi // repository is an input to the mobile repository. // See [CarrierMergedConnectionRepository] for details. @@ -106,10 +108,20 @@ constructor( context.getString(R.string.status_bar_network_name_separator) private val carrierMergedSubId: StateFlow = - wifiRepository.wifiNetwork - .mapLatest { - if (it is WifiNetworkModel.CarrierMerged) { - it.subscriptionId + combine( + wifiRepository.wifiNetwork, + connectivityRepository.defaultConnections, + airplaneModeRepository.isAirplaneMode, + ) { wifiNetwork, defaultConnections, isAirplaneMode -> + // The carrier merged connection should only be used if it's also the default + // connection or mobile connections aren't available because of airplane mode. + val defaultConnectionIsNonMobile = + defaultConnections.carrierMerged.isDefault || + defaultConnections.wifi.isDefault || + isAirplaneMode + + if (wifiNetwork is WifiNetworkModel.CarrierMerged && defaultConnectionIsNonMobile) { + wifiNetwork.subscriptionId } else { null } @@ -269,12 +281,8 @@ constructor( .stateIn(scope, SharingStarted.WhileSubscribed(), false) override val hasCarrierMergedConnection: StateFlow = - combine( - connectivityRepository.defaultConnections, - carrierMergedSubId, - ) { defaultConnections, carrierMergedSubId -> - defaultConnections.carrierMerged.isDefault || carrierMergedSubId != null - } + carrierMergedSubId + .map { it != null } .distinctUntilChanged() .logDiffsForTable( tableLogger, diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/MobileRepositorySwitcherTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/MobileRepositorySwitcherTest.kt index bde05b9f499ed..5a887ebcee800 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/MobileRepositorySwitcherTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/MobileRepositorySwitcherTest.kt @@ -26,6 +26,7 @@ import com.android.systemui.demomode.DemoModeController import com.android.systemui.dump.DumpManager import com.android.systemui.log.table.TableLogBuffer import com.android.systemui.log.table.TableLogBufferFactory +import com.android.systemui.statusbar.pipeline.airplane.data.repository.FakeAirplaneModeRepository import com.android.systemui.statusbar.pipeline.mobile.data.MobileInputLogger import com.android.systemui.statusbar.pipeline.mobile.data.model.SubscriptionModel import com.android.systemui.statusbar.pipeline.mobile.data.repository.demo.DemoMobileConnectionsRepository @@ -131,6 +132,7 @@ class MobileRepositorySwitcherTest : SysuiTestCase() { context, IMMEDIATE, scope, + FakeAirplaneModeRepository(), wifiRepository, mock(), ) diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryTest.kt index dccbe915792a9..38c7432eb288a 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryTest.kt @@ -43,6 +43,7 @@ import com.android.systemui.SysuiTestCase import com.android.systemui.coroutines.collectLastValue import com.android.systemui.log.table.TableLogBuffer import com.android.systemui.log.table.TableLogBufferFactory +import com.android.systemui.statusbar.pipeline.airplane.data.repository.FakeAirplaneModeRepository import com.android.systemui.statusbar.pipeline.mobile.data.MobileInputLogger import com.android.systemui.statusbar.pipeline.mobile.data.model.SubscriptionModel import com.android.systemui.statusbar.pipeline.mobile.data.repository.CarrierConfigRepository @@ -72,7 +73,6 @@ import kotlinx.coroutines.test.StandardTestDispatcher import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.runCurrent import kotlinx.coroutines.test.runTest -import kotlinx.coroutines.yield import org.junit.Assert.assertThrows import org.junit.Assert.assertTrue import org.junit.Before @@ -96,6 +96,7 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { private lateinit var carrierMergedFactory: CarrierMergedConnectionRepository.Factory private lateinit var fullConnectionFactory: FullMobileConnectionRepository.Factory private lateinit var connectivityRepository: ConnectivityRepository + private lateinit var airplaneModeRepository: FakeAirplaneModeRepository private lateinit var wifiRepository: WifiRepository private lateinit var carrierConfigRepository: CarrierConfigRepository @Mock private lateinit var connectivityManager: ConnectivityManager @@ -149,6 +150,8 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { mock(), ) + airplaneModeRepository = FakeAirplaneModeRepository() + wifiRepository = WifiRepositoryImpl( fakeBroadcastDispatcher, @@ -207,6 +210,7 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { context, dispatcher, testScope.backgroundScope, + airplaneModeRepository, wifiRepository, fullConnectionFactory, ) @@ -422,6 +426,7 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { testScope.runTest { collectLastValue(underTest.subscriptions) + getDefaultNetworkCallback().onCapabilitiesChanged(NETWORK, WIFI_NETWORK_CAPS_CM) getNormalNetworkCallback().onCapabilitiesChanged(NETWORK, WIFI_NETWORK_CAPS_CM) whenever(subscriptionManager.completeActiveSubscriptionInfoList) .thenReturn(listOf(SUB_CM)) @@ -438,6 +443,7 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { testScope.runTest { collectLastValue(underTest.subscriptions) + getDefaultNetworkCallback().onCapabilitiesChanged(NETWORK, WIFI_NETWORK_CAPS_CM) getNormalNetworkCallback().onCapabilitiesChanged(NETWORK, WIFI_NETWORK_CAPS_CM) whenever(subscriptionManager.completeActiveSubscriptionInfoList) .thenReturn(listOf(SUB_1, SUB_CM)) @@ -454,6 +460,7 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { testScope.runTest { collectLastValue(underTest.subscriptions) + getDefaultNetworkCallback().onCapabilitiesChanged(NETWORK, WIFI_NETWORK_CAPS_CM) getNormalNetworkCallback().onCapabilitiesChanged(NETWORK, WIFI_NETWORK_CAPS_CM) whenever(subscriptionManager.completeActiveSubscriptionInfoList) .thenReturn(listOf(SUB_1, SUB_CM)) @@ -465,6 +472,7 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { assertThat(mobileRepo.getIsCarrierMerged()).isFalse() // WHEN the wifi network updates to be not carrier merged + getDefaultNetworkCallback().onCapabilitiesChanged(NETWORK, WIFI_NETWORK_CAPS_ACTIVE) getNormalNetworkCallback().onCapabilitiesChanged(NETWORK, WIFI_NETWORK_CAPS_ACTIVE) runCurrent() @@ -480,6 +488,7 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { testScope.runTest { collectLastValue(underTest.subscriptions) + getDefaultNetworkCallback().onCapabilitiesChanged(NETWORK, WIFI_NETWORK_CAPS_ACTIVE) getNormalNetworkCallback().onCapabilitiesChanged(NETWORK, WIFI_NETWORK_CAPS_ACTIVE) whenever(subscriptionManager.completeActiveSubscriptionInfoList) .thenReturn(listOf(SUB_1, SUB_CM)) @@ -492,6 +501,7 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { assertThat(mobileRepo.getIsCarrierMerged()).isFalse() // WHEN the wifi network updates to be carrier merged + getDefaultNetworkCallback().onCapabilitiesChanged(NETWORK, WIFI_NETWORK_CAPS_CM) getNormalNetworkCallback().onCapabilitiesChanged(NETWORK, WIFI_NETWORK_CAPS_CM) runCurrent() @@ -531,6 +541,7 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { testScope.runTest { collectLastValue(underTest.subscriptions) + getDefaultNetworkCallback().onCapabilitiesChanged(NETWORK, WIFI_NETWORK_CAPS_CM) getNormalNetworkCallback().onCapabilitiesChanged(NETWORK, WIFI_NETWORK_CAPS_CM) whenever(subscriptionManager.completeActiveSubscriptionInfoList) .thenReturn(listOf(SUB_1, SUB_2, SUB_CM)) @@ -759,7 +770,10 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { fun hasCarrierMergedConnection_carrierMergedViaWifi_isTrue() = testScope.runTest { val carrierMergedInfo = - mock().apply { whenever(this.isCarrierMerged).thenReturn(true) } + mock().apply { + whenever(this.isCarrierMerged).thenReturn(true) + whenever(this.isPrimary).thenReturn(true) + } val caps = mock().also { whenever(it.hasTransport(TRANSPORT_WIFI)).thenReturn(true) @@ -769,7 +783,7 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { val latest by collectLastValue(underTest.hasCarrierMergedConnection) getDefaultNetworkCallback().onCapabilitiesChanged(NETWORK, caps) - yield() + getNormalNetworkCallback().onCapabilitiesChanged(NETWORK, caps) assertThat(latest).isTrue() } @@ -778,7 +792,10 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { fun hasCarrierMergedConnection_carrierMergedViaMobile_isTrue() = testScope.runTest { val carrierMergedInfo = - mock().apply { whenever(this.isCarrierMerged).thenReturn(true) } + mock().apply { + whenever(this.isCarrierMerged).thenReturn(true) + whenever(this.isPrimary).thenReturn(true) + } val caps = mock().also { whenever(it.hasTransport(TRANSPORT_CELLULAR)).thenReturn(true) @@ -788,7 +805,7 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { val latest by collectLastValue(underTest.hasCarrierMergedConnection) getDefaultNetworkCallback().onCapabilitiesChanged(NETWORK, caps) - yield() + getNormalNetworkCallback().onCapabilitiesChanged(NETWORK, caps) assertThat(latest).isTrue() } @@ -798,7 +815,10 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { fun hasCarrierMergedConnection_carrierMergedViaWifiWithVcnTransport_isTrue() = testScope.runTest { val carrierMergedInfo = - mock().apply { whenever(this.isCarrierMerged).thenReturn(true) } + mock().apply { + whenever(this.isCarrierMerged).thenReturn(true) + whenever(this.isPrimary).thenReturn(true) + } val caps = mock().also { whenever(it.hasTransport(TRANSPORT_WIFI)).thenReturn(true) @@ -808,7 +828,7 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { val latest by collectLastValue(underTest.hasCarrierMergedConnection) getDefaultNetworkCallback().onCapabilitiesChanged(NETWORK, caps) - yield() + getNormalNetworkCallback().onCapabilitiesChanged(NETWORK, caps) assertThat(latest).isTrue() } @@ -817,7 +837,10 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { fun hasCarrierMergedConnection_carrierMergedViaMobileWithVcnTransport_isTrue() = testScope.runTest { val carrierMergedInfo = - mock().apply { whenever(this.isCarrierMerged).thenReturn(true) } + mock().apply { + whenever(this.isCarrierMerged).thenReturn(true) + whenever(this.isPrimary).thenReturn(true) + } val caps = mock().also { whenever(it.hasTransport(TRANSPORT_CELLULAR)).thenReturn(true) @@ -827,7 +850,7 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { val latest by collectLastValue(underTest.hasCarrierMergedConnection) getDefaultNetworkCallback().onCapabilitiesChanged(NETWORK, caps) - yield() + getNormalNetworkCallback().onCapabilitiesChanged(NETWORK, caps) assertThat(latest).isTrue() } @@ -839,7 +862,10 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { val underlyingNetwork = mock() val carrierMergedInfo = - mock().apply { whenever(this.isCarrierMerged).thenReturn(true) } + mock().apply { + whenever(this.isCarrierMerged).thenReturn(true) + whenever(this.isPrimary).thenReturn(true) + } val underlyingWifiCapabilities = mock().also { whenever(it.hasTransport(TRANSPORT_WIFI)).thenReturn(true) @@ -858,7 +884,7 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { } getDefaultNetworkCallback().onCapabilitiesChanged(NETWORK, mainCapabilities) - yield() + getNormalNetworkCallback().onCapabilitiesChanged(NETWORK, mainCapabilities) // THEN there's a carrier merged connection assertThat(latest).isTrue() @@ -871,7 +897,10 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { val underlyingCarrierMergedNetwork = mock() val carrierMergedInfo = - mock().apply { whenever(this.isCarrierMerged).thenReturn(true) } + mock().apply { + whenever(this.isCarrierMerged).thenReturn(true) + whenever(this.isPrimary).thenReturn(true) + } val underlyingCapabilities = mock().also { whenever(it.hasTransport(TRANSPORT_CELLULAR)).thenReturn(true) @@ -891,7 +920,7 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { } getDefaultNetworkCallback().onCapabilitiesChanged(NETWORK, mainCapabilities) - yield() + getNormalNetworkCallback().onCapabilitiesChanged(NETWORK, mainCapabilities) // THEN there's a carrier merged connection assertThat(latest).isTrue() @@ -899,11 +928,11 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { /** Regression test for b/272586234. */ @Test - fun hasCarrierMergedConnection_defaultNotCarrierMerged_butWifiRepoHasCarrierMerged_isTrue() = + fun hasCarrierMergedConnection_defaultIsWifiNotCarrierMerged_wifiRepoIsCarrierMerged_isTrue() = testScope.runTest { val latest by collectLastValue(underTest.hasCarrierMergedConnection) - // WHEN the default callback isn't carrier merged + // WHEN the default callback is TRANSPORT_WIFI but not carrier merged val carrierMergedInfo = mock().apply { whenever(this.isCarrierMerged).thenReturn(false) } val caps = @@ -912,7 +941,6 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { whenever(it.transportInfo).thenReturn(carrierMergedInfo) } getDefaultNetworkCallback().onCapabilitiesChanged(NETWORK, caps) - yield() // BUT the wifi repo has gotten updates that it *is* carrier merged getNormalNetworkCallback().onCapabilitiesChanged(NETWORK, WIFI_NETWORK_CAPS_CM) @@ -921,6 +949,51 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { assertThat(latest).isTrue() } + /** Regression test for b/278618530. */ + @Test + fun hasCarrierMergedConnection_defaultIsCellular_wifiRepoIsCarrierMerged_isFalse() = + testScope.runTest { + val latest by collectLastValue(underTest.hasCarrierMergedConnection) + + // WHEN the default callback is TRANSPORT_CELLULAR and not carrier merged + val caps = + mock().also { + whenever(it.hasTransport(TRANSPORT_CELLULAR)).thenReturn(true) + whenever(it.transportInfo).thenReturn(null) + } + getDefaultNetworkCallback().onCapabilitiesChanged(NETWORK, caps) + + // BUT the wifi repo has gotten updates that it *is* carrier merged + getNormalNetworkCallback().onCapabilitiesChanged(NETWORK, WIFI_NETWORK_CAPS_CM) + + // THEN hasCarrierMergedConnection is **false** (The default network being CELLULAR + // takes precedence over the wifi network being carrier merged.) + assertThat(latest).isFalse() + } + + /** Regression test for b/278618530. */ + @Test + fun hasCarrierMergedConnection_defaultCellular_wifiIsCarrierMerged_airplaneMode_isTrue() = + testScope.runTest { + val latest by collectLastValue(underTest.hasCarrierMergedConnection) + + // WHEN the default callback is TRANSPORT_CELLULAR and not carrier merged + val caps = + mock().also { + whenever(it.hasTransport(TRANSPORT_CELLULAR)).thenReturn(true) + whenever(it.transportInfo).thenReturn(null) + } + getDefaultNetworkCallback().onCapabilitiesChanged(NETWORK, caps) + + // BUT the wifi repo has gotten updates that it *is* carrier merged + getNormalNetworkCallback().onCapabilitiesChanged(NETWORK, WIFI_NETWORK_CAPS_CM) + // AND we're in airplane mode + airplaneModeRepository.setIsAirplaneMode(true) + + // THEN hasCarrierMergedConnection is true. + assertThat(latest).isTrue() + } + @Test fun defaultConnectionIsValidated_startsAsFalse() { assertThat(underTest.defaultConnectionIsValidated.value).isFalse() @@ -978,6 +1051,7 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { context, dispatcher, testScope.backgroundScope, + airplaneModeRepository, wifiRepository, fullConnectionFactory, )