diff --git a/staticlibs/testutils/devicetests/com/android/testutils/TestableNetworkOfferCallback.kt b/staticlibs/testutils/devicetests/com/android/testutils/TestableNetworkOfferCallback.kt index a0078d21d7..8f9014c816 100644 --- a/staticlibs/testutils/devicetests/com/android/testutils/TestableNetworkOfferCallback.kt +++ b/staticlibs/testutils/devicetests/com/android/testutils/TestableNetworkOfferCallback.kt @@ -23,14 +23,20 @@ import android.util.Log import com.android.net.module.util.ArrayTrackRecord import kotlin.test.fail -class TestableNetworkOfferCallback(val timeoutMs: Long, private val noCallbackTimeoutMs: Long) - : NetworkProvider.NetworkOfferCallback { +private const val DEFAULT_TIMEOUT = 30_000L // ms +private const val DEFAULT_NO_CALLBACK_TIMEOUT = 200L // ms + +class TestableNetworkOfferCallback( + val timeoutMs: Long = DEFAULT_TIMEOUT, + private val noCallbackTimeoutMs: Long = DEFAULT_NO_CALLBACK_TIMEOUT, +) : NetworkProvider.NetworkOfferCallback { private val TAG = this::class.simpleName val history = ArrayTrackRecord().newReadHead() + val mark get() = history.mark sealed class CallbackEntry { - data class OnNetworkNeeded(val request: NetworkRequest) : CallbackEntry() - data class OnNetworkUnneeded(val request: NetworkRequest) : CallbackEntry() + data class Needed(val request: NetworkRequest) : CallbackEntry() + data class Unneeded(val request: NetworkRequest) : CallbackEntry() } /** @@ -39,7 +45,7 @@ class TestableNetworkOfferCallback(val timeoutMs: Long, private val noCallbackTi */ override fun onNetworkNeeded(request: NetworkRequest) { Log.d(TAG, "onNetworkNeeded $request") - history.add(CallbackEntry.OnNetworkNeeded(request)) + history.add(CallbackEntry.Needed(request)) } /** @@ -47,25 +53,59 @@ class TestableNetworkOfferCallback(val timeoutMs: Long, private val noCallbackTi */ override fun onNetworkUnneeded(request: NetworkRequest) { Log.d(TAG, "onNetworkUnneeded $request") - history.add(CallbackEntry.OnNetworkUnneeded(request)) + 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 } + ): T { + val nextEntry = history.poll(timeoutMs) ?: failWithErrorReason(errorMsg, + "Did not receive ${T::class.simpleName} after ${timeoutMs}ms") + if (nextEntry !is T) { + failWithErrorReason( + errorMsg, + "Expected callback ${T::class.simpleName}, got $nextEntry" + ) + } + if (!predicate(nextEntry)) { + failWithErrorReason(errorMsg, "Callback doesn't match predicate: $nextEntry") + } + return nextEntry + } + + // TODO : remove when there are no callers + @Deprecated("Use expect instead of expectCallbackThat") inline fun expectCallbackThat( crossinline predicate: (T) -> Boolean - ): T { - val event = history.poll(timeoutMs) - ?: fail("Did not receive callback after ${timeoutMs}ms") - if (event !is T || !predicate(event)) fail("Received unexpected callback $event") - return event - } + ): T = expect(null, predicate) + + inline fun eventuallyExpect( + errorMsg: String? = null, + from: Int = mark, + crossinline predicate: (T) -> Boolean = { true } + ) = history.poll(timeoutMs, from) { it is T && predicate(it) }.also { + if (null == it) { + failWithErrorReason( + errorMsg, + "Callback ${T::class} not received within ${timeoutMs}ms." + ) + } + } as T fun expectOnNetworkNeeded(capabilities: NetworkCapabilities) = - expectCallbackThat { + expectCallbackThat { it.request.canBeSatisfiedBy(capabilities) } fun expectOnNetworkUnneeded(capabilities: NetworkCapabilities) = - expectCallbackThat { + expectCallbackThat { it.request.canBeSatisfiedBy(capabilities) } @@ -73,4 +113,4 @@ class TestableNetworkOfferCallback(val timeoutMs: Long, private val noCallbackTi val cb = history.poll(noCallbackTimeoutMs) if (null != cb) fail("Expected no callback but got $cb") } -} \ No newline at end of file +} diff --git a/staticlibs/testutils/hostdevice/com/android/net/module/util/TrackRecord.kt b/staticlibs/testutils/hostdevice/com/android/net/module/util/TrackRecord.kt index 8051558f1c..541fccad1f 100644 --- a/staticlibs/testutils/hostdevice/com/android/net/module/util/TrackRecord.kt +++ b/staticlibs/testutils/hostdevice/com/android/net/module/util/TrackRecord.kt @@ -255,7 +255,7 @@ class ArrayTrackRecord : TrackRecord { * the last element in the queue if null is returned. This means this method will always * skip elements that do not match the predicate, even if it returns null. * - * This method can only be used by the thread that created this ManagedRecordingQueue. + * This method can only be used by the thread that created this ReadHead. * If used on another thread, this throws IllegalStateException. * * @param timeoutMs how long, in milliseconds, to wait at most (best effort approximation). @@ -277,6 +277,55 @@ class ArrayTrackRecord : TrackRecord { } } + /** + * Returns the first element after the mark, optionally blocking until one is available, + * or null if no such element can be found within the timeout. + * If a predicate is given, only elements matching the predicate are returned. + * + * Upon return, the mark will be advanced up to the point of the returned element (or + * after the last element in the queue if not found) if and only if that element was + * found after the current mark. This means this method will always skip elements that + * do not match the predicate even if it returns null, and repeated calls with the + * same position will advance the mark up to the last of the found elements. + * + * This method can only be used by the thread that created this ReadHead. + * If used on another thread, this throws IllegalStateException. + * + * Typical usage would be to check for multiple elements in any order. For example, + * val mark = readHead.mark + * readHead.poll(timeout, mark) { it matches condition 1 } + * readHead.poll(timeout, mark) { it matches condition 2 } + * readHead.poll(timeout, mark) { it matches condition 3 } + * The above would check that starting at the mark, there are three events, one matching + * each condition, in any order. A caller could further check that there were no other + * event beside the 3 checked with something like + * assertEquals(readHead.mark - mark, 3) + * + * @param timeoutMs how long, in milliseconds, to wait at most (best effort approximation). + * @param pos the position in the underlying track record where to start the search. + * @param predicate a predicate to filter elements to be returned. + * @return an element matching the predicate, or null if timeout. + */ + override fun poll(timeoutMs: Long, pos: Int, predicate: (E) -> Boolean): E? { + val stamp = slock.tryWriteLock() + if (0L == stamp) concurrentAccessDetected() + pollMark = readHead + try { + lock.withLock { + val index = pollForIndexReadLocked(timeoutMs, pos, predicate) + when { + index < 0 -> readHead = size + index > readHead -> readHead = index + 1 + // else do nothing, the element was found prior to the read head so + // don't move the read head + } + return getOrNull(index) + } + } finally { + slock.unlockWrite(stamp) + } + } + /** * Returns a list of events that were observed since before the last poll() operation on * this ReadHead. diff --git a/tests/cts/net/src/android/net/cts/NetworkValidationTest.kt b/tests/cts/net/src/android/net/cts/NetworkValidationTest.kt index aad072c643..4261881046 100644 --- a/tests/cts/net/src/android/net/cts/NetworkValidationTest.kt +++ b/tests/cts/net/src/android/net/cts/NetworkValidationTest.kt @@ -177,10 +177,11 @@ class NetworkValidationTest { cm.registerNetworkCallback(ethRequest, testCb) try { - val ncCb = testCb.eventuallyExpect { + val mark = testCb.mark + val ncCb = testCb.eventuallyExpect(from = mark) { it.caps.hasCapability(NET_CAPABILITY_CAPTIVE_PORTAL) } - testCb.eventuallyExpect { + testCb.eventuallyExpect(from = mark) { it.network == ncCb.network && it.lp.captivePortalData != null }.lp } finally { diff --git a/tests/unit/java/com/android/server/connectivityservice/CSNetworkReservationTest.kt b/tests/unit/java/com/android/server/connectivityservice/CSNetworkReservationTest.kt index e698930e3a..a509d1ecc8 100644 --- a/tests/unit/java/com/android/server/connectivityservice/CSNetworkReservationTest.kt +++ b/tests/unit/java/com/android/server/connectivityservice/CSNetworkReservationTest.kt @@ -36,7 +36,7 @@ import com.android.testutils.RecorderCallback.CallbackEntry.Reserved import com.android.testutils.RecorderCallback.CallbackEntry.Unavailable import com.android.testutils.TestableNetworkCallback import com.android.testutils.TestableNetworkOfferCallback -import com.android.testutils.TestableNetworkOfferCallback.CallbackEntry.OnNetworkNeeded +import com.android.testutils.TestableNetworkOfferCallback.CallbackEntry.Needed import kotlin.test.assertEquals import kotlin.test.assertNull import org.junit.Before @@ -134,7 +134,7 @@ class CSNetworkReservationTest : CSTest() { // validate the offer does not receive onNetworkNeeded for reservation request offerCb.expectNoCallbackWhere { - it is OnNetworkNeeded && it.request.type == NetworkRequest.Type.RESERVATION + it is Needed && it.request.type == NetworkRequest.Type.RESERVATION } } diff --git a/tests/unit/java/com/android/server/connectivityservice/CSPreferenceTest.kt b/tests/unit/java/com/android/server/connectivityservice/CSPreferenceTest.kt index ac3c859722..05588f6076 100644 --- a/tests/unit/java/com/android/server/connectivityservice/CSPreferenceTest.kt +++ b/tests/unit/java/com/android/server/connectivityservice/CSPreferenceTest.kt @@ -44,8 +44,8 @@ import com.android.testutils.DevSdkIgnoreRunner import com.android.testutils.RecorderCallback.CallbackEntry.Lost import com.android.testutils.TestableNetworkCallback import com.android.testutils.TestableNetworkOfferCallback -import com.android.testutils.TestableNetworkOfferCallback.CallbackEntry.OnNetworkNeeded -import com.android.testutils.TestableNetworkOfferCallback.CallbackEntry.OnNetworkUnneeded +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 org.junit.Rule @@ -276,9 +276,9 @@ class CSPreferenceTest : CSTest() { fun testOptInSatellitePreference_NetworkConnectedFirst_RestrictedSatellite() = doTestSatellitePreference_NetworkConnectedFirst(satelliteRestricted = true) - private val OnNetworkNeeded.isRestricted + private val Needed.isRestricted get() = !request.hasCapability(NET_CAPABILITY_NOT_RESTRICTED) - private val OnNetworkUnneeded.isRestricted + private val Unneeded.isRestricted get() = !request.hasCapability(NET_CAPABILITY_NOT_RESTRICTED) /** @@ -297,20 +297,14 @@ class CSPreferenceTest : CSTest() { val provider = NetworkProvider(context, csHandlerThread.looper, "Test provider") cm.registerNetworkProvider(provider) - val satelliteCallback = TestableNetworkOfferCallback( - DEFAULT_TIMEOUT_MS, - DEFAULT_NO_CALLBACK_TIMEOUT_MS - ) + val satelliteCallback = TestableNetworkOfferCallback() provider.registerNetworkOffer( NetworkScore.Builder().build(), satelliteNc(restricted = false), Runnable::run, satelliteCallback ) - val restrictedSatelliteCallback = TestableNetworkOfferCallback( - DEFAULT_TIMEOUT_MS, - DEFAULT_NO_CALLBACK_TIMEOUT_MS - ) + val restrictedSatelliteCallback = TestableNetworkOfferCallback() provider.registerNetworkOffer( NetworkScore.Builder().build(), satelliteNc(restricted = true), @@ -320,18 +314,18 @@ class CSPreferenceTest : CSTest() { satelliteCallback.assertNoCallback() updateSatellitePreference(emptySet(), setOf(UID1)) - satelliteCallback.expectCallbackThat { true } + satelliteCallback.expect() satelliteCallback.assertNoCallback() restrictedSatelliteCallback.assertNoCallback() updateSatellitePreference(emptySet(), emptySet()) - satelliteCallback.expectCallbackThat { true } + satelliteCallback.expect() satelliteCallback.assertNoCallback() restrictedSatelliteCallback.assertNoCallback() updateSatellitePreference(setOf(SMSUID), emptySet()) - satelliteCallback.expectCallbackThat { it.isRestricted } - restrictedSatelliteCallback.expectCallbackThat { it.isRestricted } + satelliteCallback.expect { it.isRestricted } + restrictedSatelliteCallback.expect { it.isRestricted } satelliteCallback.assertNoCallback() restrictedSatelliteCallback.assertNoCallback() @@ -346,39 +340,39 @@ class CSPreferenceTest : CSTest() { // happening because updating the preferences removes the requests, which causes a // rematch (where the requests are not registered), then adds the requests again, which // causes another rematch. - satelliteCallback.expectCallbackThat { it.isRestricted } - restrictedSatelliteCallback.expectCallbackThat { it.isRestricted } - satelliteCallback.expectCallbackThat { !it.isRestricted } - satelliteCallback.expectCallbackThat { it.isRestricted } - restrictedSatelliteCallback.expectCallbackThat { it.isRestricted } + satelliteCallback.expect { it.isRestricted } + restrictedSatelliteCallback.expect { it.isRestricted } + satelliteCallback.expect { !it.isRestricted } + satelliteCallback.expect { it.isRestricted } + restrictedSatelliteCallback.expect { it.isRestricted } satelliteCallback.assertNoCallback() restrictedSatelliteCallback.assertNoCallback() updateSatellitePreference(emptySet(), setOf(UID1)) - satelliteCallback.expectCallbackThat { it.isRestricted } - satelliteCallback.expectCallbackThat { !it.isRestricted } - satelliteCallback.expectCallbackThat { !it.isRestricted } - restrictedSatelliteCallback.expectCallbackThat { it.isRestricted } + satelliteCallback.expect { it.isRestricted } + satelliteCallback.expect { !it.isRestricted } + satelliteCallback.expect { !it.isRestricted } + restrictedSatelliteCallback.expect { it.isRestricted } satelliteCallback.assertNoCallback() restrictedSatelliteCallback.assertNoCallback() updateSatellitePreference(setOf(SMSUID), setOf(UID1)) - restrictedSatelliteCallback.expectCallbackThat { it.isRestricted } + restrictedSatelliteCallback.expect { it.isRestricted } satelliteCallback.assertNoCallback() restrictedSatelliteCallback.assertNoCallback() val wifiAgent = Agent(TRANSPORT_WIFI) wifiAgent.connect() - satelliteCallback.expectCallbackThat { it.isRestricted } - satelliteCallback.expectCallbackThat { !it.isRestricted } - restrictedSatelliteCallback.expectCallbackThat { it.isRestricted } + satelliteCallback.expect { it.isRestricted } + satelliteCallback.expect { !it.isRestricted } + restrictedSatelliteCallback.expect { it.isRestricted } satelliteCallback.assertNoCallback() restrictedSatelliteCallback.assertNoCallback() wifiAgent.disconnect() - satelliteCallback.expectCallbackThat { it.isRestricted } - satelliteCallback.expectCallbackThat { !it.isRestricted } - restrictedSatelliteCallback.expectCallbackThat { it.isRestricted } + satelliteCallback.expect { it.isRestricted } + satelliteCallback.expect { !it.isRestricted } + restrictedSatelliteCallback.expect { it.isRestricted } satelliteCallback.assertNoCallback() restrictedSatelliteCallback.assertNoCallback() }