subscription tests and support #108

martin-volf · 2023-03-16T17:31:49Z

No description provided.

Signed-off-by: Martin Volf <[email protected]>

jomiklos · 2023-03-16T18:42:31Z

confdgnmi/testtool/xr.yaml

@@ -10,3 +10,6 @@ enable_extra_logs: False # configuration of gNMI library - whether to enable int

 # test-case specific settings
 oc_interface: MgmtEth0/RP0/CPU0/0
+
+get-path: /interfaces


some comment to mark "subscribe" test related variables (plus short description of allowed value/meaning?)

jomiklos · 2023-03-16T18:43:49Z

confdgnmi/testtool/gNMI_Interface/SubscribeLibrary.py

+        self.requester: t.Optional[Requester] = None
+
+    def test_teardown(self) -> None:
+        self.paths = []


super().test_teardown() should be first item imho in case executive steps fail/raise exception...

jomiklos · 2023-03-16T18:58:11Z

confdgnmi/testtool/gNMI_Interface/04__Subscribe.robot

-Suite Setup      Setup gNMI Client
-Suite Teardown   Close gNMI Client
+Test Setup       Setup gNMI Client
+Test Teardown    Teardown gNMI state


Assuming you are sure we want new client for each test (not suite), shouldn't you close the client in [Test teardown] as well? (you removed invocation Close gNMI Client, and teardown_test() does not invoke close_client()!)

jomiklos · 2023-03-16T19:04:18Z

confdgnmi/testtool/gNMI_Interface/04__Subscribe.robot



 Subscribe ONCE with updates_only in the SubscriptionList
    # Failure: Device does not respond, responds with an error, responds with an empty notification set or with a notification without updates, responds with incorrect encoding.
    # Test is passed path to the data model, which contains limited number of elements. ONCE Subscription operation is invoked with updates_only field set in SubscriptionRequest. Test verifies only one SubscribeResponse with only sync_response set to true.
+    [Tags]    unimplemented

 Basic functionality of the Subscribe POLL RPC .


[W] 0301 Not allowed character '.' found in 'Basic functionality of the Subscribe POLL RPC .' test case name (not-allowed-char-in-name)

(output of robocop -e wrong-case-in-keyword-name ./testtool/ | grep Subscribe, there are few other instances...)

jomiklos · 2023-03-16T19:08:35Z

confdgnmi/testtool/gNMI_Interface/04__Subscribe.robot


 QOS marked subscriptions
+    [Tags]    unimplemented


imho adding also Skip KW as test "implementation" to silence some robocop lints allows easier resolution of actually failing tests vs non-existing ones... (same for other instances of "unimplemented" further)

jomiklos · 2023-03-16T19:18:02Z

confdgnmi/testtool/gNMI_Interface/04__Subscribe.robot

+    ...    response with "sync_response".
+    Given Subscription paths    ${GET-PATH}
+    And Subscription ONCE with encoding JSON_IETF
+    Then Device sends terminated response series and closes the stream


a bit too detailed/verbose imho, shorten at least a bit - maybe something like "Device terminates series on stream" - technical/interesting details can be clarified in [Documentation] of KW...

jomiklos · 2023-03-16T19:18:50Z

confdgnmi/testtool/gNMI_Interface/04__Subscribe.robot

+    ...    respond with a series of responses terminated by an empty
+    ...    response with "sync_response".
+    Given Subscription paths    ${GET-PATH}
+    And Subscription ONCE with encoding JSON_IETF


Subscription "mode" to clarify a bit?

jomiklos · 2023-03-16T19:25:02Z

confdgnmi/testtool/defaults.yaml

These values might be strongly specific to supported models per device itself...
Maybe this should be part of device configuration .yaml as standalone section?
IMHO no need to separate from other device settings - no need to maintain in two (or more) separate places and having to remember them, instead of one single .yaml (woth proper description/sections separation) passed to robot.

jomiklos · 2023-03-16T19:35:54Z

confdgnmi/testtool/gNMI_Interface/SubscribeLibrary.py

-                                                       encoding_str_to_int('JSON_IETF'))
-        responses = self._client.subscribe(slist)
-        return next(responses)
+    def __init__(self) -> None:


super().__init__()!

jomiklos · 2023-03-16T19:39:10Z

confdgnmi/testtool/gNMI_Interface/Subscribe.resource

[Documentation] missing across all KWs, but i assume that's to be filled in when tests stabilize/are cleaned up/de-linted later...

micnovak · 2023-03-17T07:41:54Z

confdgnmi/testtool/gNMI_Interface/04__Subscribe.robot

-    [Documentation]  Test that the device correctly responds to all three
-    ...    "Subscribe" request modes.  No further functionality (such as actually
+    [Documentation]    Test that the device correctly responds to all three
+    ...    "Subscribe" request modes.    No further functionality (such as actually


Does test close request on the server? Or does request remain open?

micnovak · 2023-03-17T07:51:17Z

confdgnmi/testtool/gNMI_Interface/04__Subscribe.robot

+    ...    send an initial set of responses terminated by an empty
+    ...    response with "sync_response".
+    Given Subscription paths    ${GET-PATH}
+    And Subscription POLL with encoding JSON_IETF


Description mentions STREAM but POLL is in test.

micnovak · 2023-03-17T07:56:48Z

confdgnmi/testtool/gNMI_Interface/04__Subscribe.robot


 STREAM with TARGET_DEFINED mode
+    [Tags]    unimplemented


I suggest to remove TARGET_DEFINED completely

micnovak · 2023-03-17T07:58:09Z

confdgnmi/testtool/gNMI_Interface/04__Subscribe.robot



 Subscribe ONCE with updates_only in the SubscriptionList
    # Failure: Device does not respond, responds with an error, responds with an empty notification set or with a notification without updates, responds with incorrect encoding.
    # Test is passed path to the data model, which contains limited number of elements. ONCE Subscription operation is invoked with updates_only field set in SubscriptionRequest. Test verifies only one SubscribeResponse with only sync_response set to true.
+    [Tags]    unimplemented


Improve text
" Test verifies only one SubscribeResponse is received with only sync_response set to true:

micnovak · 2023-03-17T08:01:33Z

confdgnmi/testtool/gNMI_Interface/04__Subscribe.robot



 Subscribe ONCE with updates_only in the SubscriptionList
    # Failure: Device does not respond, responds with an error, responds with an empty notification set or with a notification without updates, responds with incorrect encoding.
    # Test is passed path to the data model, which contains limited number of elements. ONCE Subscription operation is invoked with updates_only field set in SubscriptionRequest. Test verifies only one SubscribeResponse with only sync_response set to true.
+    [Tags]    unimplemented

 Basic functionality of the Subscribe POLL RPC .
    # Parameters: common connection parameters, path, poll count, poll interval.
    # Failure: Device does not respond, responds with an error, responds with an empty notification set or with a notification without updates, responds with incorrect encoding.
    # Test is passed path to the data model, which contains limited number of elements. Subscription operation is invoked. First SubscriptionRequest is ONCE. After that POLL subscription requests are invoked with poll interval delay. Test verifies for each subscription data is received. Optionally, it can verify the data is the same as the one received in response for ONCE subscription. Test issues WARNING, if SubscriptionResponse stream is closed prematurely. Test issues WARNING if Updates in SubscriptionResponse are aggregated.


First SubscriptionRequest is ONCE. - this is wrong, it should be First SubscriptionRequest is with POLL mode in SubscriptionList.

micnovak · 2023-03-17T08:01:48Z

confdgnmi/testtool/gNMI_Interface/04__Subscribe.robot


 Basic functionality of the Subscribe POLL RPC .
    # Parameters: common connection parameters, path, poll count, poll interval.
    # Failure: Device does not respond, responds with an error, responds with an empty notification set or with a notification without updates, responds with incorrect encoding.
    # Test is passed path to the data model, which contains limited number of elements. Subscription operation is invoked. First SubscriptionRequest is ONCE. After that POLL subscription requests are invoked with poll interval delay. Test verifies for each subscription data is received. Optionally, it can verify the data is the same as the one received in response for ONCE subscription. Test issues WARNING, if SubscriptionResponse stream is closed prematurely. Test issues WARNING if Updates in SubscriptionResponse are aggregated.
+    [Tags]    unimplemented

 Subscribe POLL RPC with updates_only in the SubscriptionList.
    # Parameters: common connection parameters, path, poll count, poll interval.
    # Failure: Device does not respond, responds with an error, responds with an empty notification set or with a notification without updates, responds with incorrect encoding.
    # Test is passed path to the data model, which contains limited number of elements. Subscription` operation is invoked. First SubscriptionRequest is ONCE with filled SubscriptionList containing updates_only set to true. This subscription is handled as ONCE subscription. After that, empty POLL subscription requests are invoked with poll interval delay. Test verifies for each subscription (also for initial ONCE) a SubscriptionResponse is received with only sync_response set to true (without other fields). Test issues WARNING, if SubscriptionResponse stream is closed prematurely. Test issues WARNING if Updates in SubscriptionResponse are aggregated.


First SubscriptionRequest is ONCE. - this is wrong, it should be First SubscriptionRequest is with POLL mode in SubscriptionList.

micnovak · 2023-03-17T08:20:20Z

confdgnmi/testtool/gNMI_Interface/SubscribeLibrary.py

+            while (response := self.response_queue.get(block=wait)) is not None:
+                yield response
+        except queue.Empty:
+            pass


Is there equivalent for responses.cancel() found in read_subscribe_responses in the client?

micnovak · 2023-03-17T08:20:41Z

confdgnmi/testtool/gNMI_Interface/SubscribeLibrary.py

+        super().__init__()
+
+    def run(self) -> None:
+        # TODO: this dumps the MultiThreaded thing exception for XR


How it will be handled? I suggest WARNING

subscription tests and support

238ba0d

Signed-off-by: Martin Volf <[email protected]>

jomiklos reviewed Mar 16, 2023

View reviewed changes

micnovak reviewed Mar 17, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

subscription tests and support #108

subscription tests and support #108

martin-volf commented Mar 16, 2023

jomiklos Mar 16, 2023

jomiklos Mar 16, 2023

jomiklos Mar 16, 2023

jomiklos Mar 16, 2023

jomiklos Mar 16, 2023

jomiklos Mar 16, 2023

jomiklos Mar 16, 2023

jomiklos Mar 16, 2023

jomiklos Mar 16, 2023

jomiklos Mar 16, 2023

micnovak Mar 17, 2023

micnovak Mar 17, 2023

micnovak Mar 17, 2023

micnovak Mar 17, 2023

micnovak Mar 17, 2023

micnovak Mar 17, 2023

micnovak Mar 17, 2023

micnovak Mar 17, 2023

subscription tests and support #108

Are you sure you want to change the base?

subscription tests and support #108

Conversation

martin-volf commented Mar 16, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment