Francesco Romani has posted comments on this change.

Change subject: virt: common handling of exceptions
......................................................................


Patch Set 9:

(3 comments)

https://gerrit.ovirt.org/#/c/54664/9/tests/virt_api_test.py
File tests/virt_api_test.py:

Line 43:     def test_success_with_args(self):
Line 44:         status = { 'vmName': 'foo', 'vmId': 'bar' }
Line 45:         self.assertRaises(TypeError,
Line 46:                           self.vm.succeed_with_args,
Line 47:                           status)
> This is a strange success
It is. It is the weird mix of test with positional args and wrong type. Let me 
clean this mess.
Line 48: 
Line 49:     def test_success_with_kwargs(self):
Line 50:         port = 0
Line 51:         res = self.vm.succeed_with_kwargs(migrationPort=port)


Line 70:         self.assertEquals(res, expected)
Line 71: 
Line 72:     def test_fail_with_plain_exception(self):
Line 73:         res = self.vm.fail(ValueError)
Line 74:         self.assertTrue(response.is_error(res))
> It seems that we control both the error handler and the code raising the ex
Good point, your suggestion is an improvement.
I think this is reasonnable and not too fragile, implemented.
Line 75: 
Line 76: 
Line 77: class FakeVM(object):
Line 78: 


Line 89:         return ret
Line 90: 
Line 91:     @api.method
Line 92:     def succeed_with_args(self, *args):
Line 93:         return args
> We can use single method for these tests:
let me always use a dict
Line 94: 
Line 95:     @api.method
Line 96:     def succeed_with_kwargs(self, **kwargs):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic186dc8fa062d8b3789c6057bba68fbbc23f311b
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <[email protected]>
Gerrit-Reviewer: Adam Litke <[email protected]>
Gerrit-Reviewer: Amit Aviram <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Francesco Romani <[email protected]>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: Piotr Kliczewski <[email protected]>
Gerrit-Reviewer: Vinzenz Feenstra <[email protected]>
Gerrit-Reviewer: gerrit-hooks <[email protected]>
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to