Conversation
Signed-off-by: Martin Volf <mvolf@cisco.com>
| # test-case specific settings | ||
| oc_interface: MgmtEth0/RP0/CPU0/0 | ||
|
|
||
| get-path: /interfaces |
There was a problem hiding this comment.
some comment to mark "subscribe" test related variables (plus short description of allowed value/meaning?)
| self.requester: t.Optional[Requester] = None | ||
|
|
||
| def test_teardown(self) -> None: | ||
| self.paths = [] |
There was a problem hiding this comment.
super().test_teardown() should be first item imho in case executive steps fail/raise exception...
| Suite Setup Setup gNMI Client | ||
| Suite Teardown Close gNMI Client | ||
| Test Setup Setup gNMI Client | ||
| Test Teardown Teardown gNMI state |
There was a problem hiding this comment.
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()!)
| # 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 . |
There was a problem hiding this comment.
[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...)
| Then Device sends terminated response series | ||
|
|
||
| QOS marked subscriptions | ||
| [Tags] unimplemented |
There was a problem hiding this comment.
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)
| ... 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 |
There was a problem hiding this comment.
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...
| ... 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 |
There was a problem hiding this comment.
Subscription "mode" to clarify a bit?
There was a problem hiding this comment.
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.
| encoding_str_to_int('JSON_IETF')) | ||
| responses = self._client.subscribe(slist) | ||
| return next(responses) | ||
| def __init__(self) -> None: |
There was a problem hiding this comment.
[Documentation] missing across all KWs, but i assume that's to be filled in when tests stabilize/are cleaned up/de-linted later...
| [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 |
There was a problem hiding this comment.
Does test close request on the server? Or does request remain open?
| ... 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 |
There was a problem hiding this comment.
Description mentions STREAM but POLL is in test.
| # idk., what to test here? | ||
|
|
||
| STREAM with TARGET_DEFINED mode | ||
| [Tags] unimplemented |
There was a problem hiding this comment.
I suggest to remove TARGET_DEFINED completely
| 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 |
There was a problem hiding this comment.
Improve text
" Test verifies only one SubscribeResponse is received with only sync_response set to true:
| 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. |
There was a problem hiding this comment.
First SubscriptionRequest is ONCE. - this is wrong, it should be First SubscriptionRequest is with POLL mode in SubscriptionList.
| 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. |
There was a problem hiding this comment.
First SubscriptionRequest is ONCE. - this is wrong, it should be First SubscriptionRequest is with POLL mode in SubscriptionList.
| while (response := self.response_queue.get(block=wait)) is not None: | ||
| yield response | ||
| except queue.Empty: | ||
| pass |
There was a problem hiding this comment.
Is there equivalent for responses.cancel() found in read_subscribe_responses in the client?
| super().__init__() | ||
|
|
||
| def run(self) -> None: | ||
| # TODO: this dumps the MultiThreaded thing exception for XR |
There was a problem hiding this comment.
How it will be handled? I suggest WARNING
3 participants
No description provided.