)]}'
{"aodh/tests/api/v2/test_alarm_scenarios.py":[{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"9eed61e118c9ba13c788c3678c6cd0eb3503aea7","unresolved":false,"context_lines":[{"line_number":2583,"context_line":"    def setUp(self):"},{"line_number":2584,"context_line":"        super(TestAlarmsRuleCombination, self).setUp()"},{"line_number":2585,"context_line":"        for alarm in ["},{"line_number":2586,"context_line":"            models.Alarm(name\u003d\u0027name1\u0027,"},{"line_number":2587,"context_line":"                         type\u003d\u0027threshold\u0027,"},{"line_number":2588,"context_line":"                         enabled\u003dTrue,"},{"line_number":2589,"context_line":"                         alarm_id\u003d\u0027a\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_e57a9a23","line":2586,"updated":"2015-07-23 07:50:11.000000000","message":"This block seems to be a duplication of the setUp in TestAlarms. Could you please extract this to reduce duplication?","commit_id":"627682e082584ed8a806ab309717fe4ae01be48f"},{"author":{"_account_id":1894,"name":"Ryota MIBU","email":"r-mibu@cq.jp.nec.com","username":"r-mibu"},"change_message_id":"d86c92ccd970295b9e84477ec5387f2cec31f895","unresolved":false,"context_lines":[{"line_number":2583,"context_line":"    def setUp(self):"},{"line_number":2584,"context_line":"        super(TestAlarmsRuleCombination, self).setUp()"},{"line_number":2585,"context_line":"        for alarm in ["},{"line_number":2586,"context_line":"            models.Alarm(name\u003d\u0027name1\u0027,"},{"line_number":2587,"context_line":"                         type\u003d\u0027threshold\u0027,"},{"line_number":2588,"context_line":"                         enabled\u003dTrue,"},{"line_number":2589,"context_line":"                         alarm_id\u003d\u0027a\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_9c697a28","line":2586,"in_reply_to":"3a50d1a3_33d0c5c8","updated":"2015-07-24 01:12:21.000000000","message":"How can we rely on the data in TestAlarms? By creating new function to register alarms, then call this in setUp() of TestAlarms and TestAlarmsRuleCombination?\n\nThe main purpose of this patch is to separate test cases in order to reduce complexity while writing test codes, otherwise we\u0027ll hit merge conflicts even changes in different feature like different rule evaluator. If we have shared function, this cannot be achieved.\n\nThe data will not be generic any more. Those default alarms should be cleaned.\n\nYes, thanks you for the support. There are several tests written in different policies.","commit_id":"627682e082584ed8a806ab309717fe4ae01be48f"},{"author":{"_account_id":6537,"name":"gordon chung","email":"gord@live.ca","username":"chungg"},"change_message_id":"1aa21ad6418e56451456be1adb969e0fb2df6c81","unresolved":false,"context_lines":[{"line_number":2583,"context_line":"    def setUp(self):"},{"line_number":2584,"context_line":"        super(TestAlarmsRuleCombination, self).setUp()"},{"line_number":2585,"context_line":"        for alarm in ["},{"line_number":2586,"context_line":"            models.Alarm(name\u003d\u0027name1\u0027,"},{"line_number":2587,"context_line":"                         type\u003d\u0027threshold\u0027,"},{"line_number":2588,"context_line":"                         enabled\u003dTrue,"},{"line_number":2589,"context_line":"                         alarm_id\u003d\u0027a\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_b0cb99a3","line":2586,"in_reply_to":"3a50d1a3_78b756d8","updated":"2015-07-28 14:19:55.000000000","message":"@Ryota, were the tests using the data from TestsAlarms.setUp() before? if so, maybe we should just reuse it for now to minimise change.","commit_id":"627682e082584ed8a806ab309717fe4ae01be48f"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"bebe3e828727eb1da34087b3d82b0f13801d1a38","unresolved":false,"context_lines":[{"line_number":2583,"context_line":"    def setUp(self):"},{"line_number":2584,"context_line":"        super(TestAlarmsRuleCombination, self).setUp()"},{"line_number":2585,"context_line":"        for alarm in ["},{"line_number":2586,"context_line":"            models.Alarm(name\u003d\u0027name1\u0027,"},{"line_number":2587,"context_line":"                         type\u003d\u0027threshold\u0027,"},{"line_number":2588,"context_line":"                         enabled\u003dTrue,"},{"line_number":2589,"context_line":"                         alarm_id\u003d\u0027a\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_33d0c5c8","line":2586,"in_reply_to":"3a50d1a3_90c3d237","updated":"2015-07-23 09:21:28.000000000","message":"I still wonder whether we could rely on the data in TestAlarms and then add new items here, when you have that patch you mentioned. What do you think?\n\nI support the helper function, also this data is quite generic and only a few fields are modified. As for direct DB access I wouldn\u0027t use it or at least not in functional tests, but we can discuss that part in that subsequent patch.","commit_id":"627682e082584ed8a806ab309717fe4ae01be48f"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"98c15c226b99dab30aaa271224c345d89b2d4fe9","unresolved":false,"context_lines":[{"line_number":2583,"context_line":"    def setUp(self):"},{"line_number":2584,"context_line":"        super(TestAlarmsRuleCombination, self).setUp()"},{"line_number":2585,"context_line":"        for alarm in ["},{"line_number":2586,"context_line":"            models.Alarm(name\u003d\u0027name1\u0027,"},{"line_number":2587,"context_line":"                         type\u003d\u0027threshold\u0027,"},{"line_number":2588,"context_line":"                         enabled\u003dTrue,"},{"line_number":2589,"context_line":"                         alarm_id\u003d\u0027a\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_caed86b8","line":2586,"in_reply_to":"3a50d1a3_9c697a28","updated":"2015-07-27 17:03:28.000000000","message":"Yeap, extracting the creation of the test data is one option to do this.\n\nMerge conflicts do not happen that often around this code base as far as I can remember, although if there is a reasonable separation of test data that still can be useful, I agree.\n\nI just meant that not all the fields are changed in these alarms, so we are defining these fields redundantly, so it can be considered how to avoid this if there is any good way.","commit_id":"627682e082584ed8a806ab309717fe4ae01be48f"},{"author":{"_account_id":1894,"name":"Ryota MIBU","email":"r-mibu@cq.jp.nec.com","username":"r-mibu"},"change_message_id":"36026dda935695a57bb1b15417dafb1a0131af64","unresolved":false,"context_lines":[{"line_number":2583,"context_line":"    def setUp(self):"},{"line_number":2584,"context_line":"        super(TestAlarmsRuleCombination, self).setUp()"},{"line_number":2585,"context_line":"        for alarm in ["},{"line_number":2586,"context_line":"            models.Alarm(name\u003d\u0027name1\u0027,"},{"line_number":2587,"context_line":"                         type\u003d\u0027threshold\u0027,"},{"line_number":2588,"context_line":"                         enabled\u003dTrue,"},{"line_number":2589,"context_line":"                         alarm_id\u003d\u0027a\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a4dcd0f_816a7e19","line":2586,"in_reply_to":"3a50d1a3_b0cb99a3","updated":"2015-08-03 14:15:42.000000000","message":"Sorry for my late response...\n\nOK, then I will work on https://review.openstack.org/#/c/204370/ first. That reduce number of initial data created in TestsAlarms.setUp() and make those setUp() the same by which we can share the function.","commit_id":"627682e082584ed8a806ab309717fe4ae01be48f"},{"author":{"_account_id":1894,"name":"Ryota MIBU","email":"r-mibu@cq.jp.nec.com","username":"r-mibu"},"change_message_id":"6c08bfe2f7cfadf6524b548e370d87d41f2a699c","unresolved":false,"context_lines":[{"line_number":2583,"context_line":"    def setUp(self):"},{"line_number":2584,"context_line":"        super(TestAlarmsRuleCombination, self).setUp()"},{"line_number":2585,"context_line":"        for alarm in ["},{"line_number":2586,"context_line":"            models.Alarm(name\u003d\u0027name1\u0027,"},{"line_number":2587,"context_line":"                         type\u003d\u0027threshold\u0027,"},{"line_number":2588,"context_line":"                         enabled\u003dTrue,"},{"line_number":2589,"context_line":"                         alarm_id\u003d\u0027a\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_78b756d8","line":2586,"in_reply_to":"3a50d1a3_caed86b8","updated":"2015-07-28 02:28:30.000000000","message":"aodh is young  and not stable so far, I saw merge conflicts...\n\nThe setUp() in this class is partially the same in TestAlarms.setUp() . I cannot see a benefit to have a shared function for further refactoring.\n\nWe can remove this setUp() eventually. I think we should move those default alarm creation from setUp() to each test (def test_*), since all tests in this file should be API scenario tests. After we done those change, developer only have to take care each test (def test_*) instead of set of tests (class *).","commit_id":"627682e082584ed8a806ab309717fe4ae01be48f"},{"author":{"_account_id":1894,"name":"Ryota MIBU","email":"r-mibu@cq.jp.nec.com","username":"r-mibu"},"change_message_id":"bacaf02852acd78ce9d2103c0fd36ea713f5523f","unresolved":false,"context_lines":[{"line_number":2583,"context_line":"    def setUp(self):"},{"line_number":2584,"context_line":"        super(TestAlarmsRuleCombination, self).setUp()"},{"line_number":2585,"context_line":"        for alarm in ["},{"line_number":2586,"context_line":"            models.Alarm(name\u003d\u0027name1\u0027,"},{"line_number":2587,"context_line":"                         type\u003d\u0027threshold\u0027,"},{"line_number":2588,"context_line":"                         enabled\u003dTrue,"},{"line_number":2589,"context_line":"                         alarm_id\u003d\u0027a\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_90c3d237","line":2586,"in_reply_to":"3a50d1a3_e57a9a23","updated":"2015-07-23 08:12:03.000000000","message":"I understand your concern, but I\u0027d still like to have unshared default alarms only for this test case.\nI have a plan to rewrite those default alarm entries in another change which focus on improvement of this test case.\n\nI also have another plan to prepare helper function for creating alarm via direct DB access which will reduce codes.","commit_id":"627682e082584ed8a806ab309717fe4ae01be48f"}]}