)]}'
{"nova/tests/unit/virt/libvirt/test_vif.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"952717c02811fe07f8f7f5d317c809cdcf638e75","unresolved":false,"context_lines":[{"line_number":503,"context_line":"            mock.patch.object(osinfo, \u0027OsInfoOs\u0027,"},{"line_number":504,"context_line":"                               return_value\u003dtest_osinfo_obj),"},{"line_number":505,"context_line":"            mock.patch.object(vif.designer, \u0027set_vif_guest_frontend_config\u0027,"},{"line_number":506,"context_line":"                               side_effect\u003dcheck_model_value)):"},{"line_number":507,"context_line":"            d \u003d vif.LibvirtGenericVIFDriver()"},{"line_number":508,"context_line":"            image_meta \u003d {\u0027properties\u0027: {\u0027os_name\u0027: \u0027fedora19\u0027}}"},{"line_number":509,"context_line":"            d.get_base_config(None, self.vif_bridge, image_meta,"}],"source_content_type":"text/x-python","patch_set":21,"id":"9a41bdd9_0105afc0","line":506,"updated":"2015-07-18 12:02:31.000000000","message":"As mentioned on a previous patch, it\u0027s easier and more explicit to assert a call structure instead of using side_effect.\n\nYou can decorate this test with mock patches and make the entire test method more explicit and simpler:\n\n @mock.patch.object(osinfo, \u0027OsInfoOs\u0027)\n @mock.patch.object(vif.designer, \u0027set_vif_guest_frontend_config\u0027)\n def test_model_with_osinfo(self, mock_set, mock_osinfo):\n     self.flags(use_virtio_for_bridges\u003dTrue,\n               virt_type\u003d\u0027kvm\u0027,\n               group\u003d\u0027libvirt\u0027)\n\n     test_osinfo_obj \u003d mock.Mock()\n     test_osinfo_obj.network_model \u003d \u0027virtio\u0027\n     test_osinfo_obj.disk_bus \u003d \u0027virtio\u0027\n     mock_osinfo.return_value \u003d test_osinfo_obj\n\n     d \u003d vif.LibvirtGenericVIFDriver()\n     image_meta \u003d {\u0027properties\u0027: {\u0027os_name\u0027: \u0027fedora19\u0027}}\n     d.get_base_config(None, self.vif_bridge, image_meta,\n                  None, \u0027kvm\u0027)\n     mock_set.assert_called_once_with(mock.ANY, mock.ANY, \u0027virtio\u0027, mock.ANY)\n\nEven better, replace the mock.ANY parameters with the actual expected arguments.","commit_id":"d2c658b4ac2d2b75cdf428156252feb8474c1d28"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"8983c030e7f5db389dcf5a8e1ed63949dfc9480f","unresolved":false,"context_lines":[{"line_number":503,"context_line":"            mock.patch.object(osinfo, \u0027OsInfoOs\u0027,"},{"line_number":504,"context_line":"                               return_value\u003dtest_osinfo_obj),"},{"line_number":505,"context_line":"            mock.patch.object(vif.designer, \u0027set_vif_guest_frontend_config\u0027,"},{"line_number":506,"context_line":"                               side_effect\u003dcheck_model_value)):"},{"line_number":507,"context_line":"            d \u003d vif.LibvirtGenericVIFDriver()"},{"line_number":508,"context_line":"            image_meta \u003d {\u0027properties\u0027: {\u0027os_name\u0027: \u0027fedora19\u0027}}"},{"line_number":509,"context_line":"            d.get_base_config(None, self.vif_bridge, image_meta,"}],"source_content_type":"text/x-python","patch_set":21,"id":"3a50d1a3_7d015a05","line":506,"in_reply_to":"9a41bdd9_0105afc0","updated":"2015-07-22 20:24:46.000000000","message":"Agree. Thanks","commit_id":"d2c658b4ac2d2b75cdf428156252feb8474c1d28"},{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"16f9c8dff719d08b3c1e0c81da79c09e4a64c872","unresolved":false,"context_lines":[{"line_number":549,"context_line":"                               return_value\u003d\u0027virtio\u0027),"},{"line_number":550,"context_line":"            mock.patch.object(osinfo.OsInfo, \u0027disk_model\u0027,"},{"line_number":551,"context_line":"                               new_callable\u003dmock.PropertyMock,"},{"line_number":552,"context_line":"                               return_value\u003d\u0027virtio\u0027)):"},{"line_number":553,"context_line":"            d \u003d vif.LibvirtGenericVIFDriver()"},{"line_number":554,"context_line":"            image_meta \u003d {\u0027properties\u0027: {\u0027os_name\u0027: \u0027fedora19\u0027}}"},{"line_number":555,"context_line":"            image_meta \u003d objects.ImageMeta.from_dict(image_meta)"}],"source_content_type":"text/x-python","patch_set":34,"id":"9a68dd71_a7252b0b","line":552,"updated":"2016-01-26 16:06:42.000000000","message":"Why exactly are we mocking osinfo? Can\u0027t we actually test against the real thing?","commit_id":"bef0004beabdff1d4a377a97ee96e4acaee18406"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"fa2211d1f344398d486373a5161f8736bf3e34ff","unresolved":false,"context_lines":[{"line_number":549,"context_line":"                               return_value\u003d\u0027virtio\u0027),"},{"line_number":550,"context_line":"            mock.patch.object(osinfo.OsInfo, \u0027disk_model\u0027,"},{"line_number":551,"context_line":"                               new_callable\u003dmock.PropertyMock,"},{"line_number":552,"context_line":"                               return_value\u003d\u0027virtio\u0027)):"},{"line_number":553,"context_line":"            d \u003d vif.LibvirtGenericVIFDriver()"},{"line_number":554,"context_line":"            image_meta \u003d {\u0027properties\u0027: {\u0027os_name\u0027: \u0027fedora19\u0027}}"},{"line_number":555,"context_line":"            image_meta \u003d objects.ImageMeta.from_dict(image_meta)"}],"source_content_type":"text/x-python","patch_set":34,"id":"7a5de9d1_7b59c7fd","line":552,"in_reply_to":"7a5de9d1_f4ebdaae","updated":"2016-01-28 21:06:14.000000000","message":"OK, I\u0027ll write a fake library for it.\nHowever, I really don\u0027t understand why do we need to do it here in a unit test, where we are testing the correctness of a vif configuration and not the libosinfo functionality..","commit_id":"bef0004beabdff1d4a377a97ee96e4acaee18406"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"d227f1285b2cf099fcd29f98492e0be62d3a208a","unresolved":false,"context_lines":[{"line_number":549,"context_line":"                               return_value\u003d\u0027virtio\u0027),"},{"line_number":550,"context_line":"            mock.patch.object(osinfo.OsInfo, \u0027disk_model\u0027,"},{"line_number":551,"context_line":"                               new_callable\u003dmock.PropertyMock,"},{"line_number":552,"context_line":"                               return_value\u003d\u0027virtio\u0027)):"},{"line_number":553,"context_line":"            d \u003d vif.LibvirtGenericVIFDriver()"},{"line_number":554,"context_line":"            image_meta \u003d {\u0027properties\u0027: {\u0027os_name\u0027: \u0027fedora19\u0027}}"},{"line_number":555,"context_line":"            image_meta \u003d objects.ImageMeta.from_dict(image_meta)"}],"source_content_type":"text/x-python","patch_set":34,"id":"9a68dd71_c5404d38","line":552,"in_reply_to":"9a68dd71_a7252b0b","updated":"2016-01-26 19:38:55.000000000","message":"I\u0027ve done that for several reasons.\n\nFirst, because I\u0027m testing the changes in VIF, to validate it\u0027s working correctly, while osinfo class itself has been already tested and has it\u0027s own tests.\n\nSecond, in order to test the whole code without the libosifo being actually installed on the test machine, I\u0027d have to create a fake library, and implement it\u0027s filters. I think that it should probably be done, but seems out of scope for this patch and more suited for a functional test.","commit_id":"bef0004beabdff1d4a377a97ee96e4acaee18406"},{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"074669c1e3016ae2d2191a798eef6a96c14d16f0","unresolved":false,"context_lines":[{"line_number":549,"context_line":"                               return_value\u003d\u0027virtio\u0027),"},{"line_number":550,"context_line":"            mock.patch.object(osinfo.OsInfo, \u0027disk_model\u0027,"},{"line_number":551,"context_line":"                               new_callable\u003dmock.PropertyMock,"},{"line_number":552,"context_line":"                               return_value\u003d\u0027virtio\u0027)):"},{"line_number":553,"context_line":"            d \u003d vif.LibvirtGenericVIFDriver()"},{"line_number":554,"context_line":"            image_meta \u003d {\u0027properties\u0027: {\u0027os_name\u0027: \u0027fedora19\u0027}}"},{"line_number":555,"context_line":"            image_meta \u003d objects.ImageMeta.from_dict(image_meta)"}],"source_content_type":"text/x-python","patch_set":34,"id":"7a5de9d1_f4ebdaae","line":552,"in_reply_to":"9a68dd71_c5404d38","updated":"2016-01-27 14:33:34.000000000","message":"I think I\u0027d feel more comfortable if there was a fixture that faked the library rather than individual mocks. mocking things outside of our code is something we typically want to avoid if possible.\n\nIt\u0027s also completely ok to just require libosinfo for this test, or skip if it\u0027s not found. It\u0027s a standard linux library, and then will run on other platforms as a skip.","commit_id":"bef0004beabdff1d4a377a97ee96e4acaee18406"},{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"b9ace86f758f784678097da135f2f68de32dcfbc","unresolved":false,"context_lines":[{"line_number":583,"context_line":"        self.flags(use_virtio_for_bridges\u003dTrue,"},{"line_number":584,"context_line":"                   virt_type\u003d\u0027kvm\u0027,"},{"line_number":585,"context_line":"                   group\u003d\u0027libvirt\u0027)"},{"line_number":586,"context_line":"        osinfo.libosinfo \u003d fakelibosinfo"},{"line_number":587,"context_line":"        d \u003d vif.LibvirtGenericVIFDriver()"},{"line_number":588,"context_line":"        image_meta \u003d {\u0027properties\u0027: {\u0027os_name\u0027: \u0027fedora22\u0027}}"},{"line_number":589,"context_line":"        image_meta \u003d objects.ImageMeta.from_dict(image_meta)"}],"source_content_type":"text/x-python","patch_set":36,"id":"5a5ae5dd_43d9abf2","line":586,"updated":"2016-02-03 21:38:46.000000000","message":"this should be a fixtures.MonkeyPatch, otherwise it\u0027s set for all other tests for all time.","commit_id":"0384c9d8f67afe0666e5c40ab3d8e796d1be6f98"}],"nova/virt/libvirt/vif.py":[{"author":{"_account_id":7055,"name":"Tapio Tallgren","username":"tapiotallgren","inactive":true},"change_message_id":"b7f05d769f5ef38c1248025136f4095f801a28ed","unresolved":false,"context_lines":[{"line_number":110,"context_line":"                                            image_meta).network_model"},{"line_number":111,"context_line":"            if vif_model is not None:"},{"line_number":112,"context_line":"                model \u003d vif_model"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"        # Else if the virt type is KVM/QEMU, use virtio according"},{"line_number":115,"context_line":"        # to the global config parameter"},{"line_number":116,"context_line":"        if (model is None and"}],"source_content_type":"text/x-python","patch_set":6,"id":"5ad0253b_02962f4c","line":113,"updated":"2015-04-29 12:58:39.000000000","message":"Should this actually be\n\nvif_model \u003d image_meta.get(\u0027properties\u0027,                                       {}).get(\u0027hw_vif_model\u0027)\nif not vif_model:\n    vif_model \u003d osinfo.get_hardware_properties(image_meta).network_model\n\nto be consistent with the comment - ie, if the vif_model is specified in the image, then it should be used.","commit_id":"4d5466e4cb8eda453dfaa8e8fb2dbe8b8eb86368"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"29970b94ad551d48e4e1e73c0f8e9f11bd364ab5","unresolved":false,"context_lines":[{"line_number":110,"context_line":"                                            image_meta).network_model"},{"line_number":111,"context_line":"            if vif_model is not None:"},{"line_number":112,"context_line":"                model \u003d vif_model"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"        # Else if the virt type is KVM/QEMU, use virtio according"},{"line_number":115,"context_line":"        # to the global config parameter"},{"line_number":116,"context_line":"        if (model is None and"}],"source_content_type":"text/x-python","patch_set":6,"id":"3acd31a7_f5f3bd08","line":113,"in_reply_to":"5ad0253b_02962f4c","updated":"2015-05-01 21:20:11.000000000","message":"Yes, thanks. I\u0027m taking care of that in the new patchset, inside the osinfo module.","commit_id":"4d5466e4cb8eda453dfaa8e8fb2dbe8b8eb86368"},{"author":{"_account_id":7055,"name":"Tapio Tallgren","username":"tapiotallgren","inactive":true},"change_message_id":"d22052638eaad3add04e3958ba6422e6b1c5e413","unresolved":false,"context_lines":[{"line_number":112,"context_line":"                raise exception.UnsupportedHardware("},{"line_number":113,"context_line":"                                            model\u003derr.kwargs.get(\u0027model\u0027),"},{"line_number":114,"context_line":"                                            virt\u003dvirt_type)"},{"line_number":115,"context_line":"            if vif_model is not None:"},{"line_number":116,"context_line":"                model \u003d vif_model"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"        # Else if the virt type is KVM/QEMU, use virtio according"}],"source_content_type":"text/x-python","patch_set":16,"id":"fa32b979_cd1df167","line":115,"updated":"2015-06-15 08:27:21.000000000","message":"model \u003d None above, so why is this test needed?","commit_id":"96ecdb7737fb05bb3b3ab608021b4473c3319ac8"},{"author":{"_account_id":7055,"name":"Tapio Tallgren","username":"tapiotallgren","inactive":true},"change_message_id":"9fa22895d193118fe9f0d3c109828f17aa0ea051","unresolved":false,"context_lines":[{"line_number":107,"context_line":"        # image then honour that model"},{"line_number":108,"context_line":"        if image_meta:"},{"line_number":109,"context_line":"            vif_model \u003d osinfo.HardwareProperties(image_meta).network_model"},{"line_number":110,"context_line":"            if vif_model is not None:"},{"line_number":111,"context_line":"                model \u003d vif_model"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"        # Else if the virt type is KVM/QEMU, use virtio according"}],"source_content_type":"text/x-python","patch_set":19,"id":"fa32b979_e76c499e","line":110,"updated":"2015-06-23 06:01:32.000000000","message":"This test is not needed: even if vif_model is None, model would get assigned None, which it is already on line 103.","commit_id":"601ffeb8fad373303ed2e712ede20eccc665f86d"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"c6a497e7f8641b8abae180b232d7b3b43e0d8671","unresolved":false,"context_lines":[{"line_number":107,"context_line":"        # image then honour that model"},{"line_number":108,"context_line":"        if image_meta:"},{"line_number":109,"context_line":"            vif_model \u003d osinfo.HardwareProperties(image_meta).network_model"},{"line_number":110,"context_line":"            if vif_model is not None:"},{"line_number":111,"context_line":"                model \u003d vif_model"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"        # Else if the virt type is KVM/QEMU, use virtio according"}],"source_content_type":"text/x-python","patch_set":19,"id":"fa32b979_a2491a33","line":110,"in_reply_to":"fa32b979_e76c499e","updated":"2015-06-24 03:58:51.000000000","message":"It\u0027s not related to the change itself, but makes sense.","commit_id":"601ffeb8fad373303ed2e712ede20eccc665f86d"}]}