)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"12e90769b01f6c0764b60d668dae19a1aded19c9","unresolved":false,"context_lines":[{"line_number":7,"context_line":"[WIP] Use annotated ddt for test_cpu_policy_constraint"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The way test_cpu_policy_constraint was set up, if it failed, it would be"},{"line_number":10,"context_line":"difficult to tell which test data item caused the failure. This patch"},{"line_number":11,"context_line":"uses ddt with annotated data items to blow this out into separate tests"},{"line_number":12,"context_line":"with an error message when error happens that make it easy to figure"},{"line_number":13,"context_line":"out which data item caused the failure."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"The error message would be like these in indicating the parameters that"},{"line_number":16,"context_line":"makes an error:"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"ff570b3c_4753a36b","line":13,"range":{"start_line":10,"start_character":59,"end_line":13,"end_character":39},"updated":"2020-05-20 10:25:13.000000000","message":"ya i think this makes sense. the test was already basically a data driven test anyway so using ddt for bettter error reporting on failure i think is a step in the right direction.","commit_id":"4f5bdf4c6a24ce41ff148cfbbd8ad9b08c619d5c"}],"nova/tests/unit/virt/test_hardware.py":[{"author":{"_account_id":30209,"name":"Huaqiang","email":"huaqiang.wang@intel.com","username":"Huaqiang.Wang"},"change_message_id":"1bc3d5d6ddcc8124d658f3fe93da98605d2b738e","unresolved":false,"context_lines":[{"line_number":854,"context_line":"        d \u003d _D(i)"},{"line_number":855,"context_line":"        setattr(d, \u0027__name__\u0027, \u0027flavor_%s_image_%s\u0027 % ("},{"line_number":856,"context_line":"            i[\u0027flavor\u0027].get(\u0027extra_specs\u0027, {}).get(\u0027hw:cpu_policy\u0027, \u0027None\u0027),"},{"line_number":857,"context_line":"            i[\u0027image\u0027][\u0027properties\u0027].get(\u0027hw_cpu_policy\u0027, \u0027None\u0027)"},{"line_number":858,"context_line":"        ))"},{"line_number":859,"context_line":"        ret.append(d)"},{"line_number":860,"context_line":"    return ddt.data(*ret)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_847ea916","line":857,"range":{"start_line":857,"start_character":0,"end_line":857,"end_character":65},"updated":"2019-11-01 15:44:59.000000000","message":"how about adding i[\u0027expect\u0027] into \u0027__name__\u0027 also? it is possible to set the same flavor and image properties but expect a different \u0027expect\u0027ing result.","commit_id":"1fd4280cc43d60cf68f27db8cc53e4c7e0f5b538"},{"author":{"_account_id":30209,"name":"Huaqiang","email":"huaqiang.wang@intel.com","username":"Huaqiang.Wang"},"change_message_id":"ae3056300112b6f47bfbc199bb22a4f3ec24cd1a","unresolved":false,"context_lines":[{"line_number":854,"context_line":"        d \u003d _D(i)"},{"line_number":855,"context_line":"        setattr(d, \u0027__name__\u0027, \u0027flavor_%s_image_%s\u0027 % ("},{"line_number":856,"context_line":"            i[\u0027flavor\u0027].get(\u0027extra_specs\u0027, {}).get(\u0027hw:cpu_policy\u0027, \u0027None\u0027),"},{"line_number":857,"context_line":"            i[\u0027image\u0027][\u0027properties\u0027].get(\u0027hw_cpu_policy\u0027, \u0027None\u0027)"},{"line_number":858,"context_line":"        ))"},{"line_number":859,"context_line":"        ret.append(d)"},{"line_number":860,"context_line":"    return ddt.data(*ret)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_df21ca9f","line":857,"range":{"start_line":857,"start_character":0,"end_line":857,"end_character":65},"in_reply_to":"3fa7e38b_847ea916","updated":"2019-11-01 15:48:16.000000000","message":"[later]: after I saw real function name this function generated, now I don\u0027t think it is necessary to add the \u0027expect\u0027 content in the function name.\nso I draw back the suggestion.","commit_id":"1fd4280cc43d60cf68f27db8cc53e4c7e0f5b538"},{"author":{"_account_id":30209,"name":"Huaqiang","email":"huaqiang.wang@intel.com","username":"Huaqiang.Wang"},"change_message_id":"1bc3d5d6ddcc8124d658f3fe93da98605d2b738e","unresolved":false,"context_lines":[{"line_number":862,"context_line":""},{"line_number":863,"context_line":"@ddt.ddt"},{"line_number":864,"context_line":"class NUMATopologyTest(test.NoDBTestCase):"},{"line_number":865,"context_line":""},{"line_number":866,"context_line":"    @annotate_cpu_policy_ddt("},{"line_number":867,"context_line":"        {"},{"line_number":868,"context_line":"            \"flavor\": objects.Flavor(extra_specs\u003d{"},{"line_number":869,"context_line":"                \"hw:cpu_policy\": \"dedicated\""}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_e413dd63","line":866,"range":{"start_line":865,"start_character":0,"end_line":866,"end_character":16},"updated":"2019-11-01 15:44:59.000000000","message":"The parameters of decorator \u0027anotate_cpu_policy_ddt\u0027 is too long. How about making these parameters into a list, and pass the list to the decorator.\n\nIllustrated with code:\n \ntestitems \u003d [\n{ \"flavor\": ....,\n  \"image\": ...,\n  \"expect\": ...},\n{\n   ...\n},\n]\n\n@anotate_cpu_policy_ddt(*testitems)\ndef test_cpu_policy_constraint(self, testitem):\n pass","commit_id":"1fd4280cc43d60cf68f27db8cc53e4c7e0f5b538"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"227952a17bdfef2e171db3926a33e6a4a7dc94d1","unresolved":false,"context_lines":[{"line_number":862,"context_line":""},{"line_number":863,"context_line":"@ddt.ddt"},{"line_number":864,"context_line":"class NUMATopologyTest(test.NoDBTestCase):"},{"line_number":865,"context_line":""},{"line_number":866,"context_line":"    @annotate_cpu_policy_ddt("},{"line_number":867,"context_line":"        {"},{"line_number":868,"context_line":"            \"flavor\": objects.Flavor(extra_specs\u003d{"},{"line_number":869,"context_line":"                \"hw:cpu_policy\": \"dedicated\""}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_1f9f62b4","line":866,"range":{"start_line":865,"start_character":0,"end_line":866,"end_character":16},"in_reply_to":"3fa7e38b_e413dd63","updated":"2019-11-01 15:53:13.000000000","message":"Sure, that would be fine, except that we would have to define the testitems either outside of the class or as a class variable. It\u0027s already a bit crappy that the decorator has to be defined outside of the class, because it\u0027s obviously tightly coupled with the test method itself.\n\nThese are reasons I\u0027m not especially confident this patch should actually be merged. I\u0027m not convinced the cure is better than the disease :)","commit_id":"1fd4280cc43d60cf68f27db8cc53e4c7e0f5b538"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"31088515958a94314f72103382446f3d75f35aa5","unresolved":false,"context_lines":[{"line_number":4405,"context_line":""},{"line_number":4406,"context_line":"@ddt.ddt"},{"line_number":4407,"context_line":"class RescuePropertyTestCase(test.NoDBTestCase):"},{"line_number":4408,"context_line":"    "},{"line_number":4409,"context_line":"    def tearDown(self):"},{"line_number":4410,"context_line":"        error_msg \u003d getattr(self, \u0027error_msg\u0027, None)"},{"line_number":4411,"context_line":"        passed \u003d getattr(self, \u0027passed\u0027, False)"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff570b3c_5516b04a","line":4408,"updated":"2020-05-20 08:39:19.000000000","message":"pep8: W293 blank line contains whitespace","commit_id":"15ae25028c4d89432135b2be132e0061e566c562"},{"author":{"_account_id":30209,"name":"Huaqiang","email":"huaqiang.wang@intel.com","username":"Huaqiang.Wang"},"change_message_id":"7406c2d4d8a2f121a5d456ac3bfd881618f15a26","unresolved":false,"context_lines":[{"line_number":4405,"context_line":""},{"line_number":4406,"context_line":"@ddt.ddt"},{"line_number":4407,"context_line":"class RescuePropertyTestCase(test.NoDBTestCase):"},{"line_number":4408,"context_line":"    "},{"line_number":4409,"context_line":"    def tearDown(self):"},{"line_number":4410,"context_line":"        error_msg \u003d getattr(self, \u0027error_msg\u0027, None)"},{"line_number":4411,"context_line":"        passed \u003d getattr(self, \u0027passed\u0027, False)"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff570b3c_35ddfcdf","line":4408,"in_reply_to":"ff570b3c_5516b04a","updated":"2020-05-20 09:20:25.000000000","message":"Done","commit_id":"15ae25028c4d89432135b2be132e0061e566c562"},{"author":{"_account_id":30209,"name":"Huaqiang","email":"huaqiang.wang@intel.com","username":"Huaqiang.Wang"},"change_message_id":"54497fdc9140187ee0afeb7d4dbd0ef98e3748c8","unresolved":false,"context_lines":[{"line_number":871,"context_line":"        if not passed and error_msg:"},{"line_number":872,"context_line":"            print(error_msg)"},{"line_number":873,"context_line":""},{"line_number":874,"context_line":"        super(NUMATopologyTest, self).tearDown()"},{"line_number":875,"context_line":""},{"line_number":876,"context_line":"    @ddt.data("},{"line_number":877,"context_line":"        {"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff570b3c_5f67d84d","line":874,"updated":"2020-05-21 05:44:07.000000000","message":"Hi Sean-k-mooney: The \u0027save_and_reraise_exception\u0027 function is really a function that I was looking for. Thanks for your advice.","commit_id":"4f5bdf4c6a24ce41ff148cfbbd8ad9b08c619d5c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"12e90769b01f6c0764b60d668dae19a1aded19c9","unresolved":false,"context_lines":[{"line_number":865,"context_line":"@ddt.ddt"},{"line_number":866,"context_line":"class NUMATopologyTest(test.NoDBTestCase):"},{"line_number":867,"context_line":""},{"line_number":868,"context_line":"    def tearDown(self):"},{"line_number":869,"context_line":"        error_msg \u003d getattr(self, \u0027error_msg\u0027, None)"},{"line_number":870,"context_line":"        passed \u003d getattr(self, \u0027passed\u0027, False)"},{"line_number":871,"context_line":"        if not passed and error_msg:"},{"line_number":872,"context_line":"            print(error_msg)"},{"line_number":873,"context_line":""},{"line_number":874,"context_line":"        super(NUMATopologyTest, self).tearDown()"},{"line_number":875,"context_line":""},{"line_number":876,"context_line":"    @ddt.data("},{"line_number":877,"context_line":"        {"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff570b3c_2aa47607","line":874,"range":{"start_line":868,"start_character":4,"end_line":874,"end_character":48},"updated":"2020-05-20 10:25:13.000000000","message":"this im less happy with why not just print\nthe error message form the error_msg_from_test_item\n\nwe can remove this tear down function and getattr and setattr usage. see my comment below.","commit_id":"4f5bdf4c6a24ce41ff148cfbbd8ad9b08c619d5c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"12e90769b01f6c0764b60d668dae19a1aded19c9","unresolved":false,"context_lines":[{"line_number":992,"context_line":"                testitem[\"flavor\"], image_meta)"},{"line_number":993,"context_line":"            self.assertIsNotNone(cpu_policy)"},{"line_number":994,"context_line":"            self.assertEqual(testitem[\"expect\"], cpu_policy)"},{"line_number":995,"context_line":"        setattr(self, \u0027passed\u0027, True)"},{"line_number":996,"context_line":""},{"line_number":997,"context_line":"    @ddt.data("},{"line_number":998,"context_line":"            {"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff570b3c_aa1a4613","line":995,"range":{"start_line":995,"start_character":6,"end_line":995,"end_character":37},"updated":"2020-05-20 10:25:13.000000000","message":"so there really is no reason to use setattr here. we we are going to have this type of non local logic in the tests just do self.passed\u003dTrue  and add passed as a paramter on the test class although i dont think you need to do that.\n\nif the test fails it will raise an exception if it passes it will return nothing so instead of the current decorator do this\n\n\ndef error_msg_from_test_item(test):\n    def inner(cls, *args, **kwargs):\n        parameters \u003d \u0027\u0027\n        if len(args):\n            parameters +\u003d \u0027, \u0027.join([str(arg) for arg in list(args)])\n        if len(kwargs):\n            parameters +\u003d \u0027, \u0027 if parameters else \u0027\u0027\n            parameters +\u003d str(kwargs)\n        error_msg \u003d (\u0027%s failed with the parameters: %s\u0027 %\n                     (test.__name__, parameters or \u0027None\u0027))\n         try: \n             test(cls, *args, **kwargs)\n         except Exception:\n             with excutils.save_and_reraise_exception():\n                 LOG.error(error_msg)\n    return inner","commit_id":"4f5bdf4c6a24ce41ff148cfbbd8ad9b08c619d5c"},{"author":{"_account_id":30209,"name":"Huaqiang","email":"huaqiang.wang@intel.com","username":"Huaqiang.Wang"},"change_message_id":"54497fdc9140187ee0afeb7d4dbd0ef98e3748c8","unresolved":false,"context_lines":[{"line_number":993,"context_line":"            self.assertIsNotNone(cpu_policy)"},{"line_number":994,"context_line":"            self.assertEqual(testitem[\"expect\"], cpu_policy)"},{"line_number":995,"context_line":"        setattr(self, \u0027passed\u0027, True)"},{"line_number":996,"context_line":""},{"line_number":997,"context_line":"    @ddt.data("},{"line_number":998,"context_line":"            {"},{"line_number":999,"context_line":"                \"flavor\": objects.Flavor(vcpus\u003d8, memory_mb\u003d2048,"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff570b3c_bf5a747f","line":996,"updated":"2020-05-21 05:44:07.000000000","message":"Hi Sean-k-mooney: Agree. No need to use the \u0027set|get attr\u0027 methods and the \u0027tearDown\u0027 method anymore once we have \u0027save_and_reraise_exception\u0027.\n\nYou code is great.","commit_id":"4f5bdf4c6a24ce41ff148cfbbd8ad9b08c619d5c"},{"author":{"_account_id":30209,"name":"Huaqiang","email":"huaqiang.wang@intel.com","username":"Huaqiang.Wang"},"change_message_id":"1692b7236bab59c9e1fab65437844161ab199c10","unresolved":false,"context_lines":[{"line_number":867,"context_line":"            test(cls, *args, **kwargs)"},{"line_number":868,"context_line":"        except Exception:"},{"line_number":869,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":870,"context_line":"                LOG.error(error_msg)"},{"line_number":871,"context_line":"    return inner"},{"line_number":872,"context_line":""},{"line_number":873,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_e7a72ca7","line":870,"updated":"2020-06-30 10:24:25.000000000","message":"Verified it works, great. Let\u0027s use the alternative.","commit_id":"2c82d7704ecb5d6434539c14fb9158f6a0790a83"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"dc5ad038d7c845446aa11821cdf2aa08588c8bf4","unresolved":false,"context_lines":[{"line_number":866,"context_line":"        try:"},{"line_number":867,"context_line":"            test(cls, *args, **kwargs)"},{"line_number":868,"context_line":"        except Exception:"},{"line_number":869,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":870,"context_line":"                LOG.error(error_msg)"},{"line_number":871,"context_line":"    return inner"},{"line_number":872,"context_line":""},{"line_number":873,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_deb11c46","line":870,"range":{"start_line":869,"start_character":0,"end_line":870,"end_character":36},"updated":"2020-06-29 16:15:26.000000000","message":"alternatively:\n\n  LOG.error(error_msg)\n  raise\n\nwhich is functionally the same as long as LOG.error doesn\u0027t raise an error (which it really shouldn\u0027t)","commit_id":"2c82d7704ecb5d6434539c14fb9158f6a0790a83"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8499106089bb2e992f9a8ba2284c1e01dfb90282","unresolved":false,"context_lines":[{"line_number":867,"context_line":"            test(cls, *args, **kwargs)"},{"line_number":868,"context_line":"        except Exception:"},{"line_number":869,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":870,"context_line":"                LOG.error(error_msg)"},{"line_number":871,"context_line":"    return inner"},{"line_number":872,"context_line":""},{"line_number":873,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_ff721049","line":870,"in_reply_to":"bf51134e_e7a72ca7","updated":"2020-06-30 13:01:04.000000000","message":"on python2 i think that had a diffrent behavior but i guess its fine.\n\n\"with excutils.save_and_reraise_exception():\"\n\nwas the prefered way to do it since it preserved the traceback for the original site where the excpetion was raised.\n\ni think just raise on python3 will also do the same","commit_id":"2c82d7704ecb5d6434539c14fb9158f6a0790a83"},{"author":{"_account_id":30209,"name":"Huaqiang","email":"huaqiang.wang@intel.com","username":"Huaqiang.Wang"},"change_message_id":"1692b7236bab59c9e1fab65437844161ab199c10","unresolved":false,"context_lines":[{"line_number":868,"context_line":"        except Exception:"},{"line_number":869,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":870,"context_line":"                LOG.error(error_msg)"},{"line_number":871,"context_line":"    return inner"},{"line_number":872,"context_line":""},{"line_number":873,"context_line":""},{"line_number":874,"context_line":"@ddt.ddt"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_47b6d8de","line":871,"updated":"2020-06-30 10:24:25.000000000","message":"We can do what the \u0027annotated\u0027 example done for each test item, but we have to assign assign a name for each test item and make sure the name is unique, right? It is not hard to set a name for each test item since the arguments of each test item are different. But is it troublesome for a developer to go through the name list and guarantee that? And I feel the test item name would be very long since the test case name is already long enough.\n\nIf you think my considerations are trival and does not matter in developing, I can do as you suggested.","commit_id":"2c82d7704ecb5d6434539c14fb9158f6a0790a83"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"dc5ad038d7c845446aa11821cdf2aa08588c8bf4","unresolved":false,"context_lines":[{"line_number":868,"context_line":"        except Exception:"},{"line_number":869,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":870,"context_line":"                LOG.error(error_msg)"},{"line_number":871,"context_line":"    return inner"},{"line_number":872,"context_line":""},{"line_number":873,"context_line":""},{"line_number":874,"context_line":"@ddt.ddt"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_be96e84a","line":871,"updated":"2020-06-29 16:15:26.000000000","message":"While this is nice, we _could_ do better. Check out the \u0027annotate\u0027 function in the example here [1]. We could use this to store the test name as a string argument, e.g.:\n\n  {\n      \u0027name\u0027: \u0027test_cpu_policy_constraint_identical_values\u0027,\n      \u0027flavor\u0027: {\"hw:cpu_policy\": \"dedicated\"},\n      \u0027image\u0027: {\"hw_cpu_policy\": \"dedicated\"},\n      \u0027expect\u0027: fields.CPUAllocationPolicy.DEDICATED,\n  },\n\n[1] https://ddt.readthedocs.io/en/latest/example.html#example-usage","commit_id":"2c82d7704ecb5d6434539c14fb9158f6a0790a83"},{"author":{"_account_id":30209,"name":"Huaqiang","email":"huaqiang.wang@intel.com","username":"Huaqiang.Wang"},"change_message_id":"7a5b50bac73299e279678f60f0c7c362cd7e6914","unresolved":false,"context_lines":[{"line_number":868,"context_line":"        except Exception:"},{"line_number":869,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":870,"context_line":"                LOG.error(error_msg)"},{"line_number":871,"context_line":"    return inner"},{"line_number":872,"context_line":""},{"line_number":873,"context_line":""},{"line_number":874,"context_line":"@ddt.ddt"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_722598ab","line":871,"in_reply_to":"bf51134e_47b6d8de","updated":"2020-06-30 11:03:38.000000000","message":"[Later] There is another reason for dumping the parameters of test item, when the exception raised, user can examine which parameter is the possible cause. If we add a \u0027name\u0027 for it, user needs to first check the test item name and then go to the corresponding parameters. Showing the parameters directly when error happens could somehow save time.","commit_id":"2c82d7704ecb5d6434539c14fb9158f6a0790a83"},{"author":{"_account_id":30209,"name":"Huaqiang","email":"huaqiang.wang@intel.com","username":"Huaqiang.Wang"},"change_message_id":"228390208631e280504944ab0c7e7924d7cfb902","unresolved":false,"context_lines":[{"line_number":868,"context_line":"        except Exception:"},{"line_number":869,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":870,"context_line":"                LOG.error(error_msg)"},{"line_number":871,"context_line":"    return inner"},{"line_number":872,"context_line":""},{"line_number":873,"context_line":""},{"line_number":874,"context_line":"@ddt.ddt"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_7961f111","line":871,"in_reply_to":"bf51134e_5f605c36","updated":"2020-07-01 07:24:55.000000000","message":"\u003e it is better then the current failure\n \u003e \n \u003e nova.tests.unit.virt.test_hardware.NUMATopologyTest.test_cpu_policy_constraint_02\n \u003e if its between that or the current solution i guess that is the\n \u003e best way to go\nEven with the function \n name \u0027nova.tests.unit.virt.test_hardware.NUMATopologyTest.test_cpu_policy_constraint_02\u0027 and the parameters that caused the error could speed up the debugging process through fast locating.\n\nA proper function name is a plus for this. Creating a name by combining parameters is not good idea when the parameter number is big or with a complex object parameter.\n\nI\u0027d like to have a function that could be used for any @ddt.data decorated function and help for locating the errors.\n\nThe current way of listing out the parameters that caused the error seems to be an easy solution for this purpose.","commit_id":"2c82d7704ecb5d6434539c14fb9158f6a0790a83"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a972f96da61dd805fe86d22629db483fbf3e6b78","unresolved":false,"context_lines":[{"line_number":868,"context_line":"        except Exception:"},{"line_number":869,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":870,"context_line":"                LOG.error(error_msg)"},{"line_number":871,"context_line":"    return inner"},{"line_number":872,"context_line":""},{"line_number":873,"context_line":""},{"line_number":874,"context_line":"@ddt.ddt"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_bfc6d848","line":871,"in_reply_to":"bf51134e_722598ab","updated":"2020-06-30 12:57:04.000000000","message":"Hmm, fair point. Well, how about we do something like this:\n\n    @ddt.data(\n        (\"dedicated\", \"dedicated\", fields.CPUAllocationPolicy.DEDICATED),\n        (\"dedicated\", \"shared\", fields.CPUAllocationPolicy.DEDICATED),\n        (\"dedicated\", None, fields.CPUAllocationPolicy.DEDICATED),\n        (\"shared\", \"dedicated\", exception.ImageCPUPinningForbidden),\n        (\"shared\", \"shared\", fields.CPUAllocationPolicy.SHARED),\n        (\"shared\", None, fields.CPUAllocationPolicy.SHARED),\n        (None, \"dedicated\", fields.CPUAllocationPolicy.DEDICATED),\n        (None, \"shared\", fields.CPUAllocationPolicy.SHARED),\n        (None, None, None),\n        (\"invalid\", None, exception.InvalidCPUAllocationPolicy),\n    )\n\n\nThen your test names will look like this:\n\n    test_cpu_policy_constraint_01___dedicated____dedicated____dedicated__\n    test_cpu_policy_constraint_02___dedicated____shared____dedicated__\n    test_cpu_policy_constraint_03___dedicated___None___dedicated__\n    test_cpu_policy_constraint_04\n    test_cpu_policy_constraint_05___shared____shared____shared__\n    test_cpu_policy_constraint_06___shared___None___shared__\n    test_cpu_policy_constraint_07__None___dedicated____dedicated__\n    test_cpu_policy_constraint_08__None___shared____shared__\n    test_cpu_policy_constraint_09__None__None__None_\n    test_cpu_policy_constraint_10\n\nIt\u0027s not perfect, but it\u0027s pretty good and pretty readable, IMO","commit_id":"2c82d7704ecb5d6434539c14fb9158f6a0790a83"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8499106089bb2e992f9a8ba2284c1e01dfb90282","unresolved":false,"context_lines":[{"line_number":868,"context_line":"        except Exception:"},{"line_number":869,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":870,"context_line":"                LOG.error(error_msg)"},{"line_number":871,"context_line":"    return inner"},{"line_number":872,"context_line":""},{"line_number":873,"context_line":""},{"line_number":874,"context_line":"@ddt.ddt"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_7f38c03c","line":871,"in_reply_to":"bf51134e_722598ab","updated":"2020-06-30 13:01:04.000000000","message":"it can be hard to figure out from the parmaters in some cases which test filed.\n\nin this case sicne the onley parmater that is varying is ths image metadata it proably not too hard but named test cases should not be too hard in this case the names are pretty clear\n\nflavor_dedicated_image_dedicated\nflavor_dedicated_image_shared\nflavor_dedicated_image_unset\n\nthat said it smore or less repoducing the parmaters so im not sure its actully an imporvement.\nit will depend on the way it is printed when it fails.\n\ncan you provide an example.","commit_id":"2c82d7704ecb5d6434539c14fb9158f6a0790a83"},{"author":{"_account_id":30209,"name":"Huaqiang","email":"huaqiang.wang@intel.com","username":"Huaqiang.Wang"},"change_message_id":"228390208631e280504944ab0c7e7924d7cfb902","unresolved":false,"context_lines":[{"line_number":868,"context_line":"        except Exception:"},{"line_number":869,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":870,"context_line":"                LOG.error(error_msg)"},{"line_number":871,"context_line":"    return inner"},{"line_number":872,"context_line":""},{"line_number":873,"context_line":""},{"line_number":874,"context_line":"@ddt.ddt"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_799f3165","line":871,"in_reply_to":"bf51134e_7f38c03c","updated":"2020-07-01 07:24:55.000000000","message":"\u003e it can be hard to figure out from the parmaters in some cases which\n \u003e test filed.\n \u003e \n \u003e in this case sicne the onley parmater that is varying is ths image\n \u003e metadata it proably not too hard but named test cases should not be\n \u003e too hard in this case the names are pretty clear\n \u003e \n \u003e flavor_dedicated_image_dedicated\n \u003e flavor_dedicated_image_shared\n \u003e flavor_dedicated_image_unset\n \u003e \n \u003e that said it smore or less repoducing the parmaters so im not sure\n \u003e its actully an imporvement.\n\nThese are better due to the label involved. \n\nBut look at the \u0027test_topology_constraints\u0027 test case, it seems it is impossible to create a readable, short function name through combing the parameters.","commit_id":"2c82d7704ecb5d6434539c14fb9158f6a0790a83"},{"author":{"_account_id":30209,"name":"Huaqiang","email":"huaqiang.wang@intel.com","username":"Huaqiang.Wang"},"change_message_id":"228390208631e280504944ab0c7e7924d7cfb902","unresolved":false,"context_lines":[{"line_number":868,"context_line":"        except Exception:"},{"line_number":869,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":870,"context_line":"                LOG.error(error_msg)"},{"line_number":871,"context_line":"    return inner"},{"line_number":872,"context_line":""},{"line_number":873,"context_line":""},{"line_number":874,"context_line":"@ddt.ddt"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_e40d5836","line":871,"in_reply_to":"bf51134e_bfc6d848","updated":"2020-07-01 07:24:55.000000000","message":"\u003e test_cpu_policy_constraint_09__None__None__None_\n \u003e test_cpu_policy_constraint_10\n \u003e \n \u003e It\u0027s not perfect, but it\u0027s pretty good and pretty readable, IMO\n\nFor this test case, the \"test_cpu_policy_constraint\", I am ok to let \u0027ddt.data\u0027 modify the test case name for each sub-test-case or we technically assign a function name for the sub-test-case do what it has been done in ddt\u0027s official example \u0027annotated\u0027.\n\nBut, look at another ddt function \u0027test_topology_constraints\u0027, it is not a good idea to \"technically\" create the function name for it, the reason is, in this case, the Flavor and ImageMeta objects are changed a lot, we cannot accommodate these parameters in test case function name, it will be very long, and not readable.\n\nWe are trying to relieve the suffering of locating which sub-test-item caused the error in developing when the error happens in any ddt function. The current code works and is simple, can be used for any ddt.data decorated function.","commit_id":"2c82d7704ecb5d6434539c14fb9158f6a0790a83"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6bdd0438078cc848ba01e0ff7eb895b236c2a51c","unresolved":false,"context_lines":[{"line_number":868,"context_line":"        except Exception:"},{"line_number":869,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":870,"context_line":"                LOG.error(error_msg)"},{"line_number":871,"context_line":"    return inner"},{"line_number":872,"context_line":""},{"line_number":873,"context_line":""},{"line_number":874,"context_line":"@ddt.ddt"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_ff1cd0c2","line":871,"in_reply_to":"bf51134e_bfc6d848","updated":"2020-06-30 13:03:05.000000000","message":"i actully dnot think htat is readable\nwithout lables for the parmater its really annoying\n\nyou can figure it out by looking that the function signiture but that is irriating","commit_id":"2c82d7704ecb5d6434539c14fb9158f6a0790a83"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"9fa125a7a13b983e5d1976ffe7bbac0149b268d8","unresolved":false,"context_lines":[{"line_number":868,"context_line":"        except Exception:"},{"line_number":869,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":870,"context_line":"                LOG.error(error_msg)"},{"line_number":871,"context_line":"    return inner"},{"line_number":872,"context_line":""},{"line_number":873,"context_line":""},{"line_number":874,"context_line":"@ddt.ddt"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_5f605c36","line":871,"in_reply_to":"bf51134e_ff1cd0c2","updated":"2020-06-30 13:04:28.000000000","message":"it is better then the current failure\n\nnova.tests.unit.virt.test_hardware.NUMATopologyTest.test_cpu_policy_constraint_02 if its between that or the current solution i guess that is the best way to go","commit_id":"2c82d7704ecb5d6434539c14fb9158f6a0790a83"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"dc5ad038d7c845446aa11821cdf2aa08588c8bf4","unresolved":false,"context_lines":[{"line_number":876,"context_line":""},{"line_number":877,"context_line":"    @ddt.data("},{"line_number":878,"context_line":"        {"},{"line_number":879,"context_line":"            \"flavor\": objects.Flavor(extra_specs\u003d{"},{"line_number":880,"context_line":"                \"hw:cpu_policy\": \"dedicated\""},{"line_number":881,"context_line":"            }),"},{"line_number":882,"context_line":"            \"image\": {"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_de975c44","line":879,"range":{"start_line":879,"start_character":22,"end_line":879,"end_character":49},"updated":"2020-06-29 16:15:26.000000000","message":"While we\u0027re here, this is identical in all tests. How about we drop it here and do it once in the test?","commit_id":"2c82d7704ecb5d6434539c14fb9158f6a0790a83"},{"author":{"_account_id":30209,"name":"Huaqiang","email":"huaqiang.wang@intel.com","username":"Huaqiang.Wang"},"change_message_id":"1692b7236bab59c9e1fab65437844161ab199c10","unresolved":false,"context_lines":[{"line_number":876,"context_line":""},{"line_number":877,"context_line":"    @ddt.data("},{"line_number":878,"context_line":"        {"},{"line_number":879,"context_line":"            \"flavor\": objects.Flavor(extra_specs\u003d{"},{"line_number":880,"context_line":"                \"hw:cpu_policy\": \"dedicated\""},{"line_number":881,"context_line":"            }),"},{"line_number":882,"context_line":"            \"image\": {"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_27bbe4c4","line":879,"updated":"2020-06-30 10:24:25.000000000","message":"Yes, we can.","commit_id":"2c82d7704ecb5d6434539c14fb9158f6a0790a83"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8499106089bb2e992f9a8ba2284c1e01dfb90282","unresolved":false,"context_lines":[{"line_number":876,"context_line":""},{"line_number":877,"context_line":"    @ddt.data("},{"line_number":878,"context_line":"        {"},{"line_number":879,"context_line":"            \"flavor\": objects.Flavor(extra_specs\u003d{"},{"line_number":880,"context_line":"                \"hw:cpu_policy\": \"dedicated\""},{"line_number":881,"context_line":"            }),"},{"line_number":882,"context_line":"            \"image\": {"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_5fb69cde","line":879,"in_reply_to":"bf51134e_27bbe4c4","updated":"2020-06-30 13:01:04.000000000","message":"+1","commit_id":"2c82d7704ecb5d6434539c14fb9158f6a0790a83"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"dc5ad038d7c845446aa11821cdf2aa08588c8bf4","unresolved":false,"context_lines":[{"line_number":975,"context_line":"            },"},{"line_number":976,"context_line":"            \"expect\": exception.InvalidCPUAllocationPolicy"},{"line_number":977,"context_line":"        },"},{"line_number":978,"context_line":"    )"},{"line_number":979,"context_line":"    @dump_parameters_in_exception"},{"line_number":980,"context_line":"    def test_cpu_policy_constraint(self, testitem):"},{"line_number":981,"context_line":"        image_meta \u003d objects.ImageMeta.from_dict(testitem[\"image\"])"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_7e829001","line":978,"updated":"2020-06-29 16:15:26.000000000","message":"Could you add ddt.unpack and replace \u0027testitem\u0027 below with separate named arguments? e.g.\n\n  def test_cpu_policy_constraint(self, flavor, image, expect):","commit_id":"2c82d7704ecb5d6434539c14fb9158f6a0790a83"},{"author":{"_account_id":30209,"name":"Huaqiang","email":"huaqiang.wang@intel.com","username":"Huaqiang.Wang"},"change_message_id":"1692b7236bab59c9e1fab65437844161ab199c10","unresolved":false,"context_lines":[{"line_number":975,"context_line":"            },"},{"line_number":976,"context_line":"            \"expect\": exception.InvalidCPUAllocationPolicy"},{"line_number":977,"context_line":"        },"},{"line_number":978,"context_line":"    )"},{"line_number":979,"context_line":"    @dump_parameters_in_exception"},{"line_number":980,"context_line":"    def test_cpu_policy_constraint(self, testitem):"},{"line_number":981,"context_line":"        image_meta \u003d objects.ImageMeta.from_dict(testitem[\"image\"])"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_87b0f0df","line":978,"updated":"2020-06-30 10:24:25.000000000","message":"Yes, I can use ddt.unpack and remove \u0027testitem\u0027, we have an example here L#4408.\n\n\u003e \u0027with separate named arguments?\u0027\nI\u0027d like to have your further comment for my considertion that I have in #L871.","commit_id":"2c82d7704ecb5d6434539c14fb9158f6a0790a83"}]}
