)]}'
{"nova/virt/xenapi/vm_utils.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"c01197c25d2e25450d85cdd9608480a48e92520d","unresolved":false,"context_lines":[{"line_number":1931,"context_line":""},{"line_number":1932,"context_line":"def _get_vhd_parent_uuid(session, vdi_ref, vdi_rec\u003dNone):"},{"line_number":1933,"context_line":"    if vdi_rec is None:"},{"line_number":1934,"context_line":"        vdi_rec \u003d session.call_xenapi(\"VDI.get_record\", vdi_ref)"},{"line_number":1935,"context_line":""},{"line_number":1936,"context_line":"    if \u0027vhd-parent\u0027 not in vdi_rec[\u0027sm_config\u0027]:"},{"line_number":1937,"context_line":"        return None"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F%2BHp0%3D","line":1934,"updated":"2013-12-02 11:47:36.000000000","message":"unit test for this please.","commit_id":"224028dec337aaaaae0254c76dd2320f9d2bc421"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"0c31c99639552bd23183a838f853d58a3868ebcd","unresolved":false,"context_lines":[{"line_number":1931,"context_line":""},{"line_number":1932,"context_line":"def _get_vhd_parent_uuid(session, vdi_ref, vdi_rec\u003dNone):"},{"line_number":1933,"context_line":"    if vdi_rec is None:"},{"line_number":1934,"context_line":"        vdi_rec \u003d session.call_xenapi(\"VDI.get_record\", vdi_ref)"},{"line_number":1935,"context_line":""},{"line_number":1936,"context_line":"    if \u0027vhd-parent\u0027 not in vdi_rec[\u0027sm_config\u0027]:"},{"line_number":1937,"context_line":"        return None"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F9dWI%3D","line":1934,"in_reply_to":"AAAATn%2F%2BHdg%3D","updated":"2013-12-11 10:13:26.000000000","message":"I would like a test that verifies VDI.get_record is not called when you pass in a vdi_rec. i.e. I would like a unit test to test the new functionality you have added to this method.\n\nhttps://github.com/openstack-dev/hacking/blob/master/doc/source/index.rst#creating-unit-tests\n\nNow a reasonable rebuttal is that this code is already covered by the existing unit tests. But then we don\u0027t get the implicit documentation the above unit test gives us.\n\nThe idea of being hard line on this is to slow increase test coverage throughout the code base. If we just raise a bug, no one will every get around to those.\n\nThe problem about speed is because we have integration tests that call out the database in our unit test tree, which is a bit broken.","commit_id":"224028dec337aaaaae0254c76dd2320f9d2bc421"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"4603904f6d39151cf6f36ecac9727c1d17d6656f","unresolved":false,"context_lines":[{"line_number":1931,"context_line":""},{"line_number":1932,"context_line":"def _get_vhd_parent_uuid(session, vdi_ref, vdi_rec\u003dNone):"},{"line_number":1933,"context_line":"    if vdi_rec is None:"},{"line_number":1934,"context_line":"        vdi_rec \u003d session.call_xenapi(\"VDI.get_record\", vdi_ref)"},{"line_number":1935,"context_line":""},{"line_number":1936,"context_line":"    if \u0027vhd-parent\u0027 not in vdi_rec[\u0027sm_config\u0027]:"},{"line_number":1937,"context_line":"        return None"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F%2BHdg%3D","line":1934,"in_reply_to":"AAAATn%2F%2BHp0%3D","updated":"2013-12-02 12:44:13.000000000","message":"What you want a unit test for this change to do?\n\nI think a test would only be able to verify that the unit test provides the vdi_rec in the same format as XAPI provides it - which wouldn\u0027t help with refactoring or verifying the method\u0027s behaviour.  I\u0027m struggling to see when that would be useful, and since there are other efforts to reduce the time in unit tests I don\u0027t really see the benefit.\n\nOf course, there should be tests for the existing function that returns a dummy VDI.get_record - as there are not I can raise a bug saying that a new unit test should be written for the existing code (although I\u0027m hesitant about even that because there are lots of \"improvement\" bugs and a new bug each time we find existing code that isn\u0027t unit tested seems OTT)","commit_id":"224028dec337aaaaae0254c76dd2320f9d2bc421"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"b5378e2a168bc0b8b069a4da7102b00bb1732ad2","unresolved":false,"context_lines":[{"line_number":1931,"context_line":""},{"line_number":1932,"context_line":"def _get_vhd_parent_uuid(session, vdi_ref, vdi_rec\u003dNone):"},{"line_number":1933,"context_line":"    if vdi_rec is None:"},{"line_number":1934,"context_line":"        vdi_rec \u003d session.call_xenapi(\"VDI.get_record\", vdi_ref)"},{"line_number":1935,"context_line":""},{"line_number":1936,"context_line":"    if \u0027vhd-parent\u0027 not in vdi_rec[\u0027sm_config\u0027]:"},{"line_number":1937,"context_line":"        return None"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F9c9M%3D","line":1934,"in_reply_to":"AAAATn%2F9dWI%3D","updated":"2013-12-11 11:38:56.000000000","message":"As I said on IRC, I\u0027ve added a test which shows that VDI.get_record is not called.\n\nOf course I\u0027m aware of the guidelines for creating new tests; but it\u0027s a stretch to call this change a \"feature\", and I still believe that this particular unit test doesn\u0027t really add value that\u0027s worth the extra maintenance","commit_id":"224028dec337aaaaae0254c76dd2320f9d2bc421"}]}