Change in vdsm[master]: imagetickets: add tests
Nir Soffer has posted comments on this change. Change subject: imagetickets: add tests .. Patch Set 13: Code-Review+2 Verified+1 Tests still pass -- 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: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: imagetickets: add tests
gerrit-hooks has posted comments on this change. Change subject: imagetickets: add tests .. Patch Set 14: * Update tracker: IGNORE, no Bug-Url found * Set MODIFIED::IGNORE, no Bug-Url found. -- 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: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: imagetickets: add tests
Nir Soffer has submitted this change and it was merged. Change subject: imagetickets: add tests .. imagetickets: add tests Adding tests to the new imagetickets module. Change-Id: I2728851a91529ec35501f423d9a798af961fb82a Signed-off-by: Amit Aviram Reviewed-on: https://gerrit.ovirt.org/52900 Continuous-Integration: Jenkins CI Reviewed-by: Nir Soffer Tested-by: Nir Soffer --- M tests/Makefile.am A tests/imagetickets_test.py M vdsm/storage/imagetickets.py 3 files changed, 184 insertions(+), 3 deletions(-) Approvals: Nir Soffer: Verified; Looks good to me, approved Jenkins CI: Passed CI tests -- To view, visit https://gerrit.ovirt.org/52900 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I2728851a91529ec35501f423d9a798af961fb82a Gerrit-PatchSet: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: imagetickets: add tests
gerrit-hooks has posted comments on this change. Change subject: imagetickets: add tests .. Patch Set 13: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- 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: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: imagetickets: add tests
Nir Soffer has posted comments on this change. Change subject: imagetickets: add tests .. Patch Set 12: Amit, can you rebase and verify again? There is a path conflict, probably because of changes in the imagetickets in master. -- 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: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: imagetickets: add tests
Nir Soffer has posted comments on this change. Change subject: imagetickets: add tests .. Patch Set 12: Code-Review+2 -- 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: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: imagetickets: add tests
Amit Aviram has posted comments on this change. Change subject: imagetickets: add tests .. Patch Set 12: (1 comment) https://gerrit.ovirt.org/#/c/52900/12/vdsm/storage/imagetickets.py File vdsm/storage/imagetickets.py: > Is this change related to the patch ? Yes, it is essential for the tests that uhttp will be used in that-The code we had before failed when ovirt_image_daemon was not installed. Thanks for the review! Line 1: # Line 2: # Copyright 2009-2012 Red Hat, Inc. Line 3: # Line 4: # This program is free software; you can redistribute it and/or modify -- 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: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: imagetickets: add tests
Freddy Rolland has posted comments on this change. Change subject: imagetickets: add tests .. Patch Set 12: Code-Review+1 (1 comment) https://gerrit.ovirt.org/#/c/52900/12/vdsm/storage/imagetickets.py File vdsm/storage/imagetickets.py: Is this change related to the patch ? Line 1: # Line 2: # Copyright 2009-2012 Red Hat, Inc. Line 3: # Line 4: # This program is free software; you can redistribute it and/or modify -- 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: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: imagetickets: add tests
Amit Aviram has posted comments on this change. Change subject: imagetickets: add tests .. Patch Set 12: Verified+1 -- 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: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: imagetickets: add tests
Nir Soffer has posted comments on this change. Change subject: imagetickets: add tests .. Patch Set 12: Code-Review+1 Looks fine now, lets get another review. -- 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: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: imagetickets: add tests
gerrit-hooks has posted comments on this change. Change subject: imagetickets: add tests .. Patch Set 12: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- 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: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: imagetickets: add tests
Nir Soffer has posted comments on this change. Change subject: imagetickets: add tests .. Patch Set 11: The test module looks fine now, but it does not run in make check: http://jenkins.ovirt.org/job/vdsm_master_check-patch-fc23-x86_64/3334/artifact/exported-artifacts/htmlcov/_home_jenkins_workspace_vdsm_master_check-patch-fc23-x86_64_vdsm_vdsm_storage_imagetickets_py.html Please add it to tests/Makefile.am -- 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: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: imagetickets: add tests
Nir Soffer has posted comments on this change. Change subject: imagetickets: add tests .. Patch Set 10: (1 comment) https://gerrit.ovirt.org/#/c/52900/10/tests/imagetickets_test.py File tests/imagetickets_test.py: Line 121: Line 122: @MonkeyPatch(imagetickets, 'uhttp', FakeUHTTP()) Line 123: def test_res_header_error(self): Line 124: imagetickets.uhttp.response = \ Line 125: FakeResponse(status=300, headers={"content-length": "invalid"}) > np, can you please elaborate why? \ is error prone (e.g. try to add trailing space after it) and make it harder to read the code. If the lines are too long, rewrite the code. See also https://www.python.org/dev/peps/pep-0008/ Line 126: with self.assertRaises(se.ImageDaemonError): Line 127: imagetickets.remove_ticket("uuid") Line 128: Line 129: @MonkeyPatch(imagetickets, 'uhttp', FakeUHTTP()) -- 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: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: imagetickets: add tests
gerrit-hooks has posted comments on this change. Change subject: imagetickets: add tests .. Patch Set 11: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- 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: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: imagetickets: add tests
Amit Aviram has posted comments on this change. Change subject: imagetickets: add tests .. Patch Set 10: (2 comments) https://gerrit.ovirt.org/#/c/52900/10/tests/imagetickets_test.py File tests/imagetickets_test.py: Line 121: Line 122: @MonkeyPatch(imagetickets, 'uhttp', FakeUHTTP()) Line 123: def test_res_header_error(self): Line 124: imagetickets.uhttp.response = \ Line 125: FakeResponse(status=300, headers={"content-length": "invalid"}) > Please avoid \ - use this format: np, can you please elaborate why? Line 126: with self.assertRaises(se.ImageDaemonError): Line 127: imagetickets.remove_ticket("uuid") Line 128: Line 129: @MonkeyPatch(imagetickets, 'uhttp', FakeUHTTP()) Line 136: @MonkeyPatch(imagetickets, 'uhttp', FakeUHTTP()) Line 137: def test_image_daemon_error_ret(self): Line 138: imagetickets.uhttp.response = \ Line 139: FakeResponse(status=300, Line 140: data=u'{"image_daemon_message":' u'"content"}') > This should be one string - it works because Python joins multiple strings, Done Line 141: try: Line 142: imagetickets.remove_ticket("uuid") Line 143: except se.ImageDaemonError as e: Line 144: self.assertTrue("image_daemon_message=content" in e.value) -- 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: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: imagetickets: add tests
Nir Soffer has posted comments on this change. Change subject: imagetickets: add tests .. Patch Set 10: (2 comments) https://gerrit.ovirt.org/#/c/52900/10/tests/imagetickets_test.py File tests/imagetickets_test.py: Line 121: Line 122: @MonkeyPatch(imagetickets, 'uhttp', FakeUHTTP()) Line 123: def test_res_header_error(self): Line 124: imagetickets.uhttp.response = \ Line 125: FakeResponse(status=300, headers={"content-length": "invalid"}) Please avoid \ - use this format: imagetickets.uhttp.response = FakeResponse( status=300, headers={"content-length": "invalid"}) Same for other tests using this pattern. Line 126: with self.assertRaises(se.ImageDaemonError): Line 127: imagetickets.remove_ticket("uuid") Line 128: Line 129: @MonkeyPatch(imagetickets, 'uhttp', FakeUHTTP()) Line 136: @MonkeyPatch(imagetickets, 'uhttp', FakeUHTTP()) Line 137: def test_image_daemon_error_ret(self): Line 138: imagetickets.uhttp.response = \ Line 139: FakeResponse(status=300, Line 140: data=u'{"image_daemon_message":' u'"content"}') This should be one string - it works because Python joins multiple strings, like C, but it is confusing. u'{"image_daemon_message":"content"}' Line 141: try: Line 142: imagetickets.remove_ticket("uuid") Line 143: except se.ImageDaemonError as e: Line 144: self.assertTrue("image_daemon_message=content" in e.value) -- 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: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: imagetickets: add tests
Amit Aviram has posted comments on this change. Change subject: imagetickets: add tests .. Patch Set 9: (4 comments) https://gerrit.ovirt.org/#/c/52900/9/tests/imagetickets_test.py File tests/imagetickets_test.py: Line 53: GET = "GET" Line 54: PATCH = "PATCH" Line 55: PUT = "PUT" Line 56: Line 57: def __init__(self, response=FakeResponse()): > You cannot use mutable object as default value - all the tests using FakeUH Done Line 58: self.closed = False Line 59: self.response = response Line 60: Line 61: def UnixHTTPConnection(self, sock_path): Line 121: Line 122: @MonkeyPatch(imagetickets, 'uhttp', Line 123: FakeUHTTP( Line 124: FakeResponse(status=300, Line 125: headers={"content-length": "invalid"}))) > This works, but it will be easier to replace the response on the fake http Done Line 126: def test_res_header_error(self): Line 127: with self.assertRaises(se.ImageDaemonError): Line 128: imagetickets.remove_ticket("uuid") Line 129: Line 128: imagetickets.remove_ticket("uuid") Line 129: Line 130: @MonkeyPatch(imagetickets, 'uhttp', Line 131: FakeUHTTP(FakeResponse(status=300, Line 132: data=u"not a json string"))) > Same Done Line 133: def test_res_invalid_json_ret(self): Line 134: with self.assertRaises(se.ImageDaemonError): Line 135: imagetickets.remove_ticket("uuid") Line 136: Line 136: Line 137: @MonkeyPatch(imagetickets, 'uhttp', Line 138: FakeUHTTP(FakeResponse(status=300, Line 139: data=u'{"image_daemon_message":' Line 140: u'"content"}'))) > Same Done Line 141: def test_image_daemon_error_ret(self): Line 142: try: Line 143: imagetickets.remove_ticket("uuid") Line 144: except se.ImageDaemonError as e: -- 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: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: imagetickets: add tests
gerrit-hooks has posted comments on this change. Change subject: imagetickets: add tests .. Patch Set 10: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- 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: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: imagetickets: add tests
Amit Aviram has posted comments on this change. Change subject: imagetickets: add tests .. Patch Set 9: -Verified -- 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: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: imagetickets: add tests
Amit Aviram has posted comments on this change. Change subject: imagetickets: add tests .. Patch Set 9: Verified+1 -- 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: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: imagetickets: add tests
Nir Soffer has posted comments on this change. Change subject: imagetickets: add tests .. Patch Set 9: (4 comments) https://gerrit.ovirt.org/#/c/52900/9/tests/imagetickets_test.py File tests/imagetickets_test.py: Line 53: GET = "GET" Line 54: PATCH = "PATCH" Line 55: PUT = "PUT" Line 56: Line 57: def __init__(self, response=FakeResponse()): You cannot use mutable object as default value - all the tests using FakeUHHTP will share this object. Instead, do: def __init__(self, response=None): self.response = response or FakeResponse() Line 58: self.closed = False Line 59: self.response = response Line 60: Line 61: def UnixHTTPConnection(self, sock_path): Line 121: Line 122: @MonkeyPatch(imagetickets, 'uhttp', Line 123: FakeUHTTP( Line 124: FakeResponse(status=300, Line 125: headers={"content-length": "invalid"}))) This works, but it will be easier to replace the response on the fake http inside the test: imagetickets.uhttp.response = FakeResponse(...) This is safe since uhttp is new instance for this test, only this test share this fake response. Line 126: def test_res_header_error(self): Line 127: with self.assertRaises(se.ImageDaemonError): Line 128: imagetickets.remove_ticket("uuid") Line 129: Line 128: imagetickets.remove_ticket("uuid") Line 129: Line 130: @MonkeyPatch(imagetickets, 'uhttp', Line 131: FakeUHTTP(FakeResponse(status=300, Line 132: data=u"not a json string"))) Same Line 133: def test_res_invalid_json_ret(self): Line 134: with self.assertRaises(se.ImageDaemonError): Line 135: imagetickets.remove_ticket("uuid") Line 136: Line 136: Line 137: @MonkeyPatch(imagetickets, 'uhttp', Line 138: FakeUHTTP(FakeResponse(status=300, Line 139: data=u'{"image_daemon_message":' Line 140: u'"content"}'))) Same Line 141: def test_image_daemon_error_ret(self): Line 142: try: Line 143: imagetickets.remove_ticket("uuid") Line 144: except se.ImageDaemonError as e: -- 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: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: imagetickets: add tests
gerrit-hooks has posted comments on this change. Change subject: imagetickets: add tests .. Patch Set 9: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- 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: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: imagetickets: add tests
Amit Aviram has posted comments on this change. Change subject: imagetickets: add tests .. Patch Set 8: -Verified -- 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: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: imagetickets: add tests
Amit Aviram has posted comments on this change. Change subject: imagetickets: add tests .. Patch Set 8: Verified+1 -- 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: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: imagetickets: add tests
gerrit-hooks has posted comments on this change. Change subject: imagetickets: add tests .. Patch Set 8: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- 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: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: imagetickets: add tests
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:
Change in vdsm[master]: imagetickets: add tests
Nir Soffer has posted comments on this change. Change subject: imagetickets: add tests .. Patch Set 7: (10 comments) Nice to see 100% coverage! 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 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? 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 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 **kwargs. We should accespt the same signature that the real getheader excepts. Also the value returned can be more clear - what is wrong in "string"? def getheader(name, default=None): return "invalid content legnth" 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: def getheader(name, defualt=None): return "not a json string" 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: def read(amt=None): return ... 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, and return only the bytes requested by the caller. I think the best way to do this is implement getheader and read in our fake response like this: class FakeResponse(object): def __init__(self, status=200, reason="OK", headers=None, data=""): self.status = status self.reason = reason self.headers = headers or {"content-length": len(data)} self.file = io.StringIO(data) def getheader(self, name, default=None): return self.headers.get(name, default) def read(self, amt=None): return self.file.read(amt) Now we can construct the response we like for each test, without faking any response methods: imagetickets.uhttp.response = FakeResponse( code=400, reason="BadRequest", data={"key": "value"} ) 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: "image_daemon_message=message" in ..
Change in vdsm[master]: imagetickets: add tests
gerrit-hooks has posted comments on this change. Change subject: imagetickets: add tests .. Patch Set 7: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- 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 Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: imagetickets: add tests
gerrit-hooks has posted comments on this change. Change subject: imagetickets: add tests .. Patch Set 6: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- 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: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: imagetickets: add tests
gerrit-hooks has posted comments on this change. Change subject: imagetickets: add tests .. Patch Set 5: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- 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: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: imagetickets: add tests
Amit Aviram has posted comments on this change. Change subject: imagetickets: add tests .. Patch Set 4: -Verified -- 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: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: imagetickets: add tests
Amit Aviram has posted comments on this change. Change subject: imagetickets: add tests .. Patch Set 4: re: first issue, Done. re: second issue: I'm guessing you are not rebased, https://gerrit.ovirt.org/#/c/53279/ changed the error name -- 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: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: imagetickets: add tests
Nir Soffer has posted comments on this change. Change subject: imagetickets: add tests .. Patch Set 4: Code-Review-1 The tests fail here when ovirt_image_daemon is not installed: Looking at the failures, it seems that replacing has_image_daemon with setting uhttp to None will fix this issue: try: from ovirt_image_daemon import uhttp except ImportError: uhttp = None And later check: if uhttp is None: Second issue: with self.assertRaises(se.ImageDaemonUnsupported): AttributeError: 'module' object has no attribute 'ImageDaemonUnsupported' Looks like mistyping of the exception name. I wonder how did you verify :-) -- imagetickets_test.TestImageTickets test_add_ticket ERROR test_extend_ticket ERROR test_image_daemon_error('getheader', at 0x7ff73ec17398>)ERROR test_image_daemon_error('read', at 0x7ff73ec17410>)ERROR test_image_daemon_error('read', at 0x7ff73ec17488>)ERROR test_image_tickets_error() ERROR test_image_tickets_error()ERROR test_image_tickets_error() ERROR test_not_supported('add_ticket', [{}]) ERROR test_not_supported('extend_ticket', ['uuid', 300]) ERROR test_not_supported('remove_ticket', ['uuid']) ERROR test_remove_ticket ERROR == ERROR: test_add_ticket (imagetickets_test.TestImageTickets) -- Traceback (most recent call last): File "/home/nsoffer/src/vdsm/tests/monkeypatch.py", line 133, in wrapper return f(*args, **kw) File "/home/nsoffer/src/vdsm/tests/monkeypatch.py", line 132, in wrapper with MonkeyPatchScope([(module, name, that)]): File "/usr/lib64/python2.7/contextlib.py", line 17, in __enter__ return self.gen.next() File "/home/nsoffer/src/vdsm/tests/monkeypatch.py", line 108, in MonkeyPatchScope patch.apply() File "/home/nsoffer/src/vdsm/tests/monkeypatch.py", line 63, in apply old = getattr(module, name) AttributeError: 'module' object has no attribute 'uhttp' == ERROR: test_extend_ticket (imagetickets_test.TestImageTickets) -- Traceback (most recent call last): File "/home/nsoffer/src/vdsm/tests/monkeypatch.py", line 133, in wrapper return f(*args, **kw) File "/home/nsoffer/src/vdsm/tests/monkeypatch.py", line 132, in wrapper with MonkeyPatchScope([(module, name, that)]): File "/usr/lib64/python2.7/contextlib.py", line 17, in __enter__ return self.gen.next() File "/home/nsoffer/src/vdsm/tests/monkeypatch.py", line 108, in MonkeyPatchScope patch.apply() File "/home/nsoffer/src/vdsm/tests/monkeypatch.py", line 63, in apply old = getattr(module, name) AttributeError: 'module' object has no attribute 'uhttp' == ERROR: test_image_daemon_error('getheader', at 0x7ff73ec17398>) (imagetickets_test.TestImageTickets) -- Traceback (most recent call last): File "/home/nsoffer/src/vdsm/tests/testlib.py", line 73, in wrapper return f(self, *args) File "/home/nsoffer/src/vdsm/tests/monkeypatch.py", line 133, in wrapper return f(*args, **kw) File "/home/nsoffer/src/vdsm/tests/monkeypatch.py", line 132, in wrapper with MonkeyPatchScope([(module, name, that)]): File "/usr/lib64/python2.7/contextlib.py", line 17, in __enter__ return self.gen.next() File "/home/nsoffer/src/vdsm/tests/monkeypatch.py", line 108, in MonkeyPatchScope patch.apply() File "/home/nsoffer/src/vdsm/tests/monkeypatch.py", line 63, in apply old = getattr(module, name) AttributeError: 'module' object has no attribute 'uhttp' == ERROR: test_image_daemon_error('read', at 0x7ff73ec17410>) (imagetickets_test.TestImageTickets) -- Traceback (most recent call last): File "/home/nsoffer/src/vdsm/tests/testlib.py", line 73, in wrapper return f(self, *args) File "/home/nsoffer/src/vdsm/tests/monkeypatch.py", line 133, in wrapper return f(*args, **kw) File "/home/nsoffer/src/vdsm/tests/monkeypatch.py", line 132, in wrapper with MonkeyPatchScope([(module, name, that)]): File "/usr/lib64/python2.7/contextlib.py", line 17, in __enter__ return self.gen.next() File "/home/nsoffer/src/vdsm/tests/monkeypatch.py", line 108, in MonkeyPatchScope patch.apply() File "/home/nsoffer/src/vdsm/tests/monkeypatch.py", line 63, in apply old = g
Change in vdsm[master]: imagetickets: add tests
Amit Aviram has posted comments on this change. Change subject: imagetickets: add tests .. Patch Set 4: Verified+1 -- 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: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: imagetickets: add tests
gerrit-hooks has posted comments on this change. Change subject: imagetickets: add tests .. Patch Set 4: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- 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: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: imagetickets: Add tests.
Amit Aviram has abandoned this change. Change subject: imagetickets: Add tests. .. Abandoned -- To view, visit https://gerrit.ovirt.org/53331 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: abandon Gerrit-Change-Id: Ia1d414c96c06484365911804e438845dfe17cbb1 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: gerrit-hooks ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: imagetickets: Add tests.
gerrit-hooks has posted comments on this change. Change subject: imagetickets: Add tests. .. Patch Set 1: * Update tracker: IGNORE, no Bug-Url found -- To view, visit https://gerrit.ovirt.org/53331 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia1d414c96c06484365911804e438845dfe17cbb1 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: imagetickets: add tests
gerrit-hooks has posted comments on this change. Change subject: imagetickets: add tests .. Patch Set 3: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- 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: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: imagetickets: Add tests.
Amit Aviram has uploaded a new change for review. Change subject: imagetickets: Add tests. .. imagetickets: Add tests. Change-Id: Ia1d414c96c06484365911804e438845dfe17cbb1 Signed-off-by: Amit Aviram --- M tests/imagetickets_test.py 1 file changed, 45 insertions(+), 1 deletion(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/31/53331/1 diff --git a/tests/imagetickets_test.py b/tests/imagetickets_test.py index 556ac63..b32d144 100644 --- a/tests/imagetickets_test.py +++ b/tests/imagetickets_test.py @@ -62,6 +62,12 @@ self.status = 200 self.reason = "OK" +def getheader(*args, **kwargs): +return 0 + +def read (*args, **kwargs): +return "{}" + @expandPermutations class TestImageTickets(VdsmTestCase): @@ -73,7 +79,7 @@ ["remove_ticket", ["uuid"]], ]) def test_not_supported(self, method, args): -with self.assertRaises(se.ImageDeamonUnsupported): +with self.assertRaises(se.ImageDaemonUnsupported): func = getattr(imagetickets, method) func(*args) @@ -91,6 +97,44 @@ @MonkeyPatch(imagetickets, '_have_image_daemon', True) @MonkeyPatch(imagetickets, 'uhttp', FakeUHTTP()) +def test_extend_ticket(self): +timeout = 300 +imagetickets.extend_ticket("uuid", timeout) +expected = [ +("request", ("PATCH", "/tickets/uuid"), {"timeout": timeout}), +] + +self.assertTrue(imagetickets.uhttp.__calls__, expected) +self.assertTrue(imagetickets.uhttp.closed) + +@MonkeyPatch(imagetickets, '_have_image_daemon', True) +@MonkeyPatch(imagetickets, 'uhttp', FakeUHTTP()) +def test_remove_ticket(self): +imagetickets.remove_ticket("uuid") +expected = [ +("request", ("PATCH", "/tickets/uuid"), None), +] + +self.assertTrue(imagetickets.uhttp.__calls__, expected) +self.assertTrue(imagetickets.uhttp.closed) + + +@MonkeyPatch(imagetickets, '_have_image_daemon', True) +@MonkeyPatch(imagetickets, 'uhttp', FakeUHTTP()) +@permutations([ +["getheader", lambda *args, **kwargs: "string"], +["read", lambda *args, **kwargs: (_ for _ in ()).throw(EnvironmentError)], +["read", lambda *args, **kwargs: "not json"], +]) +def test_image_daemon_error(self, func_name, fake_impl): +imagetickets.uhttp.response.status = 300 +setattr(imagetickets.uhttp.response, func_name, fake_impl) +with self.assertRaises(se.ImageDaemonError): +imagetickets.remove_ticket("uuid") + + +@MonkeyPatch(imagetickets, '_have_image_daemon', True) +@MonkeyPatch(imagetickets, 'uhttp', FakeUHTTP()) @permutations([[httplib.HTTPException], [socket.error], [OSError]]) def test_image_tickets_error(self, exc_type): ticket = create_ticket(uuid="uuid") -- To view, visit https://gerrit.ovirt.org/53331 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ia1d414c96c06484365911804e438845dfe17cbb1 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: imagetickets: Add tests.
gerrit-hooks has posted comments on this change. Change subject: imagetickets: Add tests. .. Patch Set 1: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/53331 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia1d414c96c06484365911804e438845dfe17cbb1 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: imagetickets: add tests
gerrit-hooks has posted comments on this change. Change subject: imagetickets: add tests .. Patch Set 2: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- 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: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: imagetickets: add tests
gerrit-hooks has posted comments on this change. Change subject: imagetickets: add tests .. Patch Set 1: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- 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: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: imagetickets: add tests
Amit Aviram has uploaded a new change for review. Change subject: imagetickets: add tests .. imagetickets: add tests Adding tests to the new imagetickets module. Change-Id: I2728851a91529ec35501f423d9a798af961fb82a Bug-Url: https://bugzilla.redhat.com/?? Signed-off-by: Amit Aviram --- A tests/imagetickets_test.py 1 file changed, 114 insertions(+), 0 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/00/52900/1 diff --git a/tests/imagetickets_test.py b/tests/imagetickets_test.py new file mode 100644 index 000..556ac63 --- /dev/null +++ b/tests/imagetickets_test.py @@ -0,0 +1,114 @@ +# +# Copyright 2016 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +# +# Refer to the README and COPYING files for full details of the license +# + +import httplib +import json +import socket + +from monkeypatch import MonkeyPatch +from testlib import VdsmTestCase +from testlib import expandPermutations, permutations +from testlib import recorded + +from storage import imagetickets +from storage import storage_exception as se + + +class FakeUHTTP(object): + +DELETE = "DELETE" +GET = "GET" +PATCH = "PATCH" +PUT = "PUT" + +def __init__(self): +self.closed = False +self.response = FakeResponse() + +def UnixHTTPConnection(self, sock_path): +return self + +@recorded +def request(self, method, path, body=None): +pass + +def getresponse(self): +return self.response + +def close(self): +self.closed = True + + +class FakeResponse(object): + +def __init__(self): +self.status = 200 +self.reason = "OK" + + +@expandPermutations +class TestImageTickets(VdsmTestCase): + +@MonkeyPatch(imagetickets, '_have_image_daemon', False) +@permutations([ +["add_ticket", [{}]], +["extend_ticket", ["uuid", 300]], +["remove_ticket", ["uuid"]], +]) +def test_not_supported(self, method, args): +with self.assertRaises(se.ImageDeamonUnsupported): +func = getattr(imagetickets, method) +func(*args) + +@MonkeyPatch(imagetickets, '_have_image_daemon', True) +@MonkeyPatch(imagetickets, 'uhttp', FakeUHTTP()) +def test_add_ticket(self): +ticket = create_ticket(uuid="uuid") +body = json.dumps(ticket) +expected = [ +("request", ("PUT", "/tickets/uuid"), {"body": body}), +] +imagetickets.add_ticket(ticket) +self.assertEqual(imagetickets.uhttp.__calls__, expected) +self.assertTrue(imagetickets.uhttp.closed) + +@MonkeyPatch(imagetickets, '_have_image_daemon', True) +@MonkeyPatch(imagetickets, 'uhttp', FakeUHTTP()) +@permutations([[httplib.HTTPException], [socket.error], [OSError]]) +def test_image_tickets_error(self, exc_type): +ticket = create_ticket(uuid="uuid") + +def failing_request(*args, **kwargs): +raise exc_type + +imagetickets.uhttp.request = failing_request +with self.assertRaises(se.ImageTicketsError): +imagetickets.add_ticket(ticket) + + +def create_ticket(uuid, ops=("read", "write"), timeout=300, + size=1024**3, path="/path/to/image"): +return { +"uuid": uuid, +"timeout": timeout, +"ops": list(ops), +"size": size, +"path": path, +} -- To view, visit https://gerrit.ovirt.org/52900 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I2728851a91529ec35501f423d9a798af961fb82a Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches