From d52096e1bb96350131785f923f91b3c2e3bfe331 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Fri, 25 Apr 2025 17:46:40 +0900 Subject: [PATCH 1/2] Enable the last preference tests Now that it's easy to expect events out of order, these tests can be implemented. Test: CSPreferenceTest Flag: EXEMPT test-only change Change-Id: If07fb6c11345be09c0942fd1c4371e8e22a4c6ec --- .../connectivityservice/CSPreferenceTest.kt | 39 ++++++++++++------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/tests/unit/java/com/android/server/connectivityservice/CSPreferenceTest.kt b/tests/unit/java/com/android/server/connectivityservice/CSPreferenceTest.kt index 05588f6076..1b7b1017ab 100644 --- a/tests/unit/java/com/android/server/connectivityservice/CSPreferenceTest.kt +++ b/tests/unit/java/com/android/server/connectivityservice/CSPreferenceTest.kt @@ -48,6 +48,7 @@ import com.android.testutils.TestableNetworkOfferCallback.CallbackEntry.Needed import com.android.testutils.TestableNetworkOfferCallback.CallbackEntry.Unneeded import com.android.testutils.postAndWait import com.android.testutils.runAsShell +import kotlin.test.assertEquals import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith @@ -329,12 +330,6 @@ class CSPreferenceTest : CSTest() { satelliteCallback.assertNoCallback() restrictedSatelliteCallback.assertNoCallback() - // TODO : the test below cannot be stabilized because the order of the callbacks - // (which request is needed first) is not guaranteed and actually changes across - // invocations, and TestableNetworkOfferCallback does not yet offer the tooling - // to check the track record from a mark. - return - updateSatellitePreference(setOf(SMSUID), setOf(UID1)) // TODO : ideally ConnectivityService would not send unneeded then needed. This is // happening because updating the preferences removes the requests, which causes a @@ -342,15 +337,20 @@ class CSPreferenceTest : CSTest() { // causes another rematch. satelliteCallback.expect { it.isRestricted } restrictedSatelliteCallback.expect { it.isRestricted } - satelliteCallback.expect { !it.isRestricted } - satelliteCallback.expect { it.isRestricted } + + val mark1 = satelliteCallback.mark + satelliteCallback.eventuallyExpect(from = mark1) { !it.isRestricted } + satelliteCallback.eventuallyExpect(from = mark1) { it.isRestricted } + assertEquals(2, satelliteCallback.mark - mark1) restrictedSatelliteCallback.expect { it.isRestricted } satelliteCallback.assertNoCallback() restrictedSatelliteCallback.assertNoCallback() updateSatellitePreference(emptySet(), setOf(UID1)) - satelliteCallback.expect { it.isRestricted } - satelliteCallback.expect { !it.isRestricted } + val mark2 = satelliteCallback.mark + satelliteCallback.eventuallyExpect(from = mark2) { it.isRestricted } + satelliteCallback.eventuallyExpect(from = mark2) { !it.isRestricted } + assertEquals(2, satelliteCallback.mark - mark2) satelliteCallback.expect { !it.isRestricted } restrictedSatelliteCallback.expect { it.isRestricted } satelliteCallback.assertNoCallback() @@ -358,20 +358,29 @@ class CSPreferenceTest : CSTest() { updateSatellitePreference(setOf(SMSUID), setOf(UID1)) restrictedSatelliteCallback.expect { it.isRestricted } + satelliteCallback.expect { !it.isRestricted } + val mark3 = satelliteCallback.mark + satelliteCallback.eventuallyExpect(from = mark3) { it.isRestricted } + satelliteCallback.eventuallyExpect(from = mark3) { !it.isRestricted } + assertEquals(2, satelliteCallback.mark - mark3) satelliteCallback.assertNoCallback() restrictedSatelliteCallback.assertNoCallback() - val wifiAgent = Agent(TRANSPORT_WIFI) + val wifiAgent = Agent(nc(TRANSPORT_WIFI, NET_CAPABILITY_INTERNET)) wifiAgent.connect() - satelliteCallback.expect { it.isRestricted } - satelliteCallback.expect { !it.isRestricted } + val mark4 = satelliteCallback.mark + satelliteCallback.eventuallyExpect(from = mark4) { it.isRestricted } + satelliteCallback.eventuallyExpect(from = mark4) { !it.isRestricted } + assertEquals(2, satelliteCallback.mark - mark4) restrictedSatelliteCallback.expect { it.isRestricted } satelliteCallback.assertNoCallback() restrictedSatelliteCallback.assertNoCallback() wifiAgent.disconnect() - satelliteCallback.expect { it.isRestricted } - satelliteCallback.expect { !it.isRestricted } + val mark5 = satelliteCallback.mark + satelliteCallback.eventuallyExpect(from = mark5) { it.isRestricted } + satelliteCallback.eventuallyExpect(from = mark5) { !it.isRestricted } + assertEquals(2, satelliteCallback.mark - mark5) restrictedSatelliteCallback.expect { it.isRestricted } satelliteCallback.assertNoCallback() restrictedSatelliteCallback.assertNoCallback() From c96d1ae0e6e83167b3dd9cdfae322ca43db99c8e Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Sun, 27 Apr 2025 11:01:10 +0900 Subject: [PATCH 2/2] Move failWithErrorReason to a common place MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is a shared public utility. Its main utility is for methods that can fail, have an error message of their own, and also want to take an error message from a caller and want to combine them. Example use : fun assertEquals(expected: T, actual: T, msg: String? = null) { if (expected != null) { failWithErrorReason(msg, "Expected $expected, got $actual") } } ...which allows someone to say assertEquals(a, b, "Checking a vs b") Test: TH Flag: EXEMPT test-only change Change-Id: Id3eaa8384b4177f38f13576d9b4d742c52027ae4 --- .../com/android/testutils/TestableNetworkCallback.kt | 8 +------- .../android/testutils/TestableNetworkOfferCallback.kt | 6 ------ .../hostdevice/com/android/testutils/MiscAsserts.kt | 10 +++++++++- 3 files changed, 10 insertions(+), 14 deletions(-) diff --git a/staticlibs/testutils/devicetests/com/android/testutils/TestableNetworkCallback.kt b/staticlibs/testutils/devicetests/com/android/testutils/TestableNetworkCallback.kt index d9c51e5201..c4e236e1d0 100644 --- a/staticlibs/testutils/devicetests/com/android/testutils/TestableNetworkCallback.kt +++ b/staticlibs/testutils/devicetests/com/android/testutils/TestableNetworkCallback.kt @@ -70,7 +70,7 @@ open class RecorderCallback private constructor( data class Reserved private constructor( override val network: Network, val caps: NetworkCapabilities - ): CallbackEntry() { + ) : CallbackEntry() { constructor(caps: NetworkCapabilities) : this(NULL_NETWORK, caps) } data class Available(override val network: Network) : CallbackEntry() @@ -399,12 +399,6 @@ open class TestableNetworkCallback private constructor( } } as T - // "Nothing" is the return type to declare a function never returns a value. - fun failWithErrorReason(errorMsg: String?, errorReason: String): Nothing { - val message = if (errorMsg != null) "$errorMsg : $errorReason" else errorReason - fail(message) - } - inline fun expect( network: HasNetwork, timeoutMs: Long = defaultTimeoutMs, diff --git a/staticlibs/testutils/devicetests/com/android/testutils/TestableNetworkOfferCallback.kt b/staticlibs/testutils/devicetests/com/android/testutils/TestableNetworkOfferCallback.kt index 8f9014c816..8228440f54 100644 --- a/staticlibs/testutils/devicetests/com/android/testutils/TestableNetworkOfferCallback.kt +++ b/staticlibs/testutils/devicetests/com/android/testutils/TestableNetworkOfferCallback.kt @@ -56,12 +56,6 @@ class TestableNetworkOfferCallback( history.add(CallbackEntry.Unneeded(request)) } - // TODO : move to some utility, this is copied from TestableNetworkCallback - fun failWithErrorReason(errorMsg: String?, errorReason: String): Nothing { - val message = if (errorMsg != null) "$errorMsg : $errorReason" else errorReason - fail(message) - } - inline fun expect( errorMsg: String? = null, predicate: (T) -> Boolean = { true } diff --git a/staticlibs/testutils/hostdevice/com/android/testutils/MiscAsserts.kt b/staticlibs/testutils/hostdevice/com/android/testutils/MiscAsserts.kt index 176546a4c0..de797b0ba4 100644 --- a/staticlibs/testutils/hostdevice/com/android/testutils/MiscAsserts.kt +++ b/staticlibs/testutils/hostdevice/com/android/testutils/MiscAsserts.kt @@ -142,4 +142,12 @@ fun assertEventuallyTrue( } Thread.sleep(pollIntervalMs) } -} \ No newline at end of file +} + +// "Nothing" is the return type to declare a function never returns a value. This is useful +// because the compiler will know the branch of the code calling this does not return, which +// lets check exhaustivity, infer return types, nullability and smart casts. +fun failWithErrorReason(errorMsg: String?, errorReason: String): Nothing { + val message = if (errorMsg != null) "$errorMsg : $errorReason" else errorReason + fail(message) +}