)]}'
{"id":"openstack%2Fpython-magnumclient~398778","triplet_id":"openstack%2Fpython-magnumclient~master~Id722b11a46e62c3aa8555ef6bcf77ea334752249","project":"openstack/python-magnumclient","branch":"master","topic":"bp/magnumclient-ut-coverage","hashtags":[],"change_id":"Id722b11a46e62c3aa8555ef6bcf77ea334752249","subject":"Use assert_called_*/assert_not_called to verify mock calls","status":"MERGED","created":"2016-11-17 07:10:37.000000000","updated":"2016-11-22 16:30:50.000000000","submitted":"2016-11-22 16:30:50.000000000","submitter":{"_account_id":3,"name":"Jenkins","username":"jenkins"},"total_comment_count":2,"unresolved_comment_count":0,"has_review_started":true,"meta_rev_id":"1292465d3659d649517467795236bac236d13cae","_number":398778,"virtual_id_number":398778,"owner":{"_account_id":13861,"name":"yatin","email":"ykarel@redhat.com","username":"yatinkarel"},"actions":{},"labels":{"Verified":{"approved":{"_account_id":3,"name":"Jenkins","username":"jenkins"},"all":[{"value":0,"_account_id":15123,"name":"Drago","email":"drago.rosson@rackspace.com","username":"Drago"},{"value":2,"date":"2016-11-22 16:30:50.000000000","post_submit":true,"_account_id":3,"name":"Jenkins","username":"jenkins"},{"value":0,"_account_id":21469,"name":"Jaycen Grant","email":"jaycen.v.grant@intel.com","username":"jvgrant"},{"value":0,"_account_id":10206,"name":"Madhuri Kumari","email":"madhuri.kumari@intel.com","username":"Madhuri"}],"values":{"-2":"Fails","-1":"Doesn\u0027t seem to work"," 0":"No score","+1":"Works for me","+2":"Verified"},"description":"","default_value":0,"optional":true},"Code-Review":{"approved":{"_account_id":21469,"name":"Jaycen Grant","email":"jaycen.v.grant@intel.com","username":"jvgrant"},"recommended":{"_account_id":15123,"name":"Drago","email":"drago.rosson@rackspace.com","username":"Drago"},"all":[{"value":1,"date":"2016-11-22 08:00:51.000000000","permitted_voting_range":{"min":1,"max":1},"_account_id":15123,"name":"Drago","email":"drago.rosson@rackspace.com","username":"Drago"},{"value":0,"_account_id":3,"name":"Jenkins","username":"jenkins"},{"value":2,"date":"2016-11-22 16:23:31.000000000","_account_id":21469,"name":"Jaycen Grant","email":"jaycen.v.grant@intel.com","username":"jvgrant"},{"value":2,"date":"2016-11-18 14:04:53.000000000","_account_id":10206,"name":"Madhuri Kumari","email":"madhuri.kumari@intel.com","username":"Madhuri"}],"values":{"-2":"Do not merge","-1":"This patch needs further work before it can be merged"," 0":"No score","+1":"Looks good to me, but someone else must approve","+2":"Looks good to me (core reviewer)"},"description":"","value":1,"default_value":0,"optional":true},"Workflow":{"approved":{"_account_id":21469,"name":"Jaycen Grant","email":"jaycen.v.grant@intel.com","username":"jvgrant"},"all":[{"value":0,"_account_id":15123,"name":"Drago","email":"drago.rosson@rackspace.com","username":"Drago"},{"value":0,"_account_id":3,"name":"Jenkins","username":"jenkins"},{"value":1,"date":"2016-11-22 16:23:31.000000000","_account_id":21469,"name":"Jaycen Grant","email":"jaycen.v.grant@intel.com","username":"jvgrant"},{"value":0,"_account_id":10206,"name":"Madhuri Kumari","email":"madhuri.kumari@intel.com","username":"Madhuri"}],"values":{"-1":"Work in progress"," 0":"Ready for reviews","+1":"Approved"},"description":"","default_value":0,"optional":true},"Backport-Candidate":{"all":[{"value":0,"_account_id":15123,"name":"Drago","email":"drago.rosson@rackspace.com","username":"Drago"},{"value":0,"_account_id":3,"name":"Jenkins","username":"jenkins"},{"value":0,"_account_id":21469,"name":"Jaycen Grant","email":"jaycen.v.grant@intel.com","username":"jvgrant"},{"value":0,"_account_id":10206,"name":"Madhuri Kumari","email":"madhuri.kumari@intel.com","username":"Madhuri"}],"values":{"-2":"Do Not Backport","-1":"Not A Backport Candidate"," 0":"Backport Review Needed","+1":"Proposed Backport","+2":"Should Backport"},"description":"","default_value":0,"optional":true}},"removable_reviewers":[],"reviewers":{"REVIEWER":[{"_account_id":3,"name":"Jenkins","username":"jenkins"},{"_account_id":10206,"name":"Madhuri Kumari","email":"madhuri.kumari@intel.com","username":"Madhuri"},{"_account_id":15123,"name":"Drago","email":"drago.rosson@rackspace.com","username":"Drago"},{"_account_id":21469,"name":"Jaycen Grant","email":"jaycen.v.grant@intel.com","username":"jvgrant"}]},"pending_reviewers":{},"reviewer_updates":[{"updated":"2016-11-18 14:04:53.000000000","updated_by":{"_account_id":10206,"name":"Madhuri Kumari","email":"madhuri.kumari@intel.com","username":"Madhuri"},"reviewer":{"_account_id":10206,"name":"Madhuri Kumari","email":"madhuri.kumari@intel.com","username":"Madhuri"},"state":"REVIEWER"},{"updated":"2016-11-22 08:00:51.000000000","updated_by":{"_account_id":15123,"name":"Drago","email":"drago.rosson@rackspace.com","username":"Drago"},"reviewer":{"_account_id":15123,"name":"Drago","email":"drago.rosson@rackspace.com","username":"Drago"},"state":"REVIEWER"},{"updated":"2016-11-22 16:23:31.000000000","updated_by":{"_account_id":21469,"name":"Jaycen Grant","email":"jaycen.v.grant@intel.com","username":"jvgrant"},"reviewer":{"_account_id":21469,"name":"Jaycen Grant","email":"jaycen.v.grant@intel.com","username":"jvgrant"},"state":"REVIEWER"},{"updated":"2016-11-22 16:30:50.000000000","updated_by":{"_account_id":3,"name":"Jenkins","username":"jenkins"},"reviewer":{"_account_id":3,"name":"Jenkins","username":"jenkins"},"state":"REVIEWER"}],"messages":[{"id":"070ebf36df9421cd0b31e033306bce2b99842e58","author":{"_account_id":13861,"name":"yatin","email":"ykarel@redhat.com","username":"yatinkarel"},"date":"2016-11-17 07:10:37.000000000","message":"Uploaded patch set 1.","accounts_in_message":[],"_revision_number":1},{"id":"f88fb445357b40dbb5cb66c92f37ca116455bcec","author":{"_account_id":3,"name":"Jenkins","username":"jenkins"},"date":"2016-11-17 07:16:19.000000000","message":"Patch Set 1: Verified+1\n\nBuild succeeded (check pipeline).\n\n- gate-python-magnumclient-docs-ubuntu-xenial http://docs-draft.openstack.org/78/398778/1/check/gate-python-magnumclient-docs-ubuntu-xenial/2f839a1//doc/build/html/ : SUCCESS in 2m 13s\n- gate-python-magnumclient-pep8-ubuntu-xenial http://logs.openstack.org/78/398778/1/check/gate-python-magnumclient-pep8-ubuntu-xenial/6e02e6f/ : SUCCESS in 2m 24s\n- gate-python-magnumclient-python27-ubuntu-xenial http://logs.openstack.org/78/398778/1/check/gate-python-magnumclient-python27-ubuntu-xenial/898f8b8/ : SUCCESS in 5m 23s\n- gate-python-magnumclient-python34 http://logs.openstack.org/78/398778/1/check/gate-python-magnumclient-python34/fb69069/ : SUCCESS in 3m 42s\n- gate-python-magnumclient-python35 http://logs.openstack.org/78/398778/1/check/gate-python-magnumclient-python35/1333fcf/ : SUCCESS in 2m 19s","accounts_in_message":[],"_revision_number":1},{"id":"d76c1f33daeeff33b23622eb358ad298bb768b9a","author":{"_account_id":10206,"name":"Madhuri Kumari","email":"madhuri.kumari@intel.com","username":"Madhuri"},"date":"2016-11-18 14:04:53.000000000","message":"Patch Set 1: Code-Review+2\n\nThanks Yatin. LGTM :)","accounts_in_message":[],"_revision_number":1},{"id":"f5ca959859a0562215edb8b403cece99d9404ee8","author":{"_account_id":15123,"name":"Drago","email":"drago.rosson@rackspace.com","username":"Drago"},"date":"2016-11-18 21:13:21.000000000","message":"Patch Set 1: Code-Review-1\n\nThis patch adds a lot of duplication in the individual unit tests. Why not modify test_arg_success and test_arg_failure to accept a map of flags to pass with their values, along with the mock method to make sure that it was called (or not)? I have included an example for each.","accounts_in_message":[],"_revision_number":1},{"id":"fa572d80d121446d383df3db48f0cbcc36becde5","author":{"_account_id":15123,"name":"Drago","email":"drago.rosson@rackspace.com","username":"Drago"},"date":"2016-11-18 21:22:39.000000000","message":"Patch Set 1:\n\n(2 comments)\n\nI clicked post too soon. Maybe instead of modifying test_arg_success and test_arg_failure, you could create a wrapper method that would build the command string and pass it to test_arg_success and also call assert_called_once_with.","accounts_in_message":[],"_revision_number":1},{"id":"ce32abeb0e245c060426897573ebcb14f8d519cd","author":{"_account_id":13861,"name":"yatin","email":"ykarel@redhat.com","username":"yatinkarel"},"date":"2016-11-21 03:36:43.000000000","message":"Patch Set 1:\n\n@Drago,\n\nYes, test cases adds some duplication but it\u0027s more clean now.\n\n- Modifying base methods \"_test_arg_success\" and \"_test_arg_failure\" doesn\u0027t look\u0027s good as it would require some resource(cluster,bay,cluster-template, baymodels) specific differences(Example: create_timeout, bay_create_timeout).\n\n- Yes, Adding a wrapper(within the resource specific test module) can be thought of as we can handle resource specific changes separately.\nHere i can found lot of things has to be handled in the wrapper, The things that i can think off currently:-\n--\u003e Conversion of \u0027-\u0027(in client args) to \u0027_\u0027(in server args), While conversion we also need to consider deprecated params(flavor-id and flavor(on client side) translate to flavor(on server side)). Also, cluster-template(cluster_template_id)\n-\u003e consider difference in names for client args vs server args(like timeout(create_timeout or bay_create_timeout),\nPassing mock methods to wrapper(args failure is always not maped with assert_not_called(Example: deprecated params test cases)) So it need\u0027s to be considered in wrapper, for multiple deletion assert_has_calls is used currently(mock_args can also be passed to wrapper for this) \n\nSeeing above known points, it seems wrapper would become complex.\nPlease suggest.\nAlso, can it be tried after this bp and other bp(which is going to change server\u0027s args for deprecated params) as lot of other things has to be considered there.","accounts_in_message":[],"_revision_number":1},{"id":"e4000c6197d94db46bb4c2da14e32257fc1be371","author":{"_account_id":15123,"name":"Drago","email":"drago.rosson@rackspace.com","username":"Drago"},"date":"2016-11-22 08:00:51.000000000","message":"Patch Set 1: Code-Review+1\n\n@yatin I think you are right. It is also easier to see the command we want to test without the wrapper because it is a simple string. The wrapper would have to rebuild the command, and this could be a source of bugs. I would rather have easy-to-understand but verbose test code than clever and (possibly) buggy test code.\n\nLGTM.","accounts_in_message":[],"_revision_number":1},{"id":"87d7cf0af3a26cf5c67c8bb51e515243d9fb579d","author":{"_account_id":21469,"name":"Jaycen Grant","email":"jaycen.v.grant@intel.com","username":"jvgrant"},"date":"2016-11-22 16:23:31.000000000","message":"Patch Set 1: Code-Review+2 Workflow+1","accounts_in_message":[],"_revision_number":1},{"id":"617dc9fc46d932c62616b644f3b85fffec09069f","author":{"_account_id":3,"name":"Jenkins","username":"jenkins"},"date":"2016-11-22 16:23:42.000000000","message":"Patch Set 1: -Verified\n\nStarting gate jobs.\nhttp://status.openstack.org/zuul/","accounts_in_message":[],"_revision_number":1},{"id":"793851d0108af85f7764537bc320221cc857f001","date":"2016-11-22 16:30:50.000000000","message":"Change has been successfully merged into the git repository by Jenkins","accounts_in_message":[],"_revision_number":1},{"id":"43b866b78726b74322c491dfe61d2752716917a3","author":{"_account_id":3,"name":"Jenkins","username":"jenkins"},"date":"2016-11-22 16:30:50.000000000","message":"Patch Set 1: Verified+2\n\nBuild succeeded (gate pipeline).\n\n- gate-python-magnumclient-docs-ubuntu-xenial http://docs-draft.openstack.org/78/398778/1/gate/gate-python-magnumclient-docs-ubuntu-xenial/7e52a50//doc/build/html/ : SUCCESS in 2m 33s\n- gate-python-magnumclient-pep8-ubuntu-xenial http://logs.openstack.org/78/398778/1/gate/gate-python-magnumclient-pep8-ubuntu-xenial/9d834cc/ : SUCCESS in 3m 59s\n- gate-python-magnumclient-python27-ubuntu-xenial http://logs.openstack.org/78/398778/1/gate/gate-python-magnumclient-python27-ubuntu-xenial/8fe09fb/ : SUCCESS in 5m 31s\n- gate-python-magnumclient-python34 http://logs.openstack.org/78/398778/1/gate/gate-python-magnumclient-python34/9e5719a/ : SUCCESS in 2m 35s\n- gate-python-magnumclient-python35 http://logs.openstack.org/78/398778/1/gate/gate-python-magnumclient-python35/0445c33/ : SUCCESS in 2m 28s","accounts_in_message":[],"_revision_number":1}],"current_revision_number":1,"current_revision":"19e61f295ec22139c8f25f5e1cec09fa92e28647","revisions":{"19e61f295ec22139c8f25f5e1cec09fa92e28647":{"kind":"REWORK","_number":1,"created":"2016-11-17 07:10:37.000000000","uploader":{"_account_id":13861,"name":"yatin","email":"ykarel@redhat.com","username":"yatinkarel"},"ref":"refs/changes/78/398778/1","fetch":{"anonymous http":{"url":"https://review.opendev.org/openstack/python-magnumclient","ref":"refs/changes/78/398778/1","commands":{"Checkout":"git fetch https://review.opendev.org/openstack/python-magnumclient refs/changes/78/398778/1 \u0026\u0026 git checkout FETCH_HEAD","Cherry Pick":"git fetch https://review.opendev.org/openstack/python-magnumclient refs/changes/78/398778/1 \u0026\u0026 git cherry-pick FETCH_HEAD","Format Patch":"git fetch https://review.opendev.org/openstack/python-magnumclient refs/changes/78/398778/1 \u0026\u0026 git format-patch -1 --stdout FETCH_HEAD","Pull":"git pull https://review.opendev.org/openstack/python-magnumclient refs/changes/78/398778/1"}}},"commit":{"parents":[{"commit":"6e11ccb08238eefe31dcd4bb17766ac244b11569","subject":"Fix: some typos in unit test","web_links":[{"name":"gitea","tooltip":"Open in GitWeb","url":"https://opendev.org/openstack/python-magnumclient/commit/6e11ccb08238eefe31dcd4bb17766ac244b11569"}]}],"author":{"name":"yatin","email":"yatin.karel@nectechnologies.in","date":"2016-11-17 07:09:01.000000000","tz":330},"committer":{"name":"yatin","email":"yatin.karel@nectechnologies.in","date":"2016-11-17 07:09:35.000000000","tz":330},"subject":"Use assert_called_*/assert_not_called to verify mock calls","message":"Use assert_called_*/assert_not_called to verify mock calls\n\nCurrently assertTrue and assertFalse are used to check\nwheter a mock method is called. It is not checked whether\nmock method is called with correct set of arguments.\nWith this patch following modules uses assert_called_* and\nassert_not_called instead of assertTrue and assertFalse.\n\n- magnumclient/tests/v1/test_baymodels_shell.py\n- magnumclient/tests/v1/test_clustertemplates_shell.py\n\nChange-Id: Id722b11a46e62c3aa8555ef6bcf77ea334752249\nPartially-Implements: blueprint magnumclient-ut-coverage\n","web_links":[{"name":"gitea","tooltip":"Open in GitWeb","url":"https://opendev.org/openstack/python-magnumclient/commit/19e61f295ec22139c8f25f5e1cec09fa92e28647"}],"resolve_conflicts_web_links":[{"name":"gitea","tooltip":"Open in GitWeb","url":"https://opendev.org/openstack/python-magnumclient/commit/19e61f295ec22139c8f25f5e1cec09fa92e28647"}]},"branch":"refs/heads/master"}},"requirements":[],"submit_records":[],"submit_requirements":[]}