From a086800e0409a74279d1caf4141e5508b6be4aa6 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Tue, 11 Jul 2017 02:29:28 +0900 Subject: [PATCH] Make NetworkManagementServiceTest pass again. 1. Mock the service manager so that NMS can fetch mock versions of INetd and IBatteryStats. 2. Call LocalServices.removeServiceForTest to avoid a duplicate service registration error. // check this 3. Change the timeout from 100ms to 200ms, as otherwise the tests that check for IfaceClass fail. 4. Convert NetworkManagementServiceTest to JUnit 4. 5. Move NetworkManagementServiceTest to tests/net Bug: 29337859 Bug: 32163131 Bug: 32561414 Bug: 62918393 Test: runtest frameworks-net Change-Id: Ic7371b427b35809ccd446addf35c9d8ae99ccfd3 --- .../android/server/NetworkManagementService.java | 42 ++++++++-- .../server/NetworkManagementServiceTest.java | 95 ++++++++++++++-------- 2 files changed, 94 insertions(+), 43 deletions(-) rename {services/tests/servicestests/src => tests/net/java}/com/android/server/NetworkManagementServiceTest.java (79%) diff --git a/services/core/java/com/android/server/NetworkManagementService.java b/services/core/java/com/android/server/NetworkManagementService.java index 6b621d2fee48..3e4453204b14 100644 --- a/services/core/java/com/android/server/NetworkManagementService.java +++ b/services/core/java/com/android/server/NetworkManagementService.java @@ -73,6 +73,7 @@ import android.net.wifi.WifiConfiguration.KeyMgmt; import android.os.BatteryStats; import android.os.Binder; import android.os.Handler; +import android.os.IBinder; import android.os.INetworkActivityListener; import android.os.INetworkManagementService; import android.os.PowerManager; @@ -132,10 +133,27 @@ import java.util.concurrent.CountDownLatch; */ public class NetworkManagementService extends INetworkManagementService.Stub implements Watchdog.Monitor { + + /** + * Helper class that encapsulates NetworkManagementService dependencies and makes them + * easier to mock in unit tests. + */ + static class SystemServices { + public IBinder getService(String name) { + return ServiceManager.getService(name); + } + public void registerLocalService(NetworkManagementInternal nmi) { + LocalServices.addService(NetworkManagementInternal.class, nmi); + } + public INetd getNetd() { + return NetdService.get(); + } + } + private static final String TAG = "NetworkManagement"; private static final boolean DBG = Log.isLoggable(TAG, Log.DEBUG); private static final String NETD_TAG = "NetdConnector"; - private static final String NETD_SERVICE_NAME = "netd"; + static final String NETD_SERVICE_NAME = "netd"; private static final int MAX_UID_RANGES_PER_COMMAND = 10; @@ -217,6 +235,8 @@ public class NetworkManagementService extends INetworkManagementService.Stub private final Handler mFgHandler; private final Handler mDaemonHandler; + private final SystemServices mServices; + private INetd mNetdService; private IBatteryStats mBatteryStats; @@ -315,8 +335,10 @@ public class NetworkManagementService extends INetworkManagementService.Stub * * @param context Binder context for this service */ - private NetworkManagementService(Context context, String socket) { + private NetworkManagementService( + Context context, String socket, SystemServices services) { mContext = context; + mServices = services; // make sure this is on the same looper as our NativeDaemonConnector for sync purposes mFgHandler = new Handler(FgThread.get().getLooper()); @@ -338,7 +360,7 @@ public class NetworkManagementService extends INetworkManagementService.Stub // Add ourself to the Watchdog monitors. Watchdog.getInstance().addMonitor(this); - LocalServices.addService(NetworkManagementInternal.class, new LocalService()); + mServices.registerLocalService(new LocalService()); synchronized (mTetheringStatsProviders) { mTetheringStatsProviders.put(new NetdTetheringStatsProvider(), "netd"); @@ -352,11 +374,13 @@ public class NetworkManagementService extends INetworkManagementService.Stub mDaemonHandler = null; mFgHandler = null; mThread = null; + mServices = null; } - static NetworkManagementService create(Context context, String socket) + static NetworkManagementService create(Context context, String socket, SystemServices services) throws InterruptedException { - final NetworkManagementService service = new NetworkManagementService(context, socket); + final NetworkManagementService service = + new NetworkManagementService(context, socket, services); final CountDownLatch connectedSignal = service.mConnectedSignal; if (DBG) Slog.d(TAG, "Creating NetworkManagementService"); service.mThread.start(); @@ -370,7 +394,7 @@ public class NetworkManagementService extends INetworkManagementService.Stub } public static NetworkManagementService create(Context context) throws InterruptedException { - return create(context, NETD_SERVICE_NAME); + return create(context, NETD_SERVICE_NAME, new SystemServices()); } public void systemReady() { @@ -390,8 +414,8 @@ public class NetworkManagementService extends INetworkManagementService.Stub if (mBatteryStats != null) { return mBatteryStats; } - mBatteryStats = IBatteryStats.Stub.asInterface(ServiceManager.getService( - BatteryStats.SERVICE_NAME)); + mBatteryStats = + IBatteryStats.Stub.asInterface(mServices.getService(BatteryStats.SERVICE_NAME)); return mBatteryStats; } } @@ -589,7 +613,7 @@ public class NetworkManagementService extends INetworkManagementService.Stub } private void connectNativeNetdService() { - mNetdService = NetdService.get(); + mNetdService = mServices.getNetd(); } /** diff --git a/services/tests/servicestests/src/com/android/server/NetworkManagementServiceTest.java b/tests/net/java/com/android/server/NetworkManagementServiceTest.java similarity index 79% rename from services/tests/servicestests/src/com/android/server/NetworkManagementServiceTest.java rename to tests/net/java/com/android/server/NetworkManagementServiceTest.java index f841bf9bff22..2ac73dbd4f52 100644 --- a/services/tests/servicestests/src/com/android/server/NetworkManagementServiceTest.java +++ b/tests/net/java/com/android/server/NetworkManagementServiceTest.java @@ -16,16 +16,6 @@ package com.android.server; -import android.content.Context; -import android.net.LinkAddress; -import android.net.LocalSocket; -import android.net.LocalServerSocket; -import android.os.Binder; -import android.test.AndroidTestCase; -import android.test.suitebuilder.annotation.LargeTest; -import com.android.server.net.BaseNetworkObserver; -import com.android.internal.util.test.BroadcastInterceptingContext; - import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.reset; @@ -33,14 +23,37 @@ import static org.mockito.Mockito.timeout; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; +import android.content.Context; +import android.net.INetd; +import android.net.LinkAddress; +import android.net.LocalSocket; +import android.net.LocalServerSocket; +import android.os.BatteryStats; +import android.os.Binder; +import android.os.IBinder; +import android.support.test.runner.AndroidJUnit4; +import android.test.suitebuilder.annotation.SmallTest; + +import com.android.internal.app.IBatteryStats; +import com.android.server.NetworkManagementService.SystemServices; +import com.android.server.net.BaseNetworkObserver; + import java.io.IOException; import java.io.OutputStream; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; + /** * Tests for {@link NetworkManagementService}. */ -@LargeTest -public class NetworkManagementServiceTest extends AndroidTestCase { +@RunWith(AndroidJUnit4.class) +@SmallTest +public class NetworkManagementServiceTest { private static final String SOCKET_NAME = "__test__NetworkManagementServiceTest"; private NetworkManagementService mNMService; @@ -48,27 +61,46 @@ public class NetworkManagementServiceTest extends AndroidTestCase { private LocalSocket mSocket; private OutputStream mOutputStream; - @Override + @Mock private Context mContext; + @Mock private IBatteryStats.Stub mBatteryStatsService; + @Mock private INetd.Stub mNetdService; + + private final SystemServices mServices = new SystemServices() { + @Override + public IBinder getService(String name) { + switch (name) { + case BatteryStats.SERVICE_NAME: + return mBatteryStatsService; + default: + throw new UnsupportedOperationException("Unknown service " + name); + } + } + @Override + public void registerLocalService(NetworkManagementInternal nmi) { + } + @Override + public INetd getNetd() { + return mNetdService; + } + }; + + @Before public void setUp() throws Exception { - super.setUp(); - // TODO: make this unnecessary. runtest might already make it unnecessary. - System.setProperty("dexmaker.dexcache", getContext().getCacheDir().toString()); + MockitoAnnotations.initMocks(this); // Set up a sheltered test environment. - BroadcastInterceptingContext context = new BroadcastInterceptingContext(getContext()); mServerSocket = new LocalServerSocket(SOCKET_NAME); // Start the service and wait until it connects to our socket. - mNMService = NetworkManagementService.create(context, SOCKET_NAME); + mNMService = NetworkManagementService.create(mContext, SOCKET_NAME, mServices); mSocket = mServerSocket.accept(); mOutputStream = mSocket.getOutputStream(); } - @Override + @After public void tearDown() throws Exception { if (mSocket != null) mSocket.close(); if (mServerSocket != null) mServerSocket.close(); - super.tearDown(); } /** @@ -80,12 +112,13 @@ public class NetworkManagementServiceTest extends AndroidTestCase { } private static T expectSoon(T mock) { - return verify(mock, timeout(100)); + return verify(mock, timeout(200)); } /** * Tests that network observers work properly. */ + @Test public void testNetworkObservers() throws Exception { BaseNetworkObserver observer = mock(BaseNetworkObserver.class); doReturn(new Binder()).when(observer).asBinder(); // Used by registerObserver. @@ -143,22 +176,16 @@ public class NetworkManagementServiceTest extends AndroidTestCase { * Interface class activity. */ - sendMessage("613 IfaceClass active rmnet0"); - expectSoon(observer).interfaceClassDataActivityChanged("rmnet0", true, 0); - - sendMessage("613 IfaceClass active rmnet0 1234"); - expectSoon(observer).interfaceClassDataActivityChanged("rmnet0", true, 1234); - - sendMessage("613 IfaceClass idle eth0"); - expectSoon(observer).interfaceClassDataActivityChanged("eth0", false, 0); + sendMessage("613 IfaceClass active 1 1234 10012"); + expectSoon(observer).interfaceClassDataActivityChanged("1", true, 1234); - sendMessage("613 IfaceClass idle eth0 1234"); - expectSoon(observer).interfaceClassDataActivityChanged("eth0", false, 1234); + sendMessage("613 IfaceClass idle 9 5678"); + expectSoon(observer).interfaceClassDataActivityChanged("9", false, 5678); - sendMessage("613 IfaceClass reallyactive rmnet0 1234"); - expectSoon(observer).interfaceClassDataActivityChanged("rmnet0", false, 1234); + sendMessage("613 IfaceClass reallyactive 9 4321"); + expectSoon(observer).interfaceClassDataActivityChanged("9", false, 4321); - sendMessage("613 InterfaceClass reallyactive rmnet0"); + sendMessage("613 InterfaceClass reallyactive 1"); // Invalid group. -- 2.11.0