Merge "Fix the comments from aosp/1719018"

tests/unit/java/com/android/server/NsdServiceTest.java

@@ -18,10 +18,12 @@ package com.android.server;
 
 import static org.junit.Assert.assertEquals;
 import static org.mockito.Mockito.any;
+import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.reset;
 import static org.mockito.Mockito.timeout;
+import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
@@ -40,6 +42,7 @@ import androidx.test.runner.AndroidJUnit4;
 import com.android.server.NsdService.DaemonConnection;
 import com.android.server.NsdService.DaemonConnectionSupplier;
 import com.android.server.NsdService.NativeCallbackReceiver;
+import com.android.testutils.HandlerUtils;
 
 import org.junit.After;
 import org.junit.Before;
@@ -48,6 +51,7 @@ import org.junit.runner.RunWith;
 import org.mockito.ArgumentCaptor;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
+import org.mockito.Spy;
 
 // TODOs:
 //  - test client can send requests and receive replies
@@ -58,14 +62,13 @@ public class NsdServiceTest {
 
     static final int PROTOCOL = NsdManager.PROTOCOL_DNS_SD;
     private static final long CLEANUP_DELAY_MS = 500;
-
+    private static final long TIMEOUT_MS = 500;
-    long mTimeoutMs = 100; // non-final so that tests can adjust the value.
 
     @Mock Context mContext;
     @Mock ContentResolver mResolver;
     @Mock NsdService.NsdSettings mSettings;
-    @Mock DaemonConnection mDaemon;
     NativeCallbackReceiver mDaemonCallback;
+    @Spy DaemonConnection mDaemon = new DaemonConnection(mDaemonCallback);
     HandlerThread mThread;
     TestHandler mHandler;
 
@@ -74,6 +77,7 @@ public class NsdServiceTest {
         MockitoAnnotations.initMocks(this);
         mThread = new HandlerThread("mock-service-handler");
         mThread.start();
+        doReturn(true).when(mDaemon).execute(any());
         mHandler = new TestHandler(mThread.getLooper());
         when(mContext.getContentResolver()).thenReturn(mResolver);
     }
@@ -95,14 +99,17 @@ public class NsdServiceTest {
         // Creating an NsdManager will not cause any cmds executed, which means
         // no daemon is started.
         NsdManager client1 = connectClient(service);
+        waitForIdle();
         verify(mDaemon, never()).execute(any());
 
         // Creating another NsdManager will not cause any cmds executed.
         NsdManager client2 = connectClient(service);
+        waitForIdle();
         verify(mDaemon, never()).execute(any());
 
         client1.disconnect();
         // Still 1 client remains, daemon shouldn't be stopped.
+        waitForIdle();
         verify(mDaemon, never()).maybeStop();
 
         client2.disconnect();
@@ -116,11 +123,11 @@ public class NsdServiceTest {
     @Test
     public void testClientRequestsAreGCedAtDisconnection() {
         when(mSettings.isEnabled()).thenReturn(true);
-        when(mDaemon.execute(any())).thenReturn(true);
 
         NsdService service = makeService();
         NsdManager client = connectClient(service);
 
+        waitForIdle();
         verify(mDaemon, never()).maybeStart();
         verify(mDaemon, never()).execute(any());
 
@@ -130,27 +137,30 @@ public class NsdServiceTest {
         // Client registration request
         NsdManager.RegistrationListener listener1 = mock(NsdManager.RegistrationListener.class);
         client.registerService(request, PROTOCOL, listener1);
-        verify(mDaemon, timeout(mTimeoutMs).times(1)).maybeStart();
+        waitForIdle();
-        verifyDaemonCommand("register 2 a_name a_type 2201");
+        verify(mDaemon, times(1)).maybeStart();
+        verifyDaemonCommands("start-service", "register 2 a_name a_type 2201");
 
         // Client discovery request
         NsdManager.DiscoveryListener listener2 = mock(NsdManager.DiscoveryListener.class);
         client.discoverServices("a_type", PROTOCOL, listener2);
-        verify(mDaemon, timeout(mTimeoutMs).times(1)).maybeStart();
+        waitForIdle();
+        verify(mDaemon, times(1)).maybeStart();
         verifyDaemonCommand("discover 3 a_type");
 
         // Client resolve request
         NsdManager.ResolveListener listener3 = mock(NsdManager.ResolveListener.class);
         client.resolveService(request, listener3);
-        verify(mDaemon, timeout(mTimeoutMs).times(1)).maybeStart();
+        waitForIdle();
+        verify(mDaemon, times(1)).maybeStart();
         verifyDaemonCommand("resolve 4 a_name a_type local.");
 
         // Client disconnects, stop the daemon after CLEANUP_DELAY_MS.
         client.disconnect();
         verifyDelayMaybeStopDaemon(CLEANUP_DELAY_MS);
-
         // checks that request are cleaned
-        verifyDaemonCommands("stop-register 2", "stop-discover 3", "stop-resolve 4");
+        verifyDaemonCommands("stop-register 2", "stop-discover 3",
+                "stop-resolve 4", "stop-service");
 
         client.disconnect();
     }
@@ -158,7 +168,6 @@ public class NsdServiceTest {
     @Test
     public void testCleanupDelayNoRequestActive() {
         when(mSettings.isEnabled()).thenReturn(true);
-        when(mDaemon.execute(any())).thenReturn(true);
 
         NsdService service = makeService();
         NsdManager client = connectClient(service);
@@ -167,19 +176,25 @@ public class NsdServiceTest {
         request.setPort(2201);
         NsdManager.RegistrationListener listener1 = mock(NsdManager.RegistrationListener.class);
         client.registerService(request, PROTOCOL, listener1);
-        verify(mDaemon, timeout(mTimeoutMs).times(1)).maybeStart();
+        waitForIdle();
-        verifyDaemonCommand("register 2 a_name a_type 2201");
+        verify(mDaemon, times(1)).maybeStart();
+        verifyDaemonCommands("start-service", "register 2 a_name a_type 2201");
 
         client.unregisterService(listener1);
         verifyDaemonCommand("stop-register 2");
 
         verifyDelayMaybeStopDaemon(CLEANUP_DELAY_MS);
+        verifyDaemonCommand("stop-service");
         reset(mDaemon);
         client.disconnect();
         // Client disconnects, after CLEANUP_DELAY_MS, maybeStop the daemon.
         verifyDelayMaybeStopDaemon(CLEANUP_DELAY_MS);
     }
 
+    private void waitForIdle() {
+        HandlerUtils.waitForIdle(mHandler, TIMEOUT_MS);
+    }
+
     NsdService makeService() {
         DaemonConnectionSupplier supplier = (callback) -> {
             mDaemonCallback = callback;
@@ -196,10 +211,11 @@ public class NsdServiceTest {
     }
 
     void verifyDelayMaybeStopDaemon(long cleanupDelayMs) {
+        waitForIdle();
         // Stop daemon shouldn't be called immediately.
-        verify(mDaemon, timeout(mTimeoutMs).times(0)).maybeStop();
+        verify(mDaemon, never()).maybeStop();
         // Clean up the daemon after CLEANUP_DELAY_MS.
-        verify(mDaemon, timeout(cleanupDelayMs + mTimeoutMs)).maybeStop();
+        verify(mDaemon, timeout(cleanupDelayMs + TIMEOUT_MS)).maybeStop();
     }
 
     void verifyDaemonCommands(String... wants) {
@@ -211,8 +227,9 @@ public class NsdServiceTest {
     }
 
     void verifyDaemonCommand(String want, int n) {
-        ArgumentCaptor<Object> argumentsCaptor = ArgumentCaptor.forClass(Object.class);
+        waitForIdle();
-        verify(mDaemon, timeout(mTimeoutMs).times(n)).execute(argumentsCaptor.capture());
+        final ArgumentCaptor<Object> argumentsCaptor = ArgumentCaptor.forClass(Object.class);
+        verify(mDaemon, times(n)).execute(argumentsCaptor.capture());
         String got = "";
         for (Object o : argumentsCaptor.getAllValues()) {
             got += o + " ";
@@ -220,7 +237,7 @@ public class NsdServiceTest {
         assertEquals(want, got.trim());
         // rearm deamon for next command verification
         reset(mDaemon);
-        when(mDaemon.execute(any())).thenReturn(true);
+        doReturn(true).when(mDaemon).execute(any());
     }
 
     public static class TestHandler extends Handler {