)]}'
{"tempest/scenario/test_aggregates_basic_ops.py":[{"author":{"_account_id":5689,"name":"Masayuki Igawa","email":"masayuki@igawa.io","username":"igawa"},"change_message_id":"1c2a83eaf8ad069f0196ba5190f5165fccf99a61","unresolved":false,"context_lines":[{"line_number":67,"context_line":"                hosts_in_zone.extend(agg[\u0027hosts\u0027])"},{"line_number":68,"context_line":"        hosts \u003d [v for v in hosts_available if v not in hosts_in_zone]"},{"line_number":69,"context_line":"        if not hosts:"},{"line_number":70,"context_line":"            raise self.skipException(\"All hosts are already in other \""},{"line_number":71,"context_line":"                                     \"availability zones, so can\u0027t add \""},{"line_number":72,"context_line":"                                     \"host to aggregate. \\nAggregates list: \""},{"line_number":73,"context_line":"                                     \"%s\" % aggregates)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_91564906","line":70,"updated":"2019-07-30 05:51:28.000000000","message":"I was debating here should fail or skip. In our gate CI, I think the test should be failed because it shouldn\u0027t be happened and we likely miss the skipped tests. On the other hand, in the downstream test CI cases, I\u0027m not sure. But I guess this should be passed in most case. I think skip condition should depend on configs or settings not on runtime states basically.\n\nIs this just over-thinking?","commit_id":"ea21684ff0352d1eba5054fdb562e2ec5e6123ea"},{"author":{"_account_id":17887,"name":"Doug Schveninger","email":"ds6901@att.com","username":"Doug.Schveninger"},"change_message_id":"abcb49833eb8af4114d150ddc0536c4cee5779d2","unresolved":false,"context_lines":[{"line_number":67,"context_line":"                hosts_in_zone.extend(agg[\u0027hosts\u0027])"},{"line_number":68,"context_line":"        hosts \u003d [v for v in hosts_available if v not in hosts_in_zone]"},{"line_number":69,"context_line":"        if not hosts:"},{"line_number":70,"context_line":"            raise self.skipException(\"All hosts are already in other \""},{"line_number":71,"context_line":"                                     \"availability zones, so can\u0027t add \""},{"line_number":72,"context_line":"                                     \"host to aggregate. \\nAggregates list: \""},{"line_number":73,"context_line":"                                     \"%s\" % aggregates)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_856a7c17","line":70,"in_reply_to":"7faddb67_5f7e9719","updated":"2019-07-30 14:15:43.000000000","message":"Rick and I talked. \nI also agree if we need to skip items we need to support more of a conf direction so it support both upstream and downstream CI.\n\nWith that said I do not see enough value to this scenario since the api test_aggregates [0] does test the API for aggregates and aggregates metadata.\n\nWith that said I would suggest that we blacklist this test in our downstream CI and abandon this commit if all agree.\n \n[0] https://github.com/openstack/tempest/blob/master/tempest/api/compute/admin/test_aggregates.py","commit_id":"ea21684ff0352d1eba5054fdb562e2ec5e6123ea"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"d65a9434793fc2c383a2c5c04e4a204d638154e2","unresolved":false,"context_lines":[{"line_number":67,"context_line":"                hosts_in_zone.extend(agg[\u0027hosts\u0027])"},{"line_number":68,"context_line":"        hosts \u003d [v for v in hosts_available if v not in hosts_in_zone]"},{"line_number":69,"context_line":"        if not hosts:"},{"line_number":70,"context_line":"            raise self.skipException(\"All hosts are already in other \""},{"line_number":71,"context_line":"                                     \"availability zones, so can\u0027t add \""},{"line_number":72,"context_line":"                                     \"host to aggregate. \\nAggregates list: \""},{"line_number":73,"context_line":"                                     \"%s\" % aggregates)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_eea6fa0f","line":70,"in_reply_to":"7faddb67_6a9bf616","updated":"2019-08-26 17:44:13.000000000","message":"I think skipping is the correct behavior because this is an environmental prerequisite issue that isn\u0027t satisfied. The test itself shouldn\u0027t fail because it can\u0027t find a suitable host for testing. That doesn\u0027t mean that the test in and of itself failed, only that it cannot pass under the given environmental circumstances.\n\nIf we do want it to fail, it would require throwing a specific error as to why it failed: namely, that no available host could be found for the test to run. But again, I think that this should be a skip, not a fail.","commit_id":"ea21684ff0352d1eba5054fdb562e2ec5e6123ea"},{"author":{"_account_id":17896,"name":"Rick Bartra","email":"rickbartra@microsoft.com","username":"rb560u"},"change_message_id":"ee8cb6401e66b877da531153b2536794c9aaed49","unresolved":false,"context_lines":[{"line_number":67,"context_line":"                hosts_in_zone.extend(agg[\u0027hosts\u0027])"},{"line_number":68,"context_line":"        hosts \u003d [v for v in hosts_available if v not in hosts_in_zone]"},{"line_number":69,"context_line":"        if not hosts:"},{"line_number":70,"context_line":"            raise self.skipException(\"All hosts are already in other \""},{"line_number":71,"context_line":"                                     \"availability zones, so can\u0027t add \""},{"line_number":72,"context_line":"                                     \"host to aggregate. \\nAggregates list: \""},{"line_number":73,"context_line":"                                     \"%s\" % aggregates)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_9653916b","line":70,"in_reply_to":"7faddb67_856a7c17","updated":"2019-07-30 19:21:59.000000000","message":"Here is the example I followed: https://github.com/openstack/tempest/blob/master/tempest/api/compute/admin/test_aggregates.py#L220 ... I found this when searching for bug reports for the same situation we were coming across. As seen in that example, the skip is based on runtime state. I agree with Doug about the limited value this scenario test provides. I just wanted to point out that there is another example of an aggregate test skipping based on runtime state.","commit_id":"ea21684ff0352d1eba5054fdb562e2ec5e6123ea"},{"author":{"_account_id":17887,"name":"Doug Schveninger","email":"ds6901@att.com","username":"Doug.Schveninger"},"change_message_id":"b0c9083593db3bca0899569b729d4ca82f720348","unresolved":false,"context_lines":[{"line_number":67,"context_line":"                hosts_in_zone.extend(agg[\u0027hosts\u0027])"},{"line_number":68,"context_line":"        hosts \u003d [v for v in hosts_available if v not in hosts_in_zone]"},{"line_number":69,"context_line":"        if not hosts:"},{"line_number":70,"context_line":"            raise self.skipException(\"All hosts are already in other \""},{"line_number":71,"context_line":"                                     \"availability zones, so can\u0027t add \""},{"line_number":72,"context_line":"                                     \"host to aggregate. \\nAggregates list: \""},{"line_number":73,"context_line":"                                     \"%s\" % aggregates)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_5f7e9719","line":70,"in_reply_to":"7faddb67_91564906","updated":"2019-07-30 12:26:28.000000000","message":"Good Question.  Let me talk to Rick on this one some more","commit_id":"ea21684ff0352d1eba5054fdb562e2ec5e6123ea"},{"author":{"_account_id":20190,"name":"zhufl","email":"zhu.fanglei@zte.com.cn","username":"zhufl"},"change_message_id":"c2b27a4d47b5945355c0645a248a8e8a4b9b34c0","unresolved":false,"context_lines":[{"line_number":67,"context_line":"                hosts_in_zone.extend(agg[\u0027hosts\u0027])"},{"line_number":68,"context_line":"        hosts \u003d [v for v in hosts_available if v not in hosts_in_zone]"},{"line_number":69,"context_line":"        if not hosts:"},{"line_number":70,"context_line":"            raise self.skipException(\"All hosts are already in other \""},{"line_number":71,"context_line":"                                     \"availability zones, so can\u0027t add \""},{"line_number":72,"context_line":"                                     \"host to aggregate. \\nAggregates list: \""},{"line_number":73,"context_line":"                                     \"%s\" % aggregates)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_e6f17ee0","line":70,"in_reply_to":"7faddb67_9653916b","updated":"2019-08-22 09:22:47.000000000","message":"I\u0027d like to raise skipException in this case.\nYes for ci environments, it\u0027s almost impossible to encounter \"all hosts have been added to other zone\", but in production cloud environments, we can\u0027t estimate what our customers will do, and usually no fail testcase is allowed, so, if one testcase fails just because they put all hosts into specific zones, we will have much trouble to explain it:)","commit_id":"ea21684ff0352d1eba5054fdb562e2ec5e6123ea"},{"author":{"_account_id":17896,"name":"Rick Bartra","email":"rickbartra@microsoft.com","username":"rb560u"},"change_message_id":"5c14a082b06a171d240ec8e1e21e9db5dba62612","unresolved":false,"context_lines":[{"line_number":67,"context_line":"                hosts_in_zone.extend(agg[\u0027hosts\u0027])"},{"line_number":68,"context_line":"        hosts \u003d [v for v in hosts_available if v not in hosts_in_zone]"},{"line_number":69,"context_line":"        if not hosts:"},{"line_number":70,"context_line":"            raise self.skipException(\"All hosts are already in other \""},{"line_number":71,"context_line":"                                     \"availability zones, so can\u0027t add \""},{"line_number":72,"context_line":"                                     \"host to aggregate. \\nAggregates list: \""},{"line_number":73,"context_line":"                                     \"%s\" % aggregates)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_6a9bf616","line":70,"in_reply_to":"7faddb67_e6f17ee0","updated":"2019-08-23 14:33:55.000000000","message":"Thanks for the review. I also see it the same way that the test should support production environments. For this particular test, I do not see how the skip can come from configuration.","commit_id":"ea21684ff0352d1eba5054fdb562e2ec5e6123ea"}]}
