-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
added config reset test fixture (empty for Demo) #35
Conversation
Signed-off-by: Martin Volf <[email protected]>
@@ -13,3 +13,6 @@ class TestGrpcDemo(AdapterTests): | |||
|
|||
def set_adapter_type(self): | |||
self.adapter_type = AdapterType.DEMO | |||
|
|||
def _do_reset_cfg(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Disadvantage of making new class. just for Set. Perhaps there can be common (abstract) ConfDTests class, so _do_reset_cfg
is implemented only once. And functionality could be implemented in this function (no need for reset_confd_config()).
@@ -73,6 +73,13 @@ def mk_gnmi_if_path(path_str, if_state_str="", if_id=None): | |||
path_str = path_str.format(if_state_str, if_id) | |||
return make_gnmi_path(path_str) | |||
|
|||
def _do_reset_cfg(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why underscore, as it is not private for the class? It is overridden in derived classes.
def _do_reset_cfg(self): | ||
raise NotImplementedError | ||
|
||
@pytest.fixture |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking about it, do we need extra fixture? I know it is slightly different, but for now adding _do_reset_cfg
into
existing fixture def fix_method(self, request):
would do.
The existing
reset_cfg
fixture was generalized for the full configuration and other tests that (potentially) change configuration (i.e. alltest_set*
). Implementation for Demo adapter tests is left empty for now.