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 <[email protected]> Gerrit-Reviewer: Amit Aviram <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <[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
