[oc_config_validate] Improve Subscription tests with multiple paths (#322)

* Improve Subscription tests with multiple paths.

 * Send a single SubscribeRequest with many paths is sent.
 * Do path inclusion comparison deal with sorted keys.
 * Compare Subscription Update paths with requested paths.
 * Check paths in Subscription tests.

* Small format corrections

Author: Jose Ignacio Tamayo Segarra

oc_config_validate/docs/testclasses.md

@@ -313,6 +313,8 @@ By default, the tests do 5 retries, with 15 seconds delay, if the assertion fail
 
 Uses gNMI Subscribe messages, of type ONCE.
 
+This testcase supports sending multiple xpaths in the gNMI Subscribe request.
+
 Optionally, every Update messages can have its timestamp value checked
   against the local time when the Subscription message was sent. The absolute
   time drift is compared against a max value in secs.

oc_config_validate/oc_config_validate/runner.py

@@ -228,7 +228,7 @@ def setInitConfigs(ctx: context.TestContext,
                          "applied at {init_config.xpath}")
         except (IOError, json.JSONDecodeError, target.BaseError) as err:
             msg = (f"Unable to set configuration '{init_config.filename}' "
-                   "at '{init_config.xpath}': {err}")
+                   f"at '{init_config.xpath}': {err}")
             if stop_on_error:
                 raise InitConfigError(msg) from err
             logging.error(msg)

oc_config_validate/oc_config_validate/schema.py

@@ -19,7 +19,7 @@
 import os
 import re
 from inspect import isclass
-from typing import Any, Iterator, List, Tuple, Union
+from typing import Any, List, Tuple, Union
 
 from pyangbind.lib import yangtypes
 from pyangbind.lib.base import PybindBase
@@ -302,25 +302,30 @@ def isPathOK(xpath: str) -> bool:
     return True
 
 
-def isPathIn(xpath: str, xpaths: List[str]) -> bool:
-    """Returns True if xpath in included or equal to any path in xpaths.
+def isPathInRequestedPaths(xpath: str, xpaths: List[str]) -> bool:
+    """Returns True if xpath in included or equal to any request path.
 
-    xpaths can contain paths with wildcard '*'.
+    The request paths can contain paths with wildcard '*'.
 
     Args:
       xpath: Non-wildcard xpath to check.
       xpaths: List of xpaths (can have wildcards) to contain xpath.
     """
     def _inOrEqual(x: str):
+        # Ensure the path has keys sorted
+        x = pathToString(parsePath(x))
         x_regex = re.escape(x).replace(r"\*", r"(.+)")
         return re.match(x_regex, xpath) is not None
 
+    xpath = pathToString(parsePath(xpath))
     return any(map(_inOrEqual, xpaths))
 
 
 def pathToString(path: gnmi_pb2.Path) -> str:
     """Parse a gNMI Path to a URI-like string.
 
+    The keys are sorted alphabetically.
+
     Args:
         A gNMI.Path oject.
 
@@ -335,7 +340,7 @@ def pathToString(path: gnmi_pb2.Path) -> str:
         elem = e.name
         if hasattr(e, "key"):
             keys = [f"[{k}={v}]" for k, v in e.key.items()]
-            elem += f"{''.join(keys)}"
+            elem += f"{''.join(sorted(keys))}"
         path_elems.append(elem)
     return "/" + "/".join(path_elems)
 
@@ -463,47 +468,43 @@ def intersectListCmp(want: list, got: list) -> Tuple[bool, str]:
     return True, ""
 
 
-def gNMISubscriptionOnceRequests(
+def gNMISubscriptionOnceRequest(
     xpaths: List[str],
     encoding: str = 'JSON_IETF'
-) -> Iterator[gnmi_pb2.SubscribeRequest]:
+) -> gnmi_pb2.SubscribeRequest:
     """
-    Returns an Iterator of gNMI Subscription ONCE requests for the xpaths.
+    Returns a gNMI Subscription ONCE requests for the xpaths.
 
     Args:
         xpaths: List of gNMI xpaths to subscribe to.
         encoding: Encoding requested for the Updates.
     """
     paths = [parsePath(xpath) for xpath in xpaths]
-    for r in [
-        gnmi_pb2.SubscribeRequest(subscribe=gnmi_pb2.SubscriptionList(
-            subscription=[gnmi_pb2.Subscription(path=path)
-                          for path in paths],
-            mode=gnmi_pb2.SubscriptionList.ONCE,
-            encoding=encoding))]:
-        yield r
+    return gnmi_pb2.SubscribeRequest(subscribe=gnmi_pb2.SubscriptionList(
+        subscription=[gnmi_pb2.Subscription(path=path)
+                      for path in paths],
+        mode=gnmi_pb2.SubscriptionList.ONCE,
+        encoding=encoding))
 
 
-def gNMISubscriptionStreamSampleRequests(
+def gNMISubscriptionStreamSampleRequest(
     xpaths: List[str],
     sample_interval: int,
     encoding: str = 'JSON_IETF'
-) -> Iterator[gnmi_pb2.SubscribeRequest]:
+) -> gnmi_pb2.SubscribeRequest:
     """
-    Returns a gNMI Subscription STREAM SAMPLE request for the xpath.
+    Returns a gNMI Subscription STREAM SAMPLE request for the xpaths.
 
     Args:
         xpaths: gNMI xpaths to subscribe to.
         sample_interval: Nanoseconds interval between Updates.
         encoding: Encoding requested for the Updates.
     """
     paths = [parsePath(xpath) for xpath in xpaths]
-    for r in [
-        gnmi_pb2.SubscribeRequest(subscribe=gnmi_pb2.SubscriptionList(
-            subscription=[gnmi_pb2.Subscription(
-                path=path,
-                mode=gnmi_pb2.SubscriptionMode.SAMPLE,
-                sample_interval=sample_interval) for path in paths],
-            mode=gnmi_pb2.SubscriptionList.STREAM,
-            encoding=encoding))]:
-        yield r
+    return gnmi_pb2.SubscribeRequest(subscribe=gnmi_pb2.SubscriptionList(
+        subscription=[gnmi_pb2.Subscription(
+            path=path,
+            mode=gnmi_pb2.SubscriptionMode.SAMPLE,
+            sample_interval=sample_interval) for path in paths],
+        mode=gnmi_pb2.SubscriptionList.STREAM,
+        encoding=encoding))

oc_config_validate/oc_config_validate/target.py

@@ -25,7 +25,7 @@
 import logging
 import ssl
 import time
-from typing import Any, Iterator, List, Optional, Tuple
+from typing import Any, List, Optional, Tuple
 
 import grpc
 from grpc._channel import _InactiveRpcError, _MultiThreadedRendezvous
@@ -278,7 +278,7 @@ def gNMISetConfigFile(self, file_path: str, xpath: str):
         self.gNMISetUpdate(xpath, json_data)
 
     def _gNMISubscribe(self,
-                       requests: Iterator[gnmi_pb2.SubscribeRequest],
+                       request: gnmi_pb2.SubscribeRequest,
                        timeout: int = 30,
                        check_sync_response: bool = False
                        ) -> List[gnmi_pb2.Notification]:
@@ -308,7 +308,7 @@ def _gNMISubscribe(self,
         got_sync_response = False
         try:
             for resp in self.stub.Subscribe(
-                    requests, timeout=timeout, metadata=auth):
+                    iter([request]), timeout=timeout, metadata=auth):
                 if resp.sync_response:
                     got_sync_response = True
                 elif resp.update:
@@ -336,8 +336,8 @@ def gNMISubsOnce(self, xpaths: List[str]) -> List[gnmi_pb2.Notification]:
         Returns:
             A list of gnmi_pb2.Notification objects received.
         """
-        requests = schema.gNMISubscriptionOnceRequests(xpaths)
-        return self._gNMISubscribe(requests)
+        request = schema.gNMISubscriptionOnceRequest(xpaths)
+        return self._gNMISubscribe(request)
 
     def gNMISubsStreamSample(self, xpath: str, sample_interval: int,
                              timeout: int) -> List[gnmi_pb2.Notification]:
@@ -353,9 +353,9 @@ def gNMISubsStreamSample(self, xpath: str, sample_interval: int,
             A list of gnmi_pb2.Notification objects received.
 
         """
-        requests = schema.gNMISubscriptionStreamSampleRequests(
+        request = schema.gNMISubscriptionStreamSampleRequest(
             [xpath], sample_interval)
-        return self._gNMISubscribe(requests, timeout=timeout)
+        return self._gNMISubscribe(request, timeout=timeout)
 
     def validate(self):
         """Ensures the Target is defined appropriately.

oc_config_validate/oc_config_validate/testcases/telemetry_once.py

@@ -66,6 +66,10 @@ def test100(self):
         for n in self.responses:
             updates += len(n.update)
             for u in n.update:
+                got_path = schema.pathToString(u.path)
+                self.assertTrue(
+                    schema.isPathInRequestedPaths(got_path, self.xpaths),
+                    f"Unexpected update path {got_path} for subscription")
                 self.assertTrue(
                     u.val.HasField(self.values_type),
                     f"Value of Update {schema.pathToString(u.path)} "
@@ -115,9 +119,8 @@ def test100(self):
             got_path = schema.pathToString(u.path)
             got_paths.append(got_path)
             self.assertTrue(
-                schema.isPathIn(got_path, want_paths),
-                f"Unexpected update path {got_path} for OC model {self.model},"
-                f"expected {want_paths}")
+                schema.isPathInRequestedPaths(got_path, want_paths),
+                f"Unexpected update path {got_path} for subscription")
 
         if self.check_missing_model_paths:
             for want_path in want_paths:

oc_config_validate/oc_config_validate/testcases/telemetry_sample.py

@@ -94,6 +94,7 @@ def testSubscribeSample(self):
             got_paths, self.update_paths_count,
             f"Expected {self.update_paths_count} Update paths, "
             f"got: {got_paths}")
+<<<<<<< HEAD
 
 
 class CheckLeafs(SubsSampleTestCase):
@@ -121,9 +122,8 @@ def testSubscribeSample(self):
         got_paths = len(self.responses)
         self.assertGreater(got_paths, 0,
                            "There are no Update replies to the Subscription")
-        if self.responses:
+       if self.responses:
             for path, _ in self.responses.items():
                 self.assertTrue(
-                    schema.isPathIn(path, want_paths),
-                    f"Unexpected update path {path} for subscription to"
-                    f" {self.xpath} with model {self.model}")
+                    schema.isPathInRequestedPaths(path, [self.xpath]),
+                    f"Unexpected update path {path} for subscription")

oc_config_validate/py_tests/test_schema.py

@@ -138,5 +138,54 @@ def test_pathStrings(self, name, path_str, is_ok):
             self.assertEqual(path_str, schema.pathToString(got_obj), name)
 
 
+class TestisPathInRequestedPathsRequestedPaths(unittest.TestCase):
+    """Test for isPathInRequestedPaths."""
+
+    @parameterized.expand([
+        ("_in", "/interfaces/interface[name=ethernet1/2]/state", True),
+        ("_deep_in",
+         "/interfaces/interface[name=ethernet1/2]/state/counters/out-errors",
+         True),
+        ("_not_int",
+         "/interfaces/interface[name=ethernet1/2]/config/name", False),
+    ])
+    def test_isPathInRequestedPathsRequestedPaths_singleReq(self, name, xpath,
+                                                            want):
+        request_xpaths = ["/interfaces/interface[name=ethernet1/2]/state"]
+        self.assertEqual(schema.isPathInRequestedPaths(
+            xpath, request_xpaths), want)
+
+    @parameterized.expand([
+        ("_in", "/interfaces/interface[name=ethernet1/0]/state", True),
+        ("_deep_in",
+         "/interfaces/interface[name=ethernet1/1]/state/counters/out-errors",
+         True),
+        ("_not_int",
+         "/interfaces/interface[name=ethernet1/2]/config/name", False),
+    ])
+    def test_isPathInRequestedPathsRequestedPaths_singleWcardReq(self, name,
+                                                                 xpath, want):
+        request_xpaths = ["/interfaces/interface[name=*]/state"]
+        self.assertEqual(schema.isPathInRequestedPaths(
+            xpath, request_xpaths), want)
+
+    @parameterized.expand([
+        ("_in", "/interfaces/interface[name=ethernet1/0][id=0]/state", True),
+        ("_deep_in",
+         "/interfaces/interface[id=1][name=1/1]/state/counters/out-errors",
+         True),
+        ("_not_int",
+         "/interfaces/interface[name=ethernet1/2]/config/state", False),
+    ])
+    def test_isPathInRequestedPathsRequestedPaths_manyKeysReq(self, name,
+                                                              xpath, want):
+        request_xpaths = ["/interfaces/interface[name=*][id=*]/state"]
+        self.assertEqual(schema.isPathInRequestedPaths(
+            xpath, request_xpaths), want)
+        request_xpaths = ["/interfaces/interface[id=*][name=*]/state"]
+        self.assertEqual(schema.isPathInRequestedPaths(
+            xpath, request_xpaths), want)
+
+
 if __name__ == '__main__':
     unittest.main()