From d09bb0773ab7182a2f6303144208dbcf19218e79 Mon Sep 17 00:00:00 2001
From: Hungming Chen <nuccachen@google.com>
Date: Mon, 29 Aug 2022 21:43:55 +0800
Subject: [PATCH] EthernetTetheringTest: toggle wifi while test network is not
 selected

Used to avoid flaky test because upstream changed events
order can't be guaranteed. Once tethering choose non-test
upstream {wifi, ..}, test network wont't be chosen.

Fix flaky tests which are using initTetheringTester():
testIcmpv6Echo
testTetherClatUdp
testTetherUdpV4_VerifyBpf
testTetherUdpV4
testTetherUdpV4Dns
testTetherUdpV6

Moreover, add permission ACCESS_WIFI_STATE to AndroidManifest.xml
to avoid nested runAsShell while using CtsNetUtils to access WIFI.

STACKTRACE:
java.lang.IllegalStateException: adoptShellPermissionIdentity calls must not be nested
at com.android.testutils.TestPermissionUtil.runAsShell(TestPermissionUtil.kt:49)
at com.android.testutils.TestPermissionUtil.runAsShell(TestPermissionUtil.kt:70)
at com.android.testutils.TestPermissionUtil.runAsShell$default(TestPermissionUtil.kt:65)
at com.android.testutils.TestPermissionUtil.runAsShell(Unknown Source:16)
at android.net.cts.util.CtsNetUtils.disconnectFromWifi(CtsNetUtils.java:293)
at android.net.cts.util.CtsNetUtils.disconnectFromWifi(CtsNetUtils.java:255)
at android.net.cts.util.CtsNetUtils.toggleWifi(CtsNetUtils.java:160)

Bug: 243314243
Test: atest EthernetTetheringTest
Change-Id: I814899ae71f5c302d490a0dd81d7210ac138faa5
---
 Tethering/tests/integration/Android.bp        |  1 +
 .../tests/integration/AndroidManifest.xml     |  1 +
 .../android/net/EthernetTetheringTest.java    | 77 +++++++++++++++++--
 3 files changed, 74 insertions(+), 5 deletions(-)

diff --git a/Tethering/tests/integration/Android.bp b/Tethering/tests/integration/Android.bp
index 9aa2cff821..11e3dc07fe 100644
--- a/Tethering/tests/integration/Android.bp
+++ b/Tethering/tests/integration/Android.bp
@@ -28,6 +28,7 @@ java_defaults {
     static_libs: [
         "NetworkStackApiStableLib",
         "androidx.test.rules",
+        "cts-net-utils",
         "mockito-target-extended-minus-junit4",
         "net-tests-utils",
         "net-utils-device-common-bpf",
diff --git a/Tethering/tests/integration/AndroidManifest.xml b/Tethering/tests/integration/AndroidManifest.xml
index 9303d0a20b..7527913a4c 100644
--- a/Tethering/tests/integration/AndroidManifest.xml
+++ b/Tethering/tests/integration/AndroidManifest.xml
@@ -17,6 +17,7 @@
           package="com.android.networkstack.tethering.tests.integration">
 
     <uses-permission android:name="android.permission.ACCESS_NETWORK_STATE"/>
+    <uses-permission android:name="android.permission.ACCESS_WIFI_STATE"/>
     <!-- The test need CHANGE_NETWORK_STATE permission to use requestNetwork API to setup test
          network. Since R shell application don't have such permission, grant permission to the test
          here. TODO: Remove CHANGE_NETWORK_STATE permission here and use adopt shell perssion to
diff --git a/Tethering/tests/integration/src/android/net/EthernetTetheringTest.java b/Tethering/tests/integration/src/android/net/EthernetTetheringTest.java
index 880a285774..b8df272491 100644
--- a/Tethering/tests/integration/src/android/net/EthernetTetheringTest.java
+++ b/Tethering/tests/integration/src/android/net/EthernetTetheringTest.java
@@ -20,6 +20,7 @@ import static android.Manifest.permission.DUMP;
 import static android.Manifest.permission.MANAGE_TEST_NETWORKS;
 import static android.Manifest.permission.NETWORK_SETTINGS;
 import static android.Manifest.permission.TETHER_PRIVILEGED;
+import static android.content.pm.PackageManager.FEATURE_WIFI;
 import static android.net.InetAddresses.parseNumericAddress;
 import static android.net.TetheringManager.CONNECTIVITY_SCOPE_GLOBAL;
 import static android.net.TetheringManager.CONNECTIVITY_SCOPE_LOCAL;
@@ -54,12 +55,14 @@ import static org.junit.Assume.assumeTrue;
 
 import android.app.UiAutomation;
 import android.content.Context;
+import android.content.pm.PackageManager;
 import android.net.EthernetManager.TetheredInterfaceCallback;
 import android.net.EthernetManager.TetheredInterfaceRequest;
 import android.net.TetheringManager.StartTetheringCallback;
 import android.net.TetheringManager.TetheringEventCallback;
 import android.net.TetheringManager.TetheringRequest;
 import android.net.TetheringTester.TetheredDevice;
+import android.net.cts.util.CtsNetUtils;;
 import android.os.Build;
 import android.os.Handler;
 import android.os.HandlerThread;
@@ -234,6 +237,8 @@ public class EthernetTetheringTest {
     private final Context mContext = InstrumentationRegistry.getContext();
     private final EthernetManager mEm = mContext.getSystemService(EthernetManager.class);
     private final TetheringManager mTm = mContext.getSystemService(TetheringManager.class);
+    private final PackageManager mPackageManager = mContext.getPackageManager();
+    private final CtsNetUtils mCtsNetUtils = new CtsNetUtils(mContext);
 
     private TestNetworkInterface mDownstreamIface;
     private HandlerThread mHandlerThread;
@@ -690,10 +695,17 @@ public class EthernetTetheringTest {
             }
         }
 
-        public Network awaitUpstreamChanged() throws Exception {
+        public Network awaitUpstreamChanged(boolean throwTimeoutException) throws Exception {
             if (!mUpstreamLatch.await(TIMEOUT_MS, TimeUnit.MILLISECONDS)) {
-                fail("Did not receive upstream " + (mAcceptAnyUpstream ? "any" : mExpectedUpstream)
-                        + " callback after " + TIMEOUT_MS + "ms");
+                final String errorMessage = "Did not receive upstream "
+                            + (mAcceptAnyUpstream ? "any" : mExpectedUpstream)
+                            + " callback after " + TIMEOUT_MS + "ms";
+
+                if (throwTimeoutException) {
+                    throw new TimeoutException(errorMessage);
+                } else {
+                    fail(errorMessage);
+                }
             }
             return mUpstream;
         }
@@ -1232,6 +1244,52 @@ public class EthernetTetheringTest {
         }
     }
 
+    // TODO: remove triggering upstream reselection once test network can replace selected upstream
+    // network in Tethering module.
+    private void maybeRetryTestedUpstreamChanged(final Network expectedUpstream,
+            final TimeoutException fallbackException) throws Exception {
+        // Fall back original exception because no way to reselect if there is no WIFI feature.
+        assertTrue(fallbackException.toString(), mPackageManager.hasSystemFeature(FEATURE_WIFI));
+
+        // Try to toggle wifi network, if any, to reselect upstream network via default network
+        // switching. Because test network has higher priority than internet network, this can
+        // help selecting test network to be upstream network for testing. This tries to avoid
+        // the flaky upstream selection under multinetwork environment. Internet and test network
+        // upstream changed event order is not guaranteed. Once tethering selects non-test
+        // upstream {wifi, ..}, test network won't be selected anymore. If too many test cases
+        // trigger the reselection, the total test time may over test suite 1 minmute timeout.
+        // Probably need to disable/restore all internet networks in a common place of test
+        // process. Currently, EthernetTetheringTest is part of CTS test which needs wifi network
+        // connection if device has wifi feature. CtsNetUtils#toggleWifi() checks wifi connection
+        // during the toggling process.
+        // See Tethering#chooseUpstreamType, CtsNetUtils#toggleWifi.
+        // TODO: toggle cellular network if the device has no WIFI feature.
+        Log.d(TAG, "Toggle WIFI to retry upstream selection");
+        mCtsNetUtils.toggleWifi();
+
+        // Wait for expected upstream.
+        final CompletableFuture<Network> future = new CompletableFuture<>();
+        final TetheringEventCallback callback = new TetheringEventCallback() {
+            @Override
+            public void onUpstreamChanged(Network network) {
+                Log.d(TAG, "Got upstream changed: " + network);
+                if (Objects.equals(expectedUpstream, network)) {
+                    future.complete(network);
+                }
+            }
+        };
+        try {
+            mTm.registerTetheringEventCallback(mHandler::post, callback);
+            assertEquals("onUpstreamChanged for unexpected network", expectedUpstream,
+                    future.get(TIMEOUT_MS, TimeUnit.MILLISECONDS));
+        } catch (TimeoutException e) {
+            throw new AssertionError("Did not receive upstream " + expectedUpstream
+                    + " callback after " + TIMEOUT_MS + "ms");
+        } finally {
+            mTm.unregisterTetheringEventCallback(callback);
+        }
+    }
+
     private TetheringTester initTetheringTester(List<LinkAddress> upstreamAddresses,
             List<InetAddress> upstreamDnses) throws Exception {
         assumeFalse(isInterfaceForTetheringAvailable());
@@ -1250,8 +1308,17 @@ public class EthernetTetheringTest {
 
         mTetheringEventCallback = enableEthernetTethering(mDownstreamIface.getInterfaceName(),
                 mUpstreamTracker.getNetwork());
-        assertEquals("onUpstreamChanged for unexpected network", mUpstreamTracker.getNetwork(),
-                mTetheringEventCallback.awaitUpstreamChanged());
+
+        try {
+            assertEquals("onUpstreamChanged for test network", mUpstreamTracker.getNetwork(),
+                    mTetheringEventCallback.awaitUpstreamChanged(
+                            true /* throwTimeoutException */));
+        } catch (TimeoutException e) {
+            // Due to race condition inside tethering module, test network may not be selected as
+            // tethering upstream. Force tethering retry upstream if possible. If it is not
+            // possible to retry, fail the test with the original timeout exception.
+            maybeRetryTestedUpstreamChanged(mUpstreamTracker.getNetwork(), e);
+        }
 
         mDownstreamReader = makePacketReader(mDownstreamIface);
         mUpstreamReader = makePacketReader(mUpstreamTracker.getTestIface());