From b4a8fd534a3822d4b603020b44082c7ff74d24f1 Mon Sep 17 00:00:00 2001 From: Xiao Ma Date: Mon, 30 May 2022 21:26:15 +0900 Subject: [PATCH] Add IpConfigStore testcase to improve the code coverage. Add new testcase in IpConfigStoreTest to cover the readIpConfiguration usage, improve the code coverage for connectivity module. To reduce the flakyness of test, this cl refactors DelayedDiskWriter constructors by allowing to pass in a Dependencies instance, then waitForIdle can be used to check if the delayed write operation associated with the passed-in HandlerThread has completed in tests. Bug: 234315786 Test: atest com.android.server.net.IpConfigStoreTest Change-Id: I57c8bd02a771f1cca815252ba502cda0210d1385 --- .../android/server/net/DelayedDiskWrite.java | 39 ++++++- .../android/server/net/IpConfigStoreTest.java | 101 ++++++++++++++---- 2 files changed, 115 insertions(+), 25 deletions(-) diff --git a/service/src/com/android/server/net/DelayedDiskWrite.java b/service/src/com/android/server/net/DelayedDiskWrite.java index 35dc455725..41cb41993a 100644 --- a/service/src/com/android/server/net/DelayedDiskWrite.java +++ b/service/src/com/android/server/net/DelayedDiskWrite.java @@ -21,6 +21,8 @@ import android.os.HandlerThread; import android.text.TextUtils; import android.util.Log; +import com.android.internal.annotations.VisibleForTesting; + import java.io.BufferedOutputStream; import java.io.DataOutputStream; import java.io.FileOutputStream; @@ -32,11 +34,42 @@ import java.io.IOException; public class DelayedDiskWrite { private static final String TAG = "DelayedDiskWrite"; + private final Dependencies mDeps; + private HandlerThread mDiskWriteHandlerThread; private Handler mDiskWriteHandler; /* Tracks multiple writes on the same thread */ private int mWriteSequence = 0; + public DelayedDiskWrite() { + this(new Dependencies()); + } + + @VisibleForTesting + DelayedDiskWrite(Dependencies deps) { + mDeps = deps; + } + + /** + * Dependencies class of DelayedDiskWrite, used for injection in tests. + */ + @VisibleForTesting + static class Dependencies { + /** + * Create a HandlerThread to use in DelayedDiskWrite. + */ + public HandlerThread makeHandlerThread() { + return new HandlerThread("DelayedDiskWriteThread"); + } + + /** + * Quit the HandlerThread looper. + */ + public void quitHandlerThread(HandlerThread handlerThread) { + handlerThread.getLooper().quit(); + } + } + /** * Used to do a delayed data write to a given {@link OutputStream}. */ @@ -65,7 +98,7 @@ public class DelayedDiskWrite { /* Do a delayed write to disk on a separate handler thread */ synchronized (this) { if (++mWriteSequence == 1) { - mDiskWriteHandlerThread = new HandlerThread("DelayedDiskWriteThread"); + mDiskWriteHandlerThread = mDeps.makeHandlerThread(); mDiskWriteHandlerThread.start(); mDiskWriteHandler = new Handler(mDiskWriteHandlerThread.getLooper()); } @@ -99,9 +132,9 @@ public class DelayedDiskWrite { // Quit if no more writes sent synchronized (this) { if (--mWriteSequence == 0) { - mDiskWriteHandler.getLooper().quit(); - mDiskWriteHandler = null; + mDeps.quitHandlerThread(mDiskWriteHandlerThread); mDiskWriteHandlerThread = null; + mDiskWriteHandler = null; } } } diff --git a/tests/unit/java/com/android/server/net/IpConfigStoreTest.java b/tests/unit/java/com/android/server/net/IpConfigStoreTest.java index 1801c45aa2..4adc999c48 100644 --- a/tests/unit/java/com/android/server/net/IpConfigStoreTest.java +++ b/tests/unit/java/com/android/server/net/IpConfigStoreTest.java @@ -20,6 +20,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.fail; +import android.content.Context; import android.net.InetAddresses; import android.net.IpConfiguration; import android.net.IpConfiguration.IpAssignment; @@ -28,10 +29,14 @@ import android.net.LinkAddress; import android.net.ProxyInfo; import android.net.StaticIpConfiguration; import android.os.Build; +import android.os.HandlerThread; import android.util.ArrayMap; +import androidx.test.InstrumentationRegistry; + import com.android.testutils.DevSdkIgnoreRule; import com.android.testutils.DevSdkIgnoreRunner; +import com.android.testutils.HandlerUtils; import org.junit.Test; import org.junit.runner.RunWith; @@ -39,11 +44,13 @@ import org.junit.runner.RunWith; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.DataOutputStream; +import java.io.File; import java.io.IOException; import java.io.InputStream; import java.net.InetAddress; import java.util.ArrayList; import java.util.Arrays; +import java.util.List; /** * Unit tests for {@link IpConfigStore} @@ -51,6 +58,7 @@ import java.util.Arrays; @RunWith(DevSdkIgnoreRunner.class) @DevSdkIgnoreRule.IgnoreUpTo(Build.VERSION_CODES.S_V2) public class IpConfigStoreTest { + private static final int TIMEOUT_MS = 2_000; private static final int KEY_CONFIG = 17; private static final String IFACE_1 = "eth0"; private static final String IFACE_2 = "eth1"; @@ -59,6 +67,22 @@ public class IpConfigStoreTest { private static final String DNS_IP_ADDR_1 = "1.2.3.4"; private static final String DNS_IP_ADDR_2 = "5.6.7.8"; + private static final ArrayList DNS_SERVERS = new ArrayList<>(List.of( + InetAddresses.parseNumericAddress(DNS_IP_ADDR_1), + InetAddresses.parseNumericAddress(DNS_IP_ADDR_2))); + private static final StaticIpConfiguration STATIC_IP_CONFIG_1 = + new StaticIpConfiguration.Builder() + .setIpAddress(new LinkAddress(IP_ADDR_1)) + .setDnsServers(DNS_SERVERS) + .build(); + private static final StaticIpConfiguration STATIC_IP_CONFIG_2 = + new StaticIpConfiguration.Builder() + .setIpAddress(new LinkAddress(IP_ADDR_2)) + .setDnsServers(DNS_SERVERS) + .build(); + private static final ProxyInfo PROXY_INFO = + ProxyInfo.buildDirectProxy("10.10.10.10", 88, Arrays.asList("host1", "host2")); + @Test public void backwardCompatibility2to3() throws IOException { ByteArrayOutputStream byteStream = new ByteArrayOutputStream(); @@ -82,40 +106,73 @@ public class IpConfigStoreTest { @Test public void staticIpMultiNetworks() throws Exception { - final ArrayList dnsServers = new ArrayList<>(); - dnsServers.add(InetAddresses.parseNumericAddress(DNS_IP_ADDR_1)); - dnsServers.add(InetAddresses.parseNumericAddress(DNS_IP_ADDR_2)); - final StaticIpConfiguration staticIpConfiguration1 = new StaticIpConfiguration.Builder() - .setIpAddress(new LinkAddress(IP_ADDR_1)) - .setDnsServers(dnsServers).build(); - final StaticIpConfiguration staticIpConfiguration2 = new StaticIpConfiguration.Builder() - .setIpAddress(new LinkAddress(IP_ADDR_2)) - .setDnsServers(dnsServers).build(); + final IpConfiguration expectedConfig1 = newIpConfiguration(IpAssignment.STATIC, + ProxySettings.STATIC, STATIC_IP_CONFIG_1, PROXY_INFO); + final IpConfiguration expectedConfig2 = newIpConfiguration(IpAssignment.STATIC, + ProxySettings.STATIC, STATIC_IP_CONFIG_2, PROXY_INFO); - ProxyInfo proxyInfo = - ProxyInfo.buildDirectProxy("10.10.10.10", 88, Arrays.asList("host1", "host2")); - - IpConfiguration expectedConfig1 = newIpConfiguration(IpAssignment.STATIC, - ProxySettings.STATIC, staticIpConfiguration1, proxyInfo); - IpConfiguration expectedConfig2 = newIpConfiguration(IpAssignment.STATIC, - ProxySettings.STATIC, staticIpConfiguration2, proxyInfo); - - ArrayMap expectedNetworks = new ArrayMap<>(); + final ArrayMap expectedNetworks = new ArrayMap<>(); expectedNetworks.put(IFACE_1, expectedConfig1); expectedNetworks.put(IFACE_2, expectedConfig2); - MockedDelayedDiskWrite writer = new MockedDelayedDiskWrite(); - IpConfigStore store = new IpConfigStore(writer); + final MockedDelayedDiskWrite writer = new MockedDelayedDiskWrite(); + final IpConfigStore store = new IpConfigStore(writer); store.writeIpConfigurations("file/path/not/used/", expectedNetworks); - InputStream in = new ByteArrayInputStream(writer.mByteStream.toByteArray()); - ArrayMap actualNetworks = IpConfigStore.readIpConfigurations(in); + final InputStream in = new ByteArrayInputStream(writer.mByteStream.toByteArray()); + final ArrayMap actualNetworks = + IpConfigStore.readIpConfigurations(in); assertNotNull(actualNetworks); assertEquals(2, actualNetworks.size()); assertEquals(expectedNetworks.get(IFACE_1), actualNetworks.get(IFACE_1)); assertEquals(expectedNetworks.get(IFACE_2), actualNetworks.get(IFACE_2)); } + @Test + public void readIpConfigurationFromFilePath() throws Exception { + final HandlerThread testHandlerThread = new HandlerThread("IpConfigStoreTest"); + final DelayedDiskWrite.Dependencies dependencies = + new DelayedDiskWrite.Dependencies() { + @Override + public HandlerThread makeHandlerThread() { + return testHandlerThread; + } + @Override + public void quitHandlerThread(HandlerThread handlerThread) { + testHandlerThread.quitSafely(); + } + }; + + final IpConfiguration ipConfig = newIpConfiguration(IpAssignment.STATIC, + ProxySettings.STATIC, STATIC_IP_CONFIG_1, PROXY_INFO); + final ArrayMap expectedNetworks = new ArrayMap<>(); + expectedNetworks.put(IFACE_1, ipConfig); + + // Write IP config to specific file path and read it later. + final Context context = InstrumentationRegistry.getContext(); + final File configFile = new File(context.getFilesDir().getPath(), + "IpConfigStoreTest-ipconfig.txt"); + final DelayedDiskWrite writer = new DelayedDiskWrite(dependencies); + final IpConfigStore store = new IpConfigStore(writer); + store.writeIpConfigurations(configFile.getPath(), expectedNetworks); + HandlerUtils.waitForIdle(testHandlerThread, TIMEOUT_MS); + + // Read IP config from the file path. + final ArrayMap actualNetworks = + IpConfigStore.readIpConfigurations(configFile.getPath()); + assertNotNull(actualNetworks); + assertEquals(1, actualNetworks.size()); + assertEquals(expectedNetworks.get(IFACE_1), actualNetworks.get(IFACE_1)); + + // Return an empty array when reading IP configuration from an non-exist config file. + final ArrayMap emptyNetworks = + IpConfigStore.readIpConfigurations("/dev/null"); + assertNotNull(emptyNetworks); + assertEquals(0, emptyNetworks.size()); + + configFile.delete(); + } + private IpConfiguration newIpConfiguration(IpAssignment ipAssignment, ProxySettings proxySettings, StaticIpConfiguration staticIpConfig, ProxyInfo info) { final IpConfiguration config = new IpConfiguration();