Change in vdsm[master]: imagetickets: add tests

aaviram at redhat.com aaviram at redhat.com
Sun Feb 28 12:17:26 UTC 2016


Amit Aviram has posted comments on this change.

Change subject: imagetickets: add tests
......................................................................


Patch Set 7:

(10 comments)

https://gerrit.ovirt.org/#/c/52900/7/tests/imagetickets_test.py
File tests/imagetickets_test.py:

Line 101:         expected = [
Line 102:             ("request", ("PATCH", "/tickets/uuid"), {"timeout": timeout}),
Line 103:         ]
Line 104: 
Line 105:         self.assertTrue(imagetickets.uhttp.__calls__, expected)
> assertEqual
Done
Line 106:         self.assertTrue(imagetickets.uhttp.closed)
Line 107: 
Line 108:     @MonkeyPatch(imagetickets, 'uhttp', FakeUHTTP())
Line 109:     def test_remove_ticket(self):


Line 108:     @MonkeyPatch(imagetickets, 'uhttp', FakeUHTTP())
Line 109:     def test_remove_ticket(self):
Line 110:         imagetickets.remove_ticket("uuid")
Line 111:         expected = [
Line 112:             ("request", ("PATCH", "/tickets/uuid"), None),
> Patch?
Done
Line 113:         ]
Line 114: 
Line 115:         self.assertTrue(imagetickets.uhttp.__calls__, expected)
Line 116:         self.assertTrue(imagetickets.uhttp.closed)


Line 111:         expected = [
Line 112:             ("request", ("PATCH", "/tickets/uuid"), None),
Line 113:         ]
Line 114: 
Line 115:         self.assertTrue(imagetickets.uhttp.__calls__, expected)
> assertEqual - this error hides the bad expected value
Done
Line 116:         self.assertTrue(imagetickets.uhttp.closed)
Line 117: 
Line 118:     @MonkeyPatch(imagetickets, 'uhttp', FakeUHTTP())
Line 119:     def test_res_header_error(self):


Line 118:     @MonkeyPatch(imagetickets, 'uhttp', FakeUHTTP())
Line 119:     def test_res_header_error(self):
Line 120:         imagetickets.uhttp.response.status = 300
Line 121: 
Line 122:         def str_ret(*args, **kwargs):
> getheader would be a better name, and we don't need to accept *args and **k
Taking the approach you suggested, so this change is irrelevant.
Line 123:             return "string"
Line 124: 
Line 125:         imagetickets.uhttp.response.getheader = str_ret
Line 126:         with self.assertRaises(se.ImageDaemonError):


Line 129:     @MonkeyPatch(imagetickets, 'uhttp', FakeUHTTP())
Line 130:     def test_res_invalid_json_ret(self):
Line 131:         imagetickets.uhttp.response.status = 300
Line 132: 
Line 133:         def not_json_ret(*args, **kwargs):
> Same as previous test:
Taking the approach you suggested, so this change is irrelevant.
Line 134:             return "string"
Line 135: 
Line 136:         imagetickets.uhttp.response.read = not_json_ret
Line 137:         with self.assertRaises(se.ImageDaemonError):


Line 140:     @MonkeyPatch(imagetickets, 'uhttp', FakeUHTTP())
Line 141:     def test_image_daemon_error_ret(self):
Line 142:         imagetickets.uhttp.response.status = 300
Line 143: 
Line 144:         def image_daemon_fake_ret(*args, **kwargs):
> Same:
Taking the approach you suggested, so this change is irrelevant.
Line 145:             return '{"image_daemon_message":"content"}'
Line 146: 
Line 147:         imagetickets.uhttp.response.read = image_daemon_fake_ret
Line 148:         try:


Line 143: 
Line 144:         def image_daemon_fake_ret(*args, **kwargs):
Line 145:             return '{"image_daemon_message":"content"}'
Line 146: 
Line 147:         imagetickets.uhttp.response.read = image_daemon_fake_ret
> We should also fake getheader to return the correct content_length value, a
Good idea, I'll go with that approach.
Line 148:         try:
Line 149:             imagetickets.remove_ticket("uuid")
Line 150:         except se.ImageDaemonError as e:
Line 151:             self.assertTrue("image_daemon_message" in e.value)


Line 147:         imagetickets.uhttp.response.read = image_daemon_fake_ret
Line 148:         try:
Line 149:             imagetickets.remove_ticket("uuid")
Line 150:         except se.ImageDaemonError as e:
Line 151:             self.assertTrue("image_daemon_message" in e.value)
> I think we expect:
It is actually "...=content", Done
Line 152: 
Line 153:     @MonkeyPatch(imagetickets, 'uhttp', FakeUHTTP())
Line 154:     def test_res_read_error(self):
Line 155:         imagetickets.uhttp.response.status = 300


Line 154:     def test_res_read_error(self):
Line 155:         imagetickets.uhttp.response.status = 300
Line 156:         err_msg = "Environment error message"
Line 157: 
Line 158:         def env_err(*args, **kwargs):
> Same:
Done
Line 159:             raise EnvironmentError(err_msg)
Line 160:         imagetickets.uhttp.response.read = env_err
Line 161: 
Line 162:         try:


Line 168:     @permutations([[httplib.HTTPException], [socket.error], [OSError]])
Line 169:     def test_image_tickets_error(self, exc_type):
Line 170:         ticket = create_ticket(uuid="uuid")
Line 171: 
Line 172:         def failing_request(*args, **kwargs):
> Better have the same signature of the real request. Will fail with TypeErro
Done
Line 173:             raise exc_type
Line 174: 
Line 175:         imagetickets.uhttp.request = failing_request
Line 176:         with self.assertRaises(se.ImageTicketsError):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2728851a91529ec35501f423d9a798af961fb82a
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Amit Aviram <aaviram at redhat.com>
Gerrit-Reviewer: Amit Aviram <aaviram at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list