Tendrl / gluster-integration

Extracts all data from a Gluster cluster for consumption by Tendrl
GNU Lesser General Public License v2.1
6 stars 20 forks source link

Added some mock test to cover 'sds_sync/__init__.py' file. #698

Open aruniiird opened 6 years ago

aruniiird commented 6 years ago

Please note, this is not complete and has some failures. Contains some minor changes made to 'sds_sync/init.py' itself.

centos-ci commented 6 years ago

Can one of the admins verify this patch?

centos-ci commented 6 years ago

Can one of the admins verify this patch?

aruniiird commented 6 years ago

@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 notifications@github.com wrote:

@shtripat commented on this pull request.

In tendrl/gluster_integration/tests/test_sds_sync.py https://github.com/Tendrl/gluster-integration/pull/698#discussion_r214862837 :

@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 https://github.com/Tendrl/gluster-integration/pull/698#discussion_r214863474 :

+

  • 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 https://github.com/Tendrl/gluster-integration/pull/698#discussion_r214863829 :

  • 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 https://github.com/Tendrl/gluster-integration/pull/698#pullrequestreview-152002074, or mute the thread https://github.com/notifications/unsubscribe-auth/AA2pAoK1KcuX1Mu0iGFbHApt_1YUc8saks5uXlWBgaJpZM4WXZp5 .

-- Thanks, Arun Kumar M

shtripat commented 6 years ago

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?

aruniiird commented 6 years ago

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 notifications@github.com 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 https://github.com/Tendrl/gluster-integration/pull/698#issuecomment-418579137, or mute the thread https://github.com/notifications/unsubscribe-auth/AA2pAr86zCmLYJGtrP8ySPt_wUmYzJPXks5uXzbGgaJpZM4WXZp5 .

-- Thanks, Arun Kumar M