commit 728cf4c9953c997fab2c820755f1157d7712dd2a
author Luke Huang <huangluke@google.com> Thu Mar 14 19:43:02 2019 +0800
committer Luke Huang <huangluke@google.com> Fri Mar 15 05:47:12 2019 +0000
tree 7281037df71f1b12ff3823b5f8c446c43b3ffca3
parent b257d61cd55c00a50d1eaaf4e7fcf436185c9a2c

refine test TestIpfwdEnableDisableStatusForwarding and some minor changes on test

1. fix flaky test for TestIpfwdEnableDisableStatusForwarding
2. fix time unit error in binder_test, set retry default
   interval as 50ms
3. refine naming in binder_test

Bug: 128533612
Test: built, flashed, booted
      system/netd/tests/runtests.sh pass

Change-Id: Ia03d0c30b49cdd233b627cce8bd2f3f05548798e

tests/binder_test.cpp

diff --git a/tests/binder_test.cpp b/tests/binder_test.cpp
index 2e425db..bb569e9 100644
--- a/tests/binder_test.cpp
+++ b/tests/binder_test.cpp
@@ -1283,7 +1283,7 @@
 namespace {
 
 std::vector<std::string> tryToFindProcesses(const std::string& processName, uint32_t maxTries = 1,
-                                            uint32_t intervalMs = 0) {
+                                            uint32_t intervalMs = 50) {
     // Output looks like:(clatd)
     // clat          4963   850 1 12:16:51 ?     00:00:00 clatd-netd10a88 -i netd10a88 ...
     // ...
@@ -1302,16 +1302,16 @@
             break;
         }
 
-        usleep(intervalMs);
+        usleep(intervalMs * 1000);
     }
     return result;
 }
 
 void expectProcessExists(const std::string& processName) {
-    EXPECT_EQ(1U, tryToFindProcesses(processName, 5 /*maxTries*/, 50 /*intervalMs*/).size());
+    EXPECT_EQ(1U, tryToFindProcesses(processName, 5 /*maxTries*/).size());
 }
 
-void expectProcessDoesNotExists(const std::string& processName) {
+void expectProcessDoesNotExist(const std::string& processName) {
     EXPECT_FALSE(tryToFindProcesses(processName).size());
 }
 
@@ -1370,13 +1370,13 @@
     // Expect clatd to stop successfully.
     status = mNetd->clatdStop(sTun.name());
     EXPECT_TRUE(status.isOk()) << status.exceptionMessage();
-    expectProcessDoesNotExists(clatdName);
+    expectProcessDoesNotExist(clatdName);
 
     // Stopping a clatd that doesn't exist returns ENODEV.
     status = mNetd->clatdStop(sTun.name());
     EXPECT_FALSE(status.isOk());
     EXPECT_EQ(ENODEV, status.serviceSpecificErrorCode());
-    expectProcessDoesNotExists(clatdName);
+    expectProcessDoesNotExist(clatdName);
 
     // Clean up.
     EXPECT_TRUE(mNetd->networkRemoveRoute(TEST_NETID1, sTun.name(), "::/0", "").isOk());
@@ -1438,25 +1438,56 @@
 }  // namespace
 
 TEST_F(BinderTest, TestIpfwdEnableDisableStatusForwarding) {
-    // Netd default enable Ipfwd with requester NetdHwService
-    const std::string defaultRequester = "NetdHwService";
-
-    binder::Status status = mNetd->ipfwdDisableForwarding(defaultRequester);
+    // Get ipfwd requester list from Netd
+    std::vector<std::string> requesterList;
+    binder::Status status = mNetd->ipfwdGetRequesterList(&requesterList);
     EXPECT_TRUE(status.isOk()) << status.exceptionMessage();
-    expectIpfwdEnable(false);
 
     bool ipfwdEnabled;
-    status = mNetd->ipfwdEnabled(&ipfwdEnabled);
-    EXPECT_TRUE(status.isOk()) << status.exceptionMessage();
-    EXPECT_FALSE(ipfwdEnabled);
+    if (requesterList.size() == 0) {
+        // No requester in Netd, ipfwd should be disabled
+        // So add one test requester and verify
+        status = mNetd->ipfwdEnableForwarding("TestRequester");
+        EXPECT_TRUE(status.isOk()) << status.exceptionMessage();
 
-    status = mNetd->ipfwdEnableForwarding(defaultRequester);
-    EXPECT_TRUE(status.isOk()) << status.exceptionMessage();
-    expectIpfwdEnable(true);
+        expectIpfwdEnable(true);
+        status = mNetd->ipfwdEnabled(&ipfwdEnabled);
+        EXPECT_TRUE(status.isOk()) << status.exceptionMessage();
+        EXPECT_TRUE(ipfwdEnabled);
 
-    status = mNetd->ipfwdEnabled(&ipfwdEnabled);
-    EXPECT_TRUE(status.isOk()) << status.exceptionMessage();
-    EXPECT_TRUE(ipfwdEnabled);
+        // Remove test one, verify again
+        status = mNetd->ipfwdDisableForwarding("TestRequester");
+        EXPECT_TRUE(status.isOk()) << status.exceptionMessage();
+
+        expectIpfwdEnable(false);
+        status = mNetd->ipfwdEnabled(&ipfwdEnabled);
+        EXPECT_TRUE(status.isOk()) << status.exceptionMessage();
+        EXPECT_FALSE(ipfwdEnabled);
+    } else {
+        // Disable all requesters
+        for (const auto& requester : requesterList) {
+            status = mNetd->ipfwdDisableForwarding(requester);
+            EXPECT_TRUE(status.isOk()) << status.exceptionMessage();
+        }
+
+        // After disable all requester, ipfwd should be disabled
+        expectIpfwdEnable(false);
+        status = mNetd->ipfwdEnabled(&ipfwdEnabled);
+        EXPECT_TRUE(status.isOk()) << status.exceptionMessage();
+        EXPECT_FALSE(ipfwdEnabled);
+
+        // Enable them back
+        for (const auto& requester : requesterList) {
+            status = mNetd->ipfwdEnableForwarding(requester);
+            EXPECT_TRUE(status.isOk()) << status.exceptionMessage();
+        }
+
+        // ipfwd should be enabled
+        expectIpfwdEnable(true);
+        status = mNetd->ipfwdEnabled(&ipfwdEnabled);
+        EXPECT_TRUE(status.isOk()) << status.exceptionMessage();
+        EXPECT_TRUE(ipfwdEnabled);
+    }
 }
 
 TEST_F(BinderTest, TestIpfwdAddRemoveInterfaceForward) {
@@ -2088,7 +2119,7 @@
 
     status = mNetd->tetherStop();
     EXPECT_TRUE(status.isOk()) << status.exceptionMessage();
-    expectProcessDoesNotExists(dnsdName);
+    expectProcessDoesNotExist(dnsdName);
 
     status = mNetd->tetherIsEnabled(&tetherEnabled);
     EXPECT_TRUE(status.isOk()) << status.exceptionMessage();