More tetheroffload VTS fixes for over-specified tests.

[1] Call addDownstream before removeDownstream in affirmative tests

In order to properly test removeDownstream() where the HAL might
reasonably reject downstreams not previously added we now call
addDownstream() first (for affirmative tests).

[2] Clarify when stopOffload() return values can be safely ignored.

Test: as follows
    - make vts -j30 BUILD_GOOGLE_VTS=true && \
      vts-tradefed run commandAndExit vts \
          --skip-all-system-status-check \
          --primary-abi-only \
          --skip-preconditions \
          --module VtsHalTetherOffloadControlV1_0Target \
          -l DEBUG
      still doesn't pass but it's better than before  :)
Bug: 65270149
Change-Id: I27a574bd2110e3a1626de343f6b57b9efb8cdf83

1 files changed, 48 insertions, 15 deletions

diff --git a/tetheroffload/control/1.0/vts/functional/VtsHalTetheroffloadControlV1_0TargetTest.cpp b/tetheroffload/control/1.0/vts/functional/VtsHalTetheroffloadControlV1_0TargetTest.cpp
index a9424bc5aa..52dd026a87 100644
--- a/tetheroffload/control/1.0/vts/functional/VtsHalTetheroffloadControlV1_0TargetTest.cpp
+++ b/tetheroffload/control/1.0/vts/functional/VtsHalTetheroffloadControlV1_0TargetTest.cpp
@@ -46,6 +46,12 @@ using android::hardware::tetheroffload::control::V1_0::NetworkProtocol;
 using android::hardware::Void;
 using android::sp;
 
+enum class ExpectBoolean {
+    Ignored = -1,
+    False = 0,
+    True = 1,
+};
+
 // We use #defines here so as to get local lamba captures and error message line numbers
 #define ASSERT_TRUE_CALLBACK                            \
     [&](bool success, std::string errMsg) {             \
@@ -112,7 +118,12 @@ class OffloadControlHidlTestBase : public testing::VtsHalHidlTargetTestBase {
         prepareControlHal();
     }
 
-    virtual void TearDown() override { stopOffload(false); }
+    virtual void TearDown() override {
+        // For good measure, we should try stopOffload() once more. Since we
+        // don't know where we are in HAL call test cycle we don't know what
+        // return code to actually expect, so we just ignore it.
+        stopOffload(ExpectBoolean::Ignored);
+    }
 
     // The IOffloadConfig HAL is tested more thoroughly elsewhere. He we just
     // setup everything correctly and verify basic readiness.
@@ -168,12 +179,21 @@ class OffloadControlHidlTestBase : public testing::VtsHalHidlTargetTestBase {
         initOffload(true);
     }
 
-    void stopOffload(const bool expected_result) {
+    void stopOffload(const ExpectBoolean value) {
         auto cb = [&](bool success, const hidl_string& errMsg) {
             if (!success) {
                 ALOGI("Error message: %s", errMsg.c_str());
             }
-            ASSERT_EQ(expected_result, success);
+            switch (value) {
+                case ExpectBoolean::False:
+                    ASSERT_EQ(false, success);
+                    break;
+                case ExpectBoolean::True:
+                    ASSERT_EQ(true, success);
+                    break;
+                case ExpectBoolean::Ignored:
+                    break;
+            }
         };
         const Return<void> ret = control->stopOffload(cb);
         ASSERT_TRUE(ret.isOk());
@@ -211,22 +231,22 @@ TEST_F(OffloadControlHidlTestBase, AdditionalInitsWithoutStopReturnFalse) {
     initOffload(false);
     initOffload(false);
     initOffload(false);
-    stopOffload(true);  // balance out initOffload(true)
+    stopOffload(ExpectBoolean::True);  // balance out initOffload(true)
 }
 
 // Check that calling stopOffload() without first having called initOffload() returns false.
 TEST_F(OffloadControlHidlTestBase, MultipleStopsWithoutInitReturnFalse) {
-    stopOffload(false);
-    stopOffload(false);
-    stopOffload(false);
+    stopOffload(ExpectBoolean::False);
+    stopOffload(ExpectBoolean::False);
+    stopOffload(ExpectBoolean::False);
 }
 
 // Check that calling stopOffload() after a complete init/stop cycle returns false.
 TEST_F(OffloadControlHidlTestBase, AdditionalStopsWithInitReturnFalse) {
     initOffload(true);
-    stopOffload(true);  // balance out initOffload(true)
-    stopOffload(false);
-    stopOffload(false);
+    stopOffload(ExpectBoolean::True);  // balance out initOffload(true)
+    stopOffload(ExpectBoolean::False);
+    stopOffload(ExpectBoolean::False);
 }
 
 // Check that calling setLocalPrefixes() without first having called initOffload() returns false.
@@ -307,7 +327,12 @@ class OffloadControlHidlTest : public OffloadControlHidlTestBase {
         setupControlHal();
     }
 
-    virtual void TearDown() override { stopOffload(true); }
+    virtual void TearDown() override {
+        // For good measure, we should try stopOffload() once more. Since we
+        // don't know where we are in HAL call test cycle we don't know what
+        // return code to actually expect, so we just ignore it.
+        stopOffload(ExpectBoolean::Ignored);
+    }
 };
 
 /*
@@ -577,16 +602,24 @@ TEST_F(OffloadControlHidlTest, AddDownstreamBogusPrefixFails) {
 TEST_F(OffloadControlHidlTest, RemoveDownstreamIPv4) {
     const hidl_string iface("dummy0");
     const hidl_string prefix("192.0.2.0/24");
-    const Return<void> ret = control->removeDownstream(iface, prefix, ASSERT_TRUE_CALLBACK);
-    EXPECT_TRUE(ret.isOk());
+    // First add the downstream, otherwise removeDownstream logic can reasonably
+    // return false for downstreams not previously added.
+    const Return<void> add = control->addDownstream(iface, prefix, ASSERT_TRUE_CALLBACK);
+    EXPECT_TRUE(add.isOk());
+    const Return<void> del = control->removeDownstream(iface, prefix, ASSERT_TRUE_CALLBACK);
+    EXPECT_TRUE(del.isOk());
 }
 
 // Test removeDownstream() works given an IPv6 prefix.
 TEST_F(OffloadControlHidlTest, RemoveDownstreamIPv6) {
     const hidl_string iface("dummy0");
     const hidl_string prefix("2001:db8::/64");
-    const Return<void> ret = control->removeDownstream(iface, prefix, ASSERT_TRUE_CALLBACK);
-    EXPECT_TRUE(ret.isOk());
+    // First add the downstream, otherwise removeDownstream logic can reasonably
+    // return false for downstreams not previously added.
+    const Return<void> add = control->addDownstream(iface, prefix, ASSERT_TRUE_CALLBACK);
+    EXPECT_TRUE(add.isOk());
+    const Return<void> del = control->removeDownstream(iface, prefix, ASSERT_TRUE_CALLBACK);
+    EXPECT_TRUE(del.isOk());
 }
 
 // Test removeDownstream() fails given all empty parameters.