Nir Soffer has posted comments on this change.

Change subject: yml: parameter type fixes for StoragePool.connect
......................................................................


Patch Set 1: Code-Review-1

(1 comment)

The schema fix looks good, I don't understand why we need the tests.

https://gerrit.ovirt.org/#/c/59702/1/tests/vdsmapi_test.py
File tests/vdsmapi_test.py:

Line 514:                   u'masterSdUUID': 
u'3d4accf7-74af-4ea0-b59b-eb9f10eedb83',
Line 515:                   u'hostID': 1}
Line 516: 
Line 517:         _schema.schema().verify_args(
Line 518:             vdsmapi.MethodRep('StoragePool', 'connect'), params)
What does this test? are we expecting this to raise if the schema does match 
params?

What if we change the schema - do we have to change params? I don't see the 
value in these tests, they are even harmful.

We should have tests for verify_args, but we don't need to test our schema.


-- 
To view, visit https://gerrit.ovirt.org/59702
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I19b6f25c17e697702ec61eba6b11f256c1df4d83
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <[email protected]>
Gerrit-Reviewer: Adam Litke <[email protected]>
Gerrit-Reviewer: Freddy Rolland <[email protected]>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: Piotr Kliczewski <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: gerrit-hooks <[email protected]>
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/admin/lists/[email protected]

Reply via email to