From c2cf7eb82d2bb76d07e9a29b131deaba6bb13dd0 Mon Sep 17 00:00:00 2001
From: Chalard Jean <jchalard@google.com>
Date: Mon, 27 Feb 2023 19:13:46 +0900
Subject: [PATCH] Remove pollOrThrow
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This method is better removed, because it doesn't follow the
naming conventions we want to keep in this file. It also isn't
useful :
• There is only one use of it outside tests, and it's in the
  same file. It's trivial to inline it.
• The method has a unit test, which becomes useless once the
  method is inlined.
• There is a corresponding command in the interpreter, but it's
  only used by the unit test and therefore can be removed once
  the unit test is removed.

Test: TH
Bug: 157405399
Change-Id: Ib5b1369ac28e4577adc98204e42ff2f27729e64c
---
 .../testutils/TestableNetworkCallbackTest.kt  | 13 --------
 .../testutils/TestableNetworkCallback.kt      | 31 ++++++-------------
 2 files changed, 10 insertions(+), 34 deletions(-)

diff --git a/staticlibs/tests/unit/src/com/android/testutils/TestableNetworkCallbackTest.kt b/staticlibs/tests/unit/src/com/android/testutils/TestableNetworkCallbackTest.kt
index ec7cdbdfee..4ed881af33 100644
--- a/staticlibs/tests/unit/src/com/android/testutils/TestableNetworkCallbackTest.kt
+++ b/staticlibs/tests/unit/src/com/android/testutils/TestableNetworkCallbackTest.kt
@@ -365,18 +365,6 @@ class TestableNetworkCallbackTest {
         """)
     }
 
-    @Test
-    fun testPollOrThrow() {
-        assertFails { mCallback.pollOrThrow(SHORT_TIMEOUT_MS) }
-        TNCInterpreter.interpretTestSpec(initial = mCallback, lineShift = 1,
-                threadTransform = { cb -> cb.createLinkedCopy() }, spec = """
-            sleep; onAvailable(133)    | pollOrThrow(2) = Available(133) time 1..4
-                                       | pollOrThrow(1) fails
-            onCapabilitiesChanged(108) | pollOrThrow(1) = CapabilitiesChanged(108) time 0..3
-            onBlockedStatus(199)       | pollOrThrow(1) = BlockedStatus(199) time 0..3
-        """)
-    }
-
     @Test
     fun testEventuallyExpect() {
         // TODO: Current test does not verify the inline one. Also verify the behavior after
@@ -455,7 +443,6 @@ private val interpretTable = listOf<InterpretMatcher<TestableNetworkCallback>>(
         }
     },
     Regex("""poll\((\d+)\)""") to { i, cb, t -> cb.poll(t.timeArg(1)) },
-    Regex("""pollOrThrow\((\d+)\)""") to { i, cb, t -> cb.pollOrThrow(t.timeArg(1)) },
     // Interpret "eventually(Available(xx), timeout)" as calling eventuallyExpect that expects
     // CallbackEntry.AVAILABLE with netId of xx within timeout*INTERPRET_TIME_UNIT timeout, and
     // likewise for all callback types.
diff --git a/staticlibs/testutils/devicetests/com/android/testutils/TestableNetworkCallback.kt b/staticlibs/testutils/devicetests/com/android/testutils/TestableNetworkCallback.kt
index cb3be45bc3..0e73112d1b 100644
--- a/staticlibs/testutils/devicetests/com/android/testutils/TestableNetworkCallback.kt
+++ b/staticlibs/testutils/devicetests/com/android/testutils/TestableNetworkCallback.kt
@@ -223,18 +223,6 @@ open class TestableNetworkCallback private constructor(
     fun poll(timeoutMs: Long = defaultTimeoutMs, predicate: (CallbackEntry) -> Boolean = { true }) =
             history.poll(timeoutMs, predicate)
 
-    /**
-     * Get the next callback or throw if timeout.
-     *
-     * With no argument, this method waits out the default timeout. To wait forever, pass
-     * Long.MAX_VALUE.
-     */
-    @JvmOverloads
-    fun pollOrThrow(
-        timeoutMs: Long = defaultTimeoutMs,
-        errorMsg: String = "Did not receive callback after $timeoutMs"
-    ): CallbackEntry = poll(timeoutMs) ?: fail(errorMsg)
-
     /*****
      * expect family of methods.
      * These methods fetch the next callback and assert it matches the conditions : type,
@@ -350,15 +338,16 @@ open class TestableNetworkCallback private constructor(
         timeoutMs: Long = defaultTimeoutMs,
         errorMsg: String? = null,
         test: (T) -> Boolean = { true }
-    ) = pollOrThrow(timeoutMs, "Did not receive ${T::class.simpleName} after ${timeoutMs}ms").also {
-        if (it !is T) fail("Expected callback ${T::class.simpleName}, got $it")
-        if (ANY_NETWORK !== network && it.network != network) {
-            fail("Expected network $network for callback : $it")
-        }
-        if (!test(it)) {
-            fail("${errorMsg ?: "Callback doesn't match predicate"} : $it")
-        }
-    } as T
+    ) = (poll(timeoutMs) ?: fail("Did not receive ${T::class.simpleName} after ${timeoutMs}ms"))
+            .also {
+                if (it !is T) fail("Expected callback ${T::class.simpleName}, got $it")
+                if (ANY_NETWORK !== network && it.network != network) {
+                    fail("Expected network $network for callback : $it")
+                }
+                if (!test(it)) {
+                    fail("${errorMsg ?: "Callback doesn't match predicate"} : $it")
+                }
+            } as T
 
     inline fun <reified T : CallbackEntry> expect(
         network: HasNetwork,