-
Notifications
You must be signed in to change notification settings - Fork 20
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 some mock test to cover 'sds_sync/__init__.py' file. #698
base: master
Are you sure you want to change the base?
Conversation
this is not complete and has some failures. Some minor changes made to 'sds_sync/__init__.py' itself.
Can one of the admins verify this patch? |
1 similar comment
Can one of the admins verify this patch? |
|
||
@patch.object(BaseObject, "save") | ||
@patch.object(BaseObject, "load_all") | ||
@patch.object(event_utils, "emit_event") | ||
@patch.object(BaseObject, "hash_compare_with_central_store") | ||
@patch.object(etcd_utils, "refresh") | ||
def test_brick_status_alert( | ||
compare, refresh, emit_event, load_all, save | ||
refresh, compare, emit_event, load_all, save |
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.
Does this have any effect by changing the order only?
sds_sync = importlib.import_module( | ||
'tendrl.gluster_integration.sds_sync' | ||
) | ||
with patch.object(etcd_utils, "read") as utils_read: |
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.
@GowthamShanmugam can you please check this and suggest?
# below is a dummy assert, which should be replaced by | ||
# other assertions to get the code coverage | ||
# and check / mock some exceptions | ||
subproc_call.assert_called_once() |
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.
Are you planning to have another PR to take care?
>
@patch.object(BaseObject, "save")
@patch.object(BaseObject, "load_all")
@patch.object(event_utils, "emit_event")
@patch.object(BaseObject, "hash_compare_with_central_store")
@patch.object(etcd_utils, "refresh")
def test_brick_status_alert(
- compare, refresh, emit_event, load_all, save
+ refresh, compare, emit_event, load_all, save
Does this have any effect by changing the order only?
In here, there is no difference, only changed it as part of correctness.
Explanation:
'@patch' decorators will provide the arguments in the form of last in first
out basis. That means (in the above sample code) the last
@patch.object(etcd_utils, "refresh") call will provide the first argument.
Are you planning to have another PR to take care?
Yes, I am. Currently I am hitting the following failure message while
running 'tox',
try:
etcd_utils.read(
'/clusters/%s/_sync_now' %
NS.tendrl_context.integration_id
)
E AttributeError: 'NoneType' object has no attribute
'integration_id'
tendrl/gluster_integration/sds_sync/__init__.py:364: AttributeError
…________________
Once the failure is taken care, I could add other mock assertions.
Thanks,
Arun
On Tue, Sep 4, 2018 at 3:57 PM Shubhendu ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In tendrl/gluster_integration/tests/test_sds_sync.py
<#698 (comment)>
:
>
@patch.object(BaseObject, "save")
@patch.object(BaseObject, "load_all")
@patch.object(event_utils, "emit_event")
@patch.object(BaseObject, "hash_compare_with_central_store")
@patch.object(etcd_utils, "refresh")
def test_brick_status_alert(
- compare, refresh, emit_event, load_all, save
+ refresh, compare, emit_event, load_all, save
Does this have any effect by changing the order only?
------------------------------
In tendrl/gluster_integration/tests/test_sds_sync.py
<#698 (comment)>
:
> +
+ def dummy_callable():
+ pass
+ setattr(NS, "tendrl_context", maps.NamedDict())
+ NS.tendrl_context["integration_id"] = "int-id"
+ NS.tendrl_context["cluster_name"] = "cluster1"
+ NS.tendrl_context["sds_name"] = "gluster"
+ # in below line, passing a callable function to 'load' key,
+ # as the tox was complaining about the value being not callable
+ # assigning a MagicMock() object, making it indefinitely long
+ NS.tendrl_context["load"] = dummy_callable
+ NS.publisher_id = "gluster-integration"
+ sds_sync = importlib.import_module(
+ 'tendrl.gluster_integration.sds_sync'
+ )
+ with patch.object(etcd_utils, "read") as utils_read:
@GowthamShanmugam <https://github.com/GowthamShanmugam> can you please
check this and suggest?
------------------------------
In tendrl/gluster_integration/tests/test_sds_sync.py
<#698 (comment)>
:
> + # assigning a MagicMock() object, making it indefinitely long
+ NS.tendrl_context["load"] = dummy_callable
+ NS.publisher_id = "gluster-integration"
+ sds_sync = importlib.import_module(
+ 'tendrl.gluster_integration.sds_sync'
+ )
+ with patch.object(etcd_utils, "read") as utils_read:
+ # not sure why the below mock not working
+ utils_read.return_value = maps.NamedDict(
+ value='{"tags":[]}'
+ )
+ sds_sync.GlusterIntegrationSdsSyncStateThread().run()
+ # below is a dummy assert, which should be replaced by
+ # other assertions to get the code coverage
+ # and check / mock some exceptions
+ subproc_call.assert_called_once()
Are you planning to have another PR to take care?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#698 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AA2pAoK1KcuX1Mu0iGFbHApt_1YUc8saks5uXlWBgaJpZM4WXZp5>
.
--
Thanks,
Arun Kumar M
|
Shouldn't we mock etcd_utils.read and return a valid value for key |
I have added a mock call for 'etcd_utils.read' method (but the return value
may not be right), but the call was evaluating the arguments passed to it
(as the mock calls evaluate the passed arguments).
Thanks,
Arun
…On Wed, Sep 5, 2018 at 7:58 AM Shubhendu ***@***.***> wrote:
E AttributeError: 'NoneType' object has no attribute 'integration_id'
Shouldn't we mock etcd_utils.read and return a valid value for key
/clusters/{int-id}/_sync_now?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#698 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AA2pAr86zCmLYJGtrP8ySPt_wUmYzJPXks5uXzbGgaJpZM4WXZp5>
.
--
Thanks,
Arun Kumar M
|
Please note, this is not complete and has some failures.
Contains some minor changes made to 'sds_sync/init.py' itself.