From 33bb7fbfb7fbd942db3befec3af640aadfcbecaa Mon Sep 17 00:00:00 2001 From: Junyu Lai Date: Wed, 2 Oct 2024 23:45:08 +0800 Subject: [PATCH] Refactor requestNetworkAndGenerateTraffic by using synchronized methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is a refactoring of the test to adopt synchronized methods. This is needed for the follow-up changes to get attributes of the network prior to generating the network traffic. This change includes:   1. Use synchronized methods in AutoReleaseNetworkCallbackRule for      better managing request/release networks and also improves      readability.   2. Decouple the steps of requesting network, set attributes and   generating network traffic.   3. Clearer fail messages since the test can directly throw exceptions      to reflect the failures of a specific procedure.   4. Remove the global timeout, which makes the test more likely      to be flaky when individual steps take longer to complete.   5. Use capabilities in the callback instead of synchronous calls to   fetch attributes.   6. Use @RestoreDefaultNetwork annotation to enforce default network   restoration. Test: atest CtsNetTestCases:android.net.cts.NetworkStatsManagerTest \ --rerun-until-failure 30 Bug: 368624224 Change-Id: I3effa3c032afb345f1c0c4b73b51397d18970a19 --- .../net/cts/NetworkStatsManagerTest.java | 198 ++++++++---------- 1 file changed, 91 insertions(+), 107 deletions(-) diff --git a/tests/cts/net/src/android/net/cts/NetworkStatsManagerTest.java b/tests/cts/net/src/android/net/cts/NetworkStatsManagerTest.java index 11fc6dfafd..fef085ddaa 100644 --- a/tests/cts/net/src/android/net/cts/NetworkStatsManagerTest.java +++ b/tests/cts/net/src/android/net/cts/NetworkStatsManagerTest.java @@ -41,6 +41,7 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import android.annotation.NonNull; import android.app.AppOpsManager; import android.app.Instrumentation; import android.app.usage.NetworkStats; @@ -68,13 +69,16 @@ import android.text.TextUtils; import android.util.Log; import androidx.test.InstrumentationRegistry; -import androidx.test.ext.junit.runners.AndroidJUnit4; import com.android.compatibility.common.util.ShellIdentityUtils; import com.android.compatibility.common.util.SystemUtil; import com.android.modules.utils.build.SdkLevel; +import com.android.testutils.AutoReleaseNetworkCallbackRule; import com.android.testutils.ConnectivityModuleTest; import com.android.testutils.DevSdkIgnoreRule; +import com.android.testutils.DevSdkIgnoreRunner; +import com.android.testutils.RecorderCallback.CallbackEntry; +import com.android.testutils.TestableNetworkCallback; import org.junit.After; import org.junit.Before; @@ -95,12 +99,18 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.TimeUnit; -@ConnectivityModuleTest +// TODO: Fix thread leaks in testCallback and annotating with @MonitorThreadLeak. @AppModeFull(reason = "instant apps cannot be granted USAGE_STATS") -@RunWith(AndroidJUnit4.class) +@ConnectivityModuleTest +@DevSdkIgnoreRunner.RestoreDefaultNetwork +@RunWith(DevSdkIgnoreRunner.class) public class NetworkStatsManagerTest { - @Rule + @Rule(order = 1) public final DevSdkIgnoreRule ignoreRule = new DevSdkIgnoreRule(Build.VERSION_CODES.Q); + @Rule(order = 2) + public final AutoReleaseNetworkCallbackRule + networkCallbackRule = new AutoReleaseNetworkCallbackRule(); + private static final String LOG_TAG = "NetworkStatsManagerTest"; private static final String APPOPS_SET_SHELL_COMMAND = "appops set {0} {1} {2}"; @@ -119,12 +129,19 @@ public class NetworkStatsManagerTest { private static final long LONG_TOLERANCE = MINUTE * 120; private abstract class NetworkInterfaceToTest { + + final TestableNetworkCallback mRequestNetworkCb = new TestableNetworkCallback(); private boolean mMetered; private boolean mRoaming; private boolean mIsDefault; abstract int getNetworkType(); - abstract int getTransportType(); + + abstract Network requestNetwork(); + + void unrequestNetwork() { + networkCallbackRule.unregisterNetworkCallback(mRequestNetworkCb); + } public boolean getMetered() { return mMetered; @@ -151,7 +168,13 @@ public class NetworkStatsManagerTest { } abstract String getSystemFeature(); - abstract String getErrorMessage(); + + @NonNull NetworkRequest buildRequestForTransport(int transport) { + return new NetworkRequest.Builder() + .addTransportType(transport) + .addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) + .build(); + } } private final NetworkInterfaceToTest[] mNetworkInterfacesToTest = @@ -163,19 +186,20 @@ public class NetworkStatsManagerTest { } @Override - public int getTransportType() { - return NetworkCapabilities.TRANSPORT_WIFI; + public Network requestNetwork() { + networkCallbackRule.requestNetwork(buildRequestForTransport( + NetworkCapabilities.TRANSPORT_WIFI), + mRequestNetworkCb, TIMEOUT_MILLIS); + return mRequestNetworkCb.expect(CallbackEntry.AVAILABLE, + "Wifi network not available. " + + "Please ensure the device has working wifi." + ).getNetwork(); } @Override public String getSystemFeature() { return PackageManager.FEATURE_WIFI; } - - @Override - public String getErrorMessage() { - return " Please make sure you are connected to a WiFi access point."; - } }, new NetworkInterfaceToTest() { @Override @@ -184,22 +208,20 @@ public class NetworkStatsManagerTest { } @Override - public int getTransportType() { - return NetworkCapabilities.TRANSPORT_CELLULAR; + public Network requestNetwork() { + networkCallbackRule.requestNetwork(buildRequestForTransport( + NetworkCapabilities.TRANSPORT_CELLULAR), + mRequestNetworkCb, TIMEOUT_MILLIS); + return mRequestNetworkCb.expect(CallbackEntry.AVAILABLE, + "Cell network not available. " + + "Please ensure the device has working mobile data." + ).getNetwork(); } @Override public String getSystemFeature() { return PackageManager.FEATURE_TELEPHONY; } - - @Override - public String getErrorMessage() { - return " Please make sure you have added a SIM card with data plan to" - + " your phone, have enabled data over cellular and in case of" - + " dual SIM devices, have selected the right SIM " - + "for data connection."; - } } }; @@ -215,7 +237,22 @@ public class NetworkStatsManagerTest { private String mWriteSettingsMode; private String mUsageStatsMode; - private void exerciseRemoteHost(Network network, URL url) throws Exception { + // The test host only has IPv4. So on a dual-stack network where IPv6 connects before IPv4, + // we need to wait until IPv4 is available or the test will spuriously fail. + private static void waitForHostResolution(@NonNull Network network, @NonNull URL url) { + for (int i = 0; i < HOST_RESOLUTION_RETRIES; i++) { + try { + network.getAllByName(url.getHost()); + return; + } catch (UnknownHostException e) { + SystemClock.sleep(HOST_RESOLUTION_INTERVAL_MS); + } + } + fail(String.format("%s could not be resolved on network %s (%d attempts %dms apart)", + url.getHost(), network, HOST_RESOLUTION_RETRIES, HOST_RESOLUTION_INTERVAL_MS)); + } + + private void exerciseRemoteHost(@NonNull Network network, @NonNull URL url) throws Exception { NetworkInfo networkInfo = mCm.getNetworkInfo(network); if (networkInfo == null) { Log.w(LOG_TAG, "Network info is null"); @@ -311,97 +348,44 @@ public class NetworkStatsManagerTest { return result.contains("FOREGROUND"); } - private class NetworkCallback extends ConnectivityManager.NetworkCallback { - private long mTolerance; - private URL mUrl; - public boolean success; - public boolean metered; - public boolean roaming; - public boolean isDefault; - - NetworkCallback(long tolerance, URL url) { - mTolerance = tolerance; - mUrl = url; - success = false; - metered = false; - roaming = false; - isDefault = false; - } - - // The test host only has IPv4. So on a dual-stack network where IPv6 connects before IPv4, - // we need to wait until IPv4 is available or the test will spuriously fail. - private void waitForHostResolution(Network network) { - for (int i = 0; i < HOST_RESOLUTION_RETRIES; i++) { - try { - network.getAllByName(mUrl.getHost()); - return; - } catch (UnknownHostException e) { - SystemClock.sleep(HOST_RESOLUTION_INTERVAL_MS); - } - } - fail(String.format("%s could not be resolved on network %s (%d attempts %dms apart)", - mUrl.getHost(), network, HOST_RESOLUTION_RETRIES, HOST_RESOLUTION_INTERVAL_MS)); - } - - @Override - public void onAvailable(Network network) { - try { - mStartTime = System.currentTimeMillis() - mTolerance; - isDefault = network.equals(mCm.getActiveNetwork()); - waitForHostResolution(network); - exerciseRemoteHost(network, mUrl); - mEndTime = System.currentTimeMillis() + mTolerance; - success = true; - metered = !mCm.getNetworkCapabilities(network) - .hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_METERED); - roaming = !mCm.getNetworkCapabilities(network) - .hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_ROAMING); - synchronized (NetworkStatsManagerTest.this) { - NetworkStatsManagerTest.this.notify(); - } - } catch (Exception e) { - Log.w(LOG_TAG, "exercising remote host failed.", e); - success = false; - } - } - } - private boolean shouldTestThisNetworkType(int networkTypeIndex) { return mPm.hasSystemFeature(mNetworkInterfacesToTest[networkTypeIndex].getSystemFeature()); } + @NonNull + private Network requestNetworkAndSetAttributes( + @NonNull NetworkInterfaceToTest networkInterface) { + final Network network = networkInterface.requestNetwork(); + + // These attributes are needed when performing NetworkStats queries. + // Fetch caps from the first capabilities changed event since the + // interested attributes are not mutable, and not expected to be + // changed during the test. + final NetworkCapabilities caps = networkInterface.mRequestNetworkCb.expect( + CallbackEntry.NETWORK_CAPS_UPDATED, network).getCaps(); + networkInterface.setMetered(!caps.hasCapability( + NetworkCapabilities.NET_CAPABILITY_NOT_METERED)); + networkInterface.setRoaming(!caps.hasCapability( + NetworkCapabilities.NET_CAPABILITY_NOT_ROAMING)); + networkInterface.setIsDefault(network.equals(mCm.getActiveNetwork())); + + return network; + } + private void requestNetworkAndGenerateTraffic(int networkTypeIndex, final long tolerance) throws Exception { final NetworkInterfaceToTest networkInterface = mNetworkInterfacesToTest[networkTypeIndex]; - final NetworkCallback callback = new NetworkCallback(tolerance, - new URL(CHECK_CONNECTIVITY_URL)); - mCm.requestNetwork(new NetworkRequest.Builder() - .addTransportType(networkInterface.getTransportType()) - .addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) - .build(), callback); - synchronized (this) { - long now = System.currentTimeMillis(); - final long deadline = (long) (now + TIMEOUT_MILLIS * 2.4); - while (!callback.success && now < deadline) { - try { - wait(deadline - now); - } catch (InterruptedException e) { - } - now = System.currentTimeMillis(); - } - } - mCm.unregisterNetworkCallback(callback); - if (!callback.success) { - fail(networkInterface.getSystemFeature() - + " is a reported system feature, however no corresponding " - + "connected network interface was found or the attempt " - + "to connect and read has timed out (timeout = " + (TIMEOUT_MILLIS * 2.4) - + "ms)." + networkInterface.getErrorMessage()); - } + final Network network = requestNetworkAndSetAttributes(networkInterface); - networkInterface.setMetered(callback.metered); - networkInterface.setRoaming(callback.roaming); - networkInterface.setIsDefault(callback.isDefault); + mStartTime = System.currentTimeMillis() - tolerance; + waitForHostResolution(network, new URL(CHECK_CONNECTIVITY_URL)); + exerciseRemoteHost(network, new URL(CHECK_CONNECTIVITY_URL)); + mEndTime = System.currentTimeMillis() + tolerance; + + // It is fine if the test fails and this line is not reached. + // The AutoReleaseNetworkCallbackRule will eventually release + // all unwanted callbacks. + networkInterface.unrequestNetwork(); } private String getSubscriberId(int networkIndex) {