commit e687d5844e60cbab42f975b63411be394beb69b7
author Remi NGUYEN VAN <reminv@google.com> Mon Jul 05 06:31:01 2021 +0000
committer Remi NGUYEN VAN <reminv@google.com> Tue Jul 06 06:09:17 2021 +0000
tree 534751e3f0e50f29406d5d8eadfb6a4882c7c0ae
parent 6be4ebcb0a334b467e4b229c5fafa0088f7b5475

Fix flake in testMobileDataAlwaysOn

The internal structure backing getAllNetworks is updated after
invoking lost callbacks, so the test needs to wait for the handler
to be idle before calling getAllNetworks.

Also avoid assertions in finally clauses, as it causes confusing
error messages.

Bug: 185081944
Change-Id: I8ad5ab45a3e65b0031672e6f594b1a6d6ad5abcb
Test: atest ConnectivityServiceTest

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

diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java
index cec3882..7877069 100644
--- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java
@@ -1167,20 +1167,14 @@
             assertEquals(count, getMyRequestCount());
         }
 
-        @Override
-        public void terminate() {
-            super.terminate();
-            // Make sure there are no remaining requests unaccounted for.
-            HandlerUtils.waitForIdle(mHandlerSendingRequests, TIMEOUT_MS);
-            assertNull(mRequestHistory.poll(0, r -> true));
-        }
-
         // Trigger releasing the request as unfulfillable
         public void triggerUnfulfillable(NetworkRequest r) {
             super.releaseRequestAsUnfulfillableByAnyFactory(r);
         }
 
         public void assertNoRequestChanged() {
+            // Make sure there are no remaining requests unaccounted for.
+            HandlerUtils.waitForIdle(mHandlerSendingRequests, TIMEOUT_MS);
             assertNull(mRequestHistory.poll(0, r -> true));
         }
     }
@@ -3234,6 +3228,7 @@
         assertTrue(testFactory.getMyStartRequested());
 
         testFactory.terminate();
+        testFactory.assertNoRequestChanged();
         if (networkCallback != null) mCm.unregisterNetworkCallback(networkCallback);
         handlerThread.quit();
     }
@@ -3318,6 +3313,7 @@
 
             testFactory.setScoreFilter(42);
             testFactory.terminate();
+            testFactory.assertNoRequestChanged();
 
             if (i % 2 == 0) {
                 try {
@@ -4619,6 +4615,9 @@
             // and the test factory should see it now that it isn't hopelessly outscored.
             mCellNetworkAgent.disconnect();
             cellNetworkCallback.expectCallback(CallbackEntry.LOST, mCellNetworkAgent);
+            // Wait for the network to be removed from internal structures before
+            // calling synchronous getter
+            waitForIdle();
             assertLength(1, mCm.getAllNetworks());
             testFactory.expectRequestAdd();
             testFactory.assertRequestCountEquals(1);
@@ -4629,6 +4628,7 @@
             mCellNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR);
             mCellNetworkAgent.connect(true);
             cellNetworkCallback.expectAvailableThenValidatedCallbacks(mCellNetworkAgent);
+            waitForIdle();
             assertLength(2, mCm.getAllNetworks());
             testFactory.expectRequestRemove();
             testFactory.assertRequestCountEquals(0);
@@ -4640,8 +4640,9 @@
             cellNetworkCallback.expectCallback(CallbackEntry.LOST, mCellNetworkAgent);
             waitForIdle();
             assertLength(1, mCm.getAllNetworks());
-        } finally {
             testFactory.terminate();
+            testFactory.assertNoRequestChanged();
+        } finally {
             mCm.unregisterNetworkCallback(cellNetworkCallback);
             handlerThread.quit();
         }