From 607985b57f9a402941e86d1b25be5f1b5e397ee9 Mon Sep 17 00:00:00 2001
From: junyulai <junyulai@google.com>
Date: Sat, 20 Feb 2021 11:18:30 +0800
Subject: [PATCH] Fix flaky test by extending timeout

The asserted capabilities change callback event took 170ms
in average to fire on cuttlefish, which causes 35% of
flakiness since the event might delay up to 500ms in practice.
Extend to 30s timeout value which is the standard value
of waiting for network callback events in CTS.

This change also ignore the test up to Android R since the API
is not available for R devices or below.

Test: atest CtsNetTestCases:android.net.cts.ConnectivityManagerTest#testRequestBackgroundNetwork \
      --rerun-until-failure 500
Test: atest CtsNetTestCasesLatestSdk:android.net.cts.ConnectivityManagerTest
      on R device
Bug: 179694867

Change-Id: Ic5e11d4a4f326150848bc48038fa01cde39f7661
---
 .../net/cts/ConnectivityManagerTest.java      | 21 ++++++++++++-------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java b/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java
index 831810c42a..ae3562f174 100644
--- a/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java
+++ b/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java
@@ -109,6 +109,8 @@ import androidx.test.InstrumentationRegistry;
 import androidx.test.runner.AndroidJUnit4;
 
 import com.android.internal.util.ArrayUtils;
+import com.android.testutils.DevSdkIgnoreRule;
+import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo;
 import com.android.testutils.RecorderCallback.CallbackEntry;
 import com.android.testutils.SkipPresubmit;
 import com.android.testutils.TestableNetworkCallback;
@@ -119,6 +121,7 @@ import libcore.io.Streams;
 
 import org.junit.After;
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
@@ -151,6 +154,8 @@ import java.util.regex.Pattern;
 
 @RunWith(AndroidJUnit4.class)
 public class ConnectivityManagerTest {
+    @Rule
+    public final DevSdkIgnoreRule ignoreRule = new DevSdkIgnoreRule();
 
     private static final String TAG = ConnectivityManagerTest.class.getSimpleName();
 
@@ -163,9 +168,7 @@ public class ConnectivityManagerTest {
     private static final int MAX_KEEPALIVE_RETRY_COUNT = 3;
     private static final int MIN_KEEPALIVE_INTERVAL = 10;
 
-    // Changing meteredness on wifi involves reconnecting, which can take several seconds (involves
-    // re-associating, DHCP...)
-    private static final int NETWORK_CHANGE_METEREDNESS_TIMEOUT = 30_000;
+    private static final int NETWORK_CALLBACK_TIMEOUT_MS = 30_000;
     private static final int NUM_TRIES_MULTIPATH_PREF_CHECK = 20;
     private static final long INTERVAL_MULTIPATH_PREF_CHECK_MS = 500;
     // device could have only one interface: data, wifi.
@@ -746,7 +749,9 @@ public class ConnectivityManagerTest {
         // with the current setting. Therefore, if the setting has already been changed,
         // this method will return right away, and if not it will wait for the setting to change.
         mCm.registerDefaultNetworkCallback(networkCallback);
-        if (!latch.await(NETWORK_CHANGE_METEREDNESS_TIMEOUT, TimeUnit.MILLISECONDS)) {
+        // Changing meteredness on wifi involves reconnecting, which can take several seconds
+        // (involves re-associating, DHCP...).
+        if (!latch.await(NETWORK_CALLBACK_TIMEOUT_MS, TimeUnit.MILLISECONDS)) {
             fail("Timed out waiting for active network metered status to change to "
                  + requestedMeteredness + " ; network = " + mCm.getActiveNetwork());
         }
@@ -1499,12 +1504,12 @@ public class ConnectivityManagerTest {
     }
 
     private void waitForAvailable(@NonNull final TestableNetworkCallback cb) {
-        cb.eventuallyExpect(CallbackEntry.AVAILABLE, AIRPLANE_MODE_CHANGE_TIMEOUT_MS,
+        cb.eventuallyExpect(CallbackEntry.AVAILABLE, NETWORK_CALLBACK_TIMEOUT_MS,
                 c -> c instanceof CallbackEntry.Available);
     }
 
     private void waitForLost(@NonNull final TestableNetworkCallback cb) {
-        cb.eventuallyExpect(CallbackEntry.LOST, AIRPLANE_MODE_CHANGE_TIMEOUT_MS,
+        cb.eventuallyExpect(CallbackEntry.LOST, NETWORK_CALLBACK_TIMEOUT_MS,
                 c -> c instanceof CallbackEntry.Lost);
     }
 
@@ -1556,8 +1561,8 @@ public class ConnectivityManagerTest {
      * Verify background request can only be requested when acquiring
      * {@link android.Manifest.permission.NETWORK_SETTINGS}.
      */
-    @SkipPresubmit(reason = "Flaky: b/179554972; add to presubmit after fixing")
     @Test
+    @IgnoreUpTo(Build.VERSION_CODES.R)
     public void testRequestBackgroundNetwork() throws Exception {
         // Create a tun interface. Use the returned interface name as the specifier to create
         // a test network request.
@@ -1608,7 +1613,7 @@ public class ConnectivityManagerTest {
             // background if no foreground request can be satisfied. Thus, wait for a short
             // period is needed to let foreground capability go away.
             callback.eventuallyExpect(CallbackEntry.NETWORK_CAPS_UPDATED,
-                    callback.getDefaultTimeoutMs(),
+                    NETWORK_CALLBACK_TIMEOUT_MS,
                     c -> c instanceof CallbackEntry.CapabilitiesChanged
                             && !((CallbackEntry.CapabilitiesChanged) c).getCaps()
                             .hasCapability(NET_CAPABILITY_FOREGROUND));